From 4dd9dc8ae03eb9ed2cf9c5520d081d5d3ea27ca7 Mon Sep 17 00:00:00 2001 From: Renn F Date: Sat, 17 Oct 2026 06:46:51 +0200 Subject: [PATCH 1/4] fix(filter): keep multipart and urlencoded form parsing working with body scanning on (#37) The guard reads the body for detection, but the container parses parts and parameters from its own input and never saw the replay. Multipart is now scanned from the container-parsed parts and urlencoded body parameters are merged from the cache. --- .../guardcore/spring/ServletGuardRequest.kt | 131 +++++++++++++++++ .../spring/GuardMultipartBodyScanTest.kt | 132 ++++++++++++++++++ 2 files changed, 263 insertions(+) create mode 100644 src/test/kotlin/com/guardcore/spring/GuardMultipartBodyScanTest.kt diff --git a/src/main/kotlin/com/guardcore/spring/ServletGuardRequest.kt b/src/main/kotlin/com/guardcore/spring/ServletGuardRequest.kt index 070994f..5d36bdc 100644 --- a/src/main/kotlin/com/guardcore/spring/ServletGuardRequest.kt +++ b/src/main/kotlin/com/guardcore/spring/ServletGuardRequest.kt @@ -4,6 +4,7 @@ import com.guardcore.protocols.BoundedBodyReader import com.guardcore.protocols.GuardRequest import com.guardcore.protocols.RequestState import jakarta.servlet.ReadListener +import jakarta.servlet.ServletException import jakarta.servlet.ServletInputStream import jakarta.servlet.http.HttpServletRequest import jakarta.servlet.http.HttpServletRequestWrapper @@ -162,6 +163,11 @@ public class MapRequestState : RequestState { * The body-caching request wrapper. Reads the servlet input stream exactly once into a growable * cache; bounded prefix reads stop pulling from the wire after the prefix while the downstream * servlet stream replays the cached bytes before continuing with the live remainder. + * + * The container's own form parsing (`getParts` for multipart, `getParameter*` for urlencoded + * bodies) reads the container's input directly and never sees this replay, so the two form flavors + * are handled apart from the replay cache: multipart is scanned from the container-parsed parts, + * and urlencoded bodies are merged into the parameter accessors from the cache. */ public class GuardHttpServletRequest(request: HttpServletRequest) : HttpServletRequestWrapper(request) { @@ -169,9 +175,20 @@ public class GuardHttpServletRequest(request: HttpServletRequest) : private val cache = ByteCache() private var streamOpened: Boolean = false + private var multipartPrefix: ByteArray? = null + private var multipartPrefixLimit: Int = 0 + + private val isMultipart: Boolean + get() = contentType?.lowercase()?.startsWith(MULTIPART_FORM_DATA) == true + + private val isUrlEncodedForm: Boolean + get() = contentType?.lowercase()?.startsWith(URLENCODED_FORM) == true @Synchronized public fun cachedBody(): ByteArray { + multipartScanBytes(Int.MAX_VALUE)?.let { + return it + } openStreamIfNecessary() cache.readUpstream() return cache.snapshot() @@ -182,11 +199,116 @@ public class GuardHttpServletRequest(request: HttpServletRequest) : if (maxBytes <= 0) { return ByteArray(0) } + multipartScanBytes(maxBytes)?.let { + return it.copyOfRange(0, minOf(maxBytes, it.size)) + } openStreamIfNecessary() cache.readUpstream(maxBytes) return cache.snapshot().copyOfRange(0, minOf(maxBytes, cache.size())) } + /** + * A multipart/form-data body rebuilt from the container-parsed parts (headers, then at most + * [maxBytes] of each part's payload), which is what the engine's multipart scan consumes (it + * scans file names, part headers and part payloads, the way the Python reference parses the raw + * body). The container spools large file parts to disk, so no upload is held in memory. Null when + * the request is not multipart or the container cannot parse parts (no multipart config); the raw + * replay path then applies. + */ + private fun multipartScanBytes(maxBytes: Int): ByteArray? { + if (!isMultipart || streamOpened) { + return null + } + multipartPrefix?.let { built -> + if (multipartPrefixLimit >= maxBytes) { + return built + } + } + val boundary = multipartBoundary() ?: return null + val parts = + try { + super.getParts() + } catch (_: ServletException) { + return null + } catch (_: IllegalStateException) { + return null + } catch (_: java.io.IOException) { + return null + } + val out = java.io.ByteArrayOutputStream() + for (part in parts) { + if (out.size() >= maxBytes) { + break + } + out.write("--$boundary\r\n".toByteArray()) + for (name in part.headerNames) { + for (value in part.getHeaders(name)) { + out.write("$name: $value\r\n".toByteArray()) + } + } + out.write("\r\n".toByteArray()) + part.inputStream.use { out.write(it.readNBytes(maxOf(0, maxBytes - out.size()))) } + out.write("\r\n".toByteArray()) + } + out.write("--$boundary--\r\n".toByteArray()) + return out.toByteArray().also { + multipartPrefix = it + multipartPrefixLimit = maxBytes + } + } + + private fun multipartBoundary(): String? = + BOUNDARY_PARAM.find(contentType ?: "")?.groupValues?.get(1) + + /** Body parameters of a urlencoded form, appended after the query parameters (Tomcat order). */ + private fun mergedParameters(): Map>? { + if (!streamOpened || !isUrlEncodedForm) { + return null + } + // Drain the live remainder first so the container's own parameter parse sees a finished stream + // instead of parsing the unread tail as if it were the whole body. + cachedBody() + val merged = linkedMapOf>() + for ((name, values) in super.getParameterMap()) { + merged.getOrPut(name) { mutableListOf() }.addAll(values) + } + val charset = characterEncoding?.let { Charset.forName(it) } ?: Charsets.UTF_8 + for (pair in String(cache.snapshot(), Charsets.ISO_8859_1).split("&")) { + if (pair.isEmpty()) { + continue + } + val separator = pair.indexOf('=') + val rawName = if (separator < 0) pair else pair.substring(0, separator) + val rawValue = if (separator < 0) "" else pair.substring(separator + 1) + merged + .getOrPut(decodeForm(rawName, charset)) { mutableListOf() } + .add(decodeForm(rawValue, charset)) + } + return merged.mapValues { it.value.toTypedArray() } + } + + private fun decodeForm(raw: String, charset: Charset): String = + URLDecoder.decode(String(raw.toByteArray(Charsets.ISO_8859_1), charset), charset) + + private var formParameterCache: Map>? = null + + @Synchronized + private fun formParameters(): Map>? = + formParameterCache ?: mergedParameters()?.also { formParameterCache = it } + + override fun getParameterMap(): Map> = + formParameters() ?: super.getParameterMap() + + override fun getParameter(name: String): String? = + formParameters()?.let { it[name]?.firstOrNull() } ?: super.getParameter(name) + + override fun getParameterValues(name: String): Array? = + formParameters()?.let { it[name] } ?: super.getParameterValues(name) + + override fun getParameterNames(): java.util.Enumeration = + formParameters()?.let { java.util.Collections.enumeration(it.keys) } + ?: super.getParameterNames() + private fun openStreamIfNecessary() { if (!streamOpened) { streamOpened = true @@ -195,6 +317,9 @@ public class GuardHttpServletRequest(request: HttpServletRequest) : } override fun getInputStream(): ServletInputStream { + if (isMultipart && multipartPrefix != null) { + return super.getInputStream() + } openStreamIfNecessary() return cache.replayingServletInputStream() } @@ -206,6 +331,12 @@ public class GuardHttpServletRequest(request: HttpServletRequest) : request.characterEncoding?.let { Charset.forName(it) } ?: Charsets.UTF_8, ), ) + + private companion object { + const val MULTIPART_FORM_DATA = "multipart/form-data" + const val URLENCODED_FORM = "application/x-www-form-urlencoded" + val BOUNDARY_PARAM = Regex("""boundary="?([^";\s]+)"?""", RegexOption.IGNORE_CASE) + } } /** Single-source byte cache serving both the facade reads and the downstream servlet stream. */ diff --git a/src/test/kotlin/com/guardcore/spring/GuardMultipartBodyScanTest.kt b/src/test/kotlin/com/guardcore/spring/GuardMultipartBodyScanTest.kt new file mode 100644 index 0000000..4a98de0 --- /dev/null +++ b/src/test/kotlin/com/guardcore/spring/GuardMultipartBodyScanTest.kt @@ -0,0 +1,132 @@ +package com.guardcore.spring + +import java.net.URI +import java.net.http.HttpClient +import java.net.http.HttpRequest +import java.net.http.HttpResponse +import org.assertj.core.api.Assertions.assertThat +import org.junit.jupiter.api.Test +import org.springframework.beans.factory.annotation.Value +import org.springframework.boot.SpringBootConfiguration +import org.springframework.boot.autoconfigure.EnableAutoConfiguration +import org.springframework.boot.test.context.SpringBootTest +import org.springframework.test.context.TestPropertySource +import org.springframework.web.bind.annotation.PostMapping +import org.springframework.web.bind.annotation.RequestParam +import org.springframework.web.bind.annotation.RequestPart +import org.springframework.web.bind.annotation.RestController +import org.springframework.web.multipart.MultipartFile + +@RestController +public class MultipartTestController { + @PostMapping("/m/upload") + public fun upload( + @RequestPart("file") file: MultipartFile, + @RequestParam("note", required = false) note: String?, + ): Map = + mapOf( + "name" to file.originalFilename, + "size" to file.size, + "content" to String(file.bytes, Charsets.ISO_8859_1), + "note" to note, + ) + + @PostMapping("/m/form") + public fun form(@RequestParam("a") a: String): Map = mapOf("a" to a) +} + +@SpringBootConfiguration +@EnableAutoConfiguration +@org.springframework.context.annotation.Import(MultipartTestController::class) +public open class MultipartGuardTestApp + +@SpringBootTest( + classes = [MultipartGuardTestApp::class], + webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT, +) +@TestPropertySource( + properties = + [ + "guard.enable-redis=false", + "guard.trusted-proxies[0]=127.0.0.1", + "guard.detection-scan-body=true", + "guard.detection-max-body-inspect-bytes=2048", + "spring.servlet.multipart.max-file-size=8MB", + "spring.servlet.multipart.max-request-size=9MB", + ], +) +public class GuardMultipartBodyScanTest { + @Value("\${local.server.port}") public var port: Int = 0 + + private val client = HttpClient.newHttpClient() + private val nextClient = java.util.concurrent.atomic.AtomicInteger(0) + private val boundary = "----guardBoundary7MA4YWxkTrZu0gW" + + private fun multipart(file: ByteArray, note: String? = null): ByteArray { + val out = java.io.ByteArrayOutputStream() + fun text(s: String) = out.write(s.toByteArray(Charsets.ISO_8859_1)) + if (note != null) { + text("--$boundary\r\nContent-Disposition: form-data; name=\"note\"\r\n\r\n$note\r\n") + } + text( + "--$boundary\r\nContent-Disposition: form-data; name=\"file\"; filename=\"a.bin\"\r\n" + + "Content-Type: application/octet-stream\r\n\r\n", + ) + out.write(file) + text("\r\n--$boundary--\r\n") + return out.toByteArray() + } + + private fun post(path: String, contentType: String, body: ByteArray): HttpResponse = + client.send( + HttpRequest.newBuilder(URI("http://127.0.0.1:$port$path")) + .header("Content-Type", contentType) + .header("User-Agent", "Mozilla/5.0") + .header("X-Forwarded-For", "198.51.100.${nextClient.incrementAndGet()}") + .POST(HttpRequest.BodyPublishers.ofByteArray(body)) + .build(), + HttpResponse.BodyHandlers.ofString(), + ) + + private fun upload(file: ByteArray, note: String? = null): HttpResponse = + post("/m/upload", "multipart/form-data; boundary=$boundary", multipart(file, note)) + + @Test + public fun `a small multipart upload reaches the controller intact with body scanning on`() { + val response = upload("hello multipart".toByteArray(), note = "hi") + assertThat(response.statusCode()).isEqualTo(200) + assertThat(response.body()) + .contains("\"name\":\"a.bin\"", "\"size\":15", "hello multipart", "\"note\":\"hi\"") + } + + @Test + public fun `a multipart upload far larger than the inspect cap reaches the controller intact`() { + val big = ByteArray(5 * 1024 * 1024) { ('a'.code + it % 26).toByte() } + val response = upload(big) + assertThat(response.statusCode()).isEqualTo(200) + assertThat(response.body()).contains("\"size\":${big.size}") + assertThat(response.body()).contains(String(big, 0, 64, Charsets.ISO_8859_1)) + assertThat(response.body()).contains(String(big, big.size - 64, 64, Charsets.ISO_8859_1)) + } + + @Test + public fun `an attack payload in a multipart text field is still blocked`() { + val response = + upload("harmless".toByteArray(), note = "1 UNION SELECT username, password FROM users") + assertThat(response.statusCode()).isEqualTo(400) + assertThat(response.body()).isEqualTo("Suspicious activity detected") + } + + @Test + public fun `an attack payload inside a text file part is blocked like the reference`() { + val response = upload("".toByteArray()) + assertThat(response.statusCode()).isEqualTo(400) + } + + @Test + public fun `a urlencoded form still binds request params with body scanning on`() { + val response = post("/m/form", "application/x-www-form-urlencoded", "a=1".toByteArray()) + assertThat(response.statusCode()).isEqualTo(200) + assertThat(response.body()).contains("\"a\":\"1\"") + } +} From f4481cfa0a7ff67802b1046052fd9e9953183918 Mon Sep 17 00:00:00 2001 From: Renn F Date: Sat, 17 Oct 2026 06:46:52 +0200 Subject: [PATCH 2/4] feat(config): add guard.enabled to switch the auto-configuration off (#38) Defaults to true. With false no guard bean, filter or properties binding is created. --- CHANGELOG.md | 11 ++++++ README.md | 4 ++ docs/installation.md | 4 ++ .../spring/GuardSecurityAutoConfiguration.kt | 9 ++++- .../GuardSecurityAutoConfigurationTest.kt | 39 +++++++++++++++++++ 5 files changed, 66 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4f78321..db7cc74 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,16 @@ # Changelog +## Unreleased + +### Added + +- `guard.enabled` (default `true`) switches the whole auto-configuration off when set to `false`: no guard bean, filter or properties binding is created. Existing apps see no change. + +### Fixed + +- A `multipart/form-data` upload no longer loses its parts when `guard.detection-scan-body` is on (`Required part 'file' is not present`). The container parses the parts itself and never saw the guard's body replay, so the guard now scans a body rebuilt from the container-parsed parts (file names, part headers and at most `guard.detection-max-body-inspect-bytes` of each part payload, the surface the engine's multipart scan covers). Large file parts stay in the container's spool and are not held in memory. The container parses the upload before the detection check runs, within its own multipart size limits. +- `application/x-www-form-urlencoded` bodies bound through `@RequestParam` or `request.getParameter` no longer come back empty when body scanning is on, for the same reason. The body parameters are now merged after the query parameters. + ## [1.2.0] - 2026-10-17 ### Fixed diff --git a/README.md b/README.md index 91ed560..f0e51ec 100644 --- a/README.md +++ b/README.md @@ -159,6 +159,10 @@ Initialization follows the fastapi-guard contract: lazy on the first request beh (the `guard.lifespan` analog); with `guard.lazy-init=false` and no eager init, the first request logs the same "eager init not honored" warning as fastapi-guard. +### Enabling and disabling + +The guard is on by default as soon as the jar is on the classpath. Set `guard.enabled=false` to switch the whole auto-configuration off: no guard bean, filter or `guard.*` properties binding is created. An app that wants the guard opt-in sets `guard.enabled=false` as its default and turns it on per environment with `guard.enabled=true`. + ## Engine dependency (includeBuild + dependency substitution) The engine artifact `com.guard-core:guard-core` (groupId is the Central Portal namespace; Kotlin diff --git a/docs/installation.md b/docs/installation.md index b00b0d8..ac266d1 100644 --- a/docs/installation.md +++ b/docs/installation.md @@ -51,6 +51,10 @@ Maven with the BOM imported (declare these imports before `spring-boot-dependenc With `spring-boot-starter-parent` as the parent, set `2.4.21` and `1.11.0` as properties instead. +## Enabling and disabling + +The guard is on by default as soon as the jar is on the classpath. Set `guard.enabled=false` to switch the whole auto-configuration off: no guard bean, filter or `guard.*` properties binding is created. An app that wants the guard opt-in sets `guard.enabled=false` as its default and turns it on per environment with `guard.enabled=true`. + ## Engine dependency (includeBuild + dependency substitution) Consumers resolve the engine artifact `com.guard-core:guard-core` from Maven Central. This repository's own build pins the engine from source instead: CI and diff --git a/src/main/kotlin/com/guardcore/spring/GuardSecurityAutoConfiguration.kt b/src/main/kotlin/com/guardcore/spring/GuardSecurityAutoConfiguration.kt index fff0312..572b7ba 100644 --- a/src/main/kotlin/com/guardcore/spring/GuardSecurityAutoConfiguration.kt +++ b/src/main/kotlin/com/guardcore/spring/GuardSecurityAutoConfiguration.kt @@ -30,11 +30,18 @@ import org.springframework.web.servlet.config.annotation.WebMvcConfigurer * Beans: SecurityConfig (guard.* properties + customizers), SpringGuardDecorator, GuardMiddleware * (the engine-facing middleware), GuardFilter (registered over every dispatcher type so error * dispatches stay guarded too, matching the ASGI middleware's envelope). `guard.eager-init=true` - * additionally registers the GuardEagerInitializer boot hook. + * additionally registers the GuardEagerInitializer boot hook. `guard.enabled=false` switches the + * whole auto-configuration off (no bean, filter or properties binding); the default is on. */ @AutoConfiguration @ConditionalOnWebApplication(type = ConditionalOnWebApplication.Type.SERVLET) @ConditionalOnClass(HttpServletRequest::class, DispatcherServlet::class) +@ConditionalOnProperty( + prefix = "guard", + name = ["enabled"], + havingValue = "true", + matchIfMissing = true, +) @EnableConfigurationProperties(GuardProperties::class) public open class GuardSecurityAutoConfiguration { @Bean diff --git a/src/test/kotlin/com/guardcore/spring/GuardSecurityAutoConfigurationTest.kt b/src/test/kotlin/com/guardcore/spring/GuardSecurityAutoConfigurationTest.kt index b2d5a05..0193aae 100644 --- a/src/test/kotlin/com/guardcore/spring/GuardSecurityAutoConfigurationTest.kt +++ b/src/test/kotlin/com/guardcore/spring/GuardSecurityAutoConfigurationTest.kt @@ -1,7 +1,11 @@ package com.guardcore.spring +import org.assertj.core.api.Assertions.assertThat import org.junit.jupiter.api.Assertions.assertNotNull import org.junit.jupiter.api.Test +import org.springframework.boot.autoconfigure.AutoConfigurations +import org.springframework.boot.test.context.runner.WebApplicationContextRunner +import org.springframework.boot.web.servlet.FilterRegistrationBean /** The W0 scaffold gate: the auto-configuration type is instantiable without a context. */ public class GuardSecurityAutoConfigurationTest { @@ -11,3 +15,38 @@ public class GuardSecurityAutoConfigurationTest { assertNotNull(configuration) } } + +/** `guard.enabled`: on when missing or true, nothing registered when false. */ +public class GuardEnabledPropertyTest { + private val runner = + WebApplicationContextRunner() + .withConfiguration(AutoConfigurations.of(GuardSecurityAutoConfiguration::class.java)) + .withPropertyValues("guard.enable-redis=false") + + @Test + public fun enabledWhenPropertyIsMissing() { + runner.run { context -> + assertThat(context).hasSingleBean(GuardMiddleware::class.java) + assertThat(context).hasSingleBean(GuardProperties::class.java) + } + } + + @Test + public fun enabledWhenPropertyIsTrue() { + runner.withPropertyValues("guard.enabled=true").run { context -> + assertThat(context).hasSingleBean(GuardMiddleware::class.java) + } + } + + @Test + public fun nothingRegisteredWhenPropertyIsFalse() { + runner.withPropertyValues("guard.enabled=false").run { context -> + assertThat(context).doesNotHaveBean(GuardSecurityAutoConfiguration::class.java) + assertThat(context).doesNotHaveBean(GuardProperties::class.java) + assertThat(context).doesNotHaveBean(GuardMiddleware::class.java) + assertThat(context).doesNotHaveBean(SpringGuardDecorator::class.java) + assertThat(context).doesNotHaveBean(AnnotationRouteConfigurator::class.java) + assertThat(context.getBeansOfType(FilterRegistrationBean::class.java)).isEmpty() + } + } +} From 3edc26732142a757aa08b16f991552952deed12c Mon Sep 17 00:00:00 2001 From: Renn F Date: Sat, 17 Oct 2026 08:53:32 +0200 Subject: [PATCH 3/4] docs(filter): state that the multipart scan cap is on the whole rebuilt body (#37) --- .../com/guardcore/spring/ServletGuardRequest.kt | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/main/kotlin/com/guardcore/spring/ServletGuardRequest.kt b/src/main/kotlin/com/guardcore/spring/ServletGuardRequest.kt index 5d36bdc..87a37f9 100644 --- a/src/main/kotlin/com/guardcore/spring/ServletGuardRequest.kt +++ b/src/main/kotlin/com/guardcore/spring/ServletGuardRequest.kt @@ -208,12 +208,13 @@ public class GuardHttpServletRequest(request: HttpServletRequest) : } /** - * A multipart/form-data body rebuilt from the container-parsed parts (headers, then at most - * [maxBytes] of each part's payload), which is what the engine's multipart scan consumes (it - * scans file names, part headers and part payloads, the way the Python reference parses the raw - * body). The container spools large file parts to disk, so no upload is held in memory. Null when - * the request is not multipart or the container cannot parse parts (no multipart config); the raw - * replay path then applies. + * A multipart/form-data body rebuilt from the container-parsed parts (each part's headers, then + * its payload), reading payloads only until the rebuilt body reaches [maxBytes] in total. This is + * what the engine's multipart scan consumes (it scans file names, part headers and part payloads, + * the way the Python reference parses the capped raw body). The container spools large file parts + * to disk, so at most [maxBytes] of an upload is held in memory. Null when the request is not + * multipart or the container cannot parse parts (no multipart config); the raw replay path then + * applies. */ private fun multipartScanBytes(maxBytes: Int): ByteArray? { if (!isMultipart || streamOpened) { From 66970ec34d0a726325b8204cc9f9fcffcc2988a1 Mon Sep 17 00:00:00 2001 From: Renn F Date: Sat, 17 Oct 2026 08:53:58 +0200 Subject: [PATCH 4/4] docs(changelog): state that the multipart scan cap is on the whole rebuilt body (#37) --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index db7cc74..b0c80d3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,7 @@ ### Fixed -- A `multipart/form-data` upload no longer loses its parts when `guard.detection-scan-body` is on (`Required part 'file' is not present`). The container parses the parts itself and never saw the guard's body replay, so the guard now scans a body rebuilt from the container-parsed parts (file names, part headers and at most `guard.detection-max-body-inspect-bytes` of each part payload, the surface the engine's multipart scan covers). Large file parts stay in the container's spool and are not held in memory. The container parses the upload before the detection check runs, within its own multipart size limits. +- A `multipart/form-data` upload no longer loses its parts when `guard.detection-scan-body` is on (`Required part 'file' is not present`). The container parses the parts itself and never saw the guard's body replay, so the guard now scans a body rebuilt from the container-parsed parts (file names, part headers and part payloads, the surface the engine's multipart scan covers), reading payloads only until the rebuilt body reaches `guard.detection-max-body-inspect-bytes` in total. Large file parts stay in the container's spool, so at most that many bytes of an upload are held in memory. The container parses the upload before the detection check runs, within its own multipart size limits. - `application/x-www-form-urlencoded` bodies bound through `@RequestParam` or `request.getParameter` no longer come back empty when body scanning is on, for the same reason. The body parameters are now merged after the query parameters. ## [1.2.0] - 2026-10-17