fix(web): validate script create/update by compilation only
This commit is contained in:
@@ -159,6 +159,54 @@ fun removeCachedMetadata(scriptFile: File) {
|
||||
compiledScriptCache.remove(scriptFile.canonicalPath)
|
||||
}
|
||||
|
||||
fun validateCompilationAndCapture(scriptFile: File): ScriptExecutionResult {
|
||||
synchronized(evalLock) {
|
||||
val oldOut = System.out
|
||||
val oldErr = System.err
|
||||
val buffer = ByteArrayOutputStream()
|
||||
val ps = PrintStream(buffer, true, Charsets.UTF_8.name())
|
||||
|
||||
return try {
|
||||
System.setOut(ps)
|
||||
System.setErr(ps)
|
||||
|
||||
val original = scriptFile.readText()
|
||||
val metadata = metadataForFile(scriptFile, original)
|
||||
val injected = injectArgsBridgeDeclaration(original)
|
||||
val compilationResult = compiledScriptFor(scriptFile, injected)
|
||||
val reports = compilationResult.reports
|
||||
val hasErrorDiagnostics = reports.any {
|
||||
it.severity == ScriptDiagnostic.Severity.ERROR || it.severity == ScriptDiagnostic.Severity.FATAL
|
||||
}
|
||||
val diagnostics = reports
|
||||
.filter { it.severity > ScriptDiagnostic.Severity.DEBUG }
|
||||
.joinToString("\n") {
|
||||
val ex = it.exception?.let { e -> ": ${e::class.simpleName}: ${e.message}" } ?: ""
|
||||
"[${it.severity}] ${it.message}$ex"
|
||||
}
|
||||
|
||||
val output = buffer.toString(Charsets.UTF_8.name()).trim()
|
||||
val finalText = buildString {
|
||||
if (output.isNotEmpty()) appendLine(output)
|
||||
if (diagnostics.isNotEmpty()) appendLine(diagnostics)
|
||||
}.trim()
|
||||
|
||||
ScriptExecutionResult(
|
||||
ok = compilationResult is ResultWithDiagnostics.Success && !hasErrorDiagnostics,
|
||||
output = finalText,
|
||||
metadata = metadata,
|
||||
missingRequiredParams = emptyList(),
|
||||
timedOut = false,
|
||||
)
|
||||
} finally {
|
||||
ps.flush()
|
||||
ps.close()
|
||||
System.setOut(oldOut)
|
||||
System.setErr(oldErr)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fun evalAndCapture(scriptFile: File, requestContext: ScriptRequestContext = ScriptRequestContext()): ScriptExecutionResult {
|
||||
return evalAndCapture(scriptFile, requestContext, enforceRequiredParams = true)
|
||||
}
|
||||
|
||||
@@ -133,7 +133,7 @@ suspend fun handleCreateScript(call: ApplicationCall, scriptsDir: File) {
|
||||
script.writeText(content)
|
||||
removeCachedMetadata(script)
|
||||
|
||||
val result = evalAndCapture(script, ScriptRequestContext(), enforceRequiredParams = false)
|
||||
val result = validateCompilationAndCapture(script)
|
||||
if (!result.ok) {
|
||||
script.delete()
|
||||
removeCachedMetadata(script)
|
||||
@@ -197,7 +197,7 @@ suspend fun handleUpdateScript(call: ApplicationCall, scriptsDir: File) {
|
||||
script.writeText(newContent)
|
||||
removeCachedMetadata(script)
|
||||
|
||||
val result = evalAndCapture(script, ScriptRequestContext(), enforceRequiredParams = false)
|
||||
val result = validateCompilationAndCapture(script)
|
||||
if (!result.ok) {
|
||||
script.writeText(previousContent)
|
||||
removeCachedMetadata(script)
|
||||
|
||||
@@ -103,4 +103,28 @@ class WebAuthAndScriptApiTest : WebHostTestSupport() {
|
||||
assertTrue(body.contains("metadata validation failed"))
|
||||
assertTrue(body.contains("missing required option"))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun createWithRequiredParamsValidatesByCompilationOnly() = withApp { _ ->
|
||||
val create = client.post("/scripts/required-param-script") {
|
||||
bearerRoot()
|
||||
setBody(
|
||||
"""
|
||||
// @desc: required param demo
|
||||
// @param: owner | required=true | desc=Repository owner
|
||||
lateinit var args: Array<String>
|
||||
val owner = args.mapNotNull {
|
||||
val i = it.indexOf('=')
|
||||
if (i <= 0) null else it.substring(0, i) to it.substring(i + 1)
|
||||
}.toMap()["owner"] ?: error("missing required param: owner")
|
||||
println(owner)
|
||||
""".trimIndent()
|
||||
)
|
||||
}
|
||||
assertEquals(HttpStatusCode.Created, create.status)
|
||||
|
||||
val runMissing = client.get("/run/required-param-script") { bearerRoot() }
|
||||
assertEquals(HttpStatusCode.BadRequest, runMissing.status)
|
||||
assertTrue(runMissing.bodyAsText().contains("missing required params: owner"))
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user