wy471x opened a new pull request, #7061: URL: https://github.com/apache/shenyu/pull/7061
<!-- Describe your PR here; e.g. Fixes #issueNo --> <!-- Thank you for proposing a pull request. This template will guide you through the essential steps necessary for a pull request. --> Make sure that: - [X] You have read the [contribution guidelines](https://shenyu.apache.org/community/contributor-guide). - [X] You submit test cases (unit or integration tests) that back your changes. - [X] Your local test passed `./mvnw clean install -Dmaven.javadoc.skip=true`. ## Summary Fixes #6627. `FileSizeFilter` put no bound on what it buffered: the constructor built its codecs with `maxInMemorySize(-1)`, so `serverRequest.bodyToMono(DataBuffer.class)` joined the **entire** multipart body in memory and the size check only ran after that. A client could stream an arbitrarily large multipart body and have the gateway buffer all of it before the request was finally rejected — a memory-exhaustion DoS (one request can take the process down). The bound had been removed on purpose in #4507 ("Corrected `maxInMemorySize` to `-1` to resolve the `DataBufferLimitException: Exceeded limit on max bytes to buffer` error"), because `DataBufferLimitException` was not handled and broke the existing rejection response. This PR re-introduces the bound **and** handles that exception, which is what makes it safe. ### Changes: 1. `FileSizeFilter` (`FileSizeFilter.java:65`, `:73`) — new `maxInMemorySize` field, computed as `fileMaxSize * Constants.BYTES_PER_MB` (saturating at `Integer.MAX_VALUE` to avoid `int` overflow) and passed to `configurer.defaultCodecs().maxInMemorySize(...)`, so the codec aborts while buffering as soon as the configured limit is crossed instead of buffering the whole body first. 2. `FileSizeFilter.filter` (`FileSizeFilter.java:111`) — `onErrorResume(DataBufferLimitException.class, ...)` maps the codec limit trip to the same rejection response the filter already produced (`400` + `ShenyuResultEnum.PAYLOAD_TOO_LARGE`), preserving the existing API behavior for oversized uploads. 3. `FileSizeFilter.filter` (`FileSizeFilter.java:83`) — a non-positive `shenyu.file.max-size` now rejects the multipart request before reading the body at all, so no configuration is left with unbounded buffering (previously such a request was buffered in full and only then rejected). 4. `FileSizeFilter.filter` (`FileSizeFilter.java:90`) — the rejection branch now releases the joined pooled `DataBuffer` before returning; the existing `.doFinally` release only covered the success path, so every oversized upload leaked one pooled buffer (the leak tracked in #6626). 5. `FileSizeFilter.payloadTooLarge` (`FileSizeFilter.java:125`) / `maxInMemorySize` (`FileSizeFilter.java:141`) — the three rejection sources now share one response/logging path instead of duplicating the status + error construction. ### Test Cases: - `FileSizeFilterTest#testFilterRejectsOversizedBodyWhileBuffering` (`FileSizeFilterTest.java:128`) — sends 4 × 512 KB against a 1 MB limit and asserts the request is rejected with `400` while the body is *not* buffered in full (only the buffers consumed until the limit trips) and the filter chain is never called. With the previous `maxInMemorySize(-1)` the same test fails with `consumed buffers: 4`, i.e. it pins the vulnerability. - Existing `FileSizeFilterTest#testFilter` / `#testDecorate` are unchanged and still pass, including the `fileMaxSize = -1` rejection case. ## Verification - `./mvnw clean install -Dmaven.javadoc.skip=true` on JDK 21: whole reactor passes, with the one pre-existing order-dependent test excluded (`-Dtest='!DubboReconcilerTest' -DfailIfNoTests=false`). `org.apache.shenyu.k8s.DubboReconcilerTest` shares the static `IngressCache` with `WebSocketReconcilerTest` / `DivideIngressReconcilerTest` (all use `mockedNamespace/mockedIngress`) and fails purely on test execution order — it passes in isolation and reproduces the same failure on unmodified `master`; `shenyu-kubernetes-controller` is unrelated to this change. - `shenyu-web` module: 82 tests, 0 failures. - `checkstyle:check` — 0 violations; `mvn validate` (checkstyle + Apache RAT) passes. Note: the release in change 4 also fixes the pooled-buffer leak tracked in #6626, since it is the same rejection path. @Aias00, could you please help review this PR? Thank you! close #6627 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
