Aias00 commented on PR #6821:
URL: https://github.com/apache/shenyu/pull/6821#issuecomment-5193353157
Good catch — hardcoding `HttpStatus.FORBIDDEN` ignored the configured
`statusCode` on the wire, and `setRawStatusCode(int)` is the right way to honor
custom codes. One behavioral change worth a guard before merge:
**Null/non-numeric `statusCode` now produces a 500 instead of a 403 (WAF no
longer fails closed).** `WafHandle.statusCode` is a plain String with no
default initializer (`WafHandle.java:37`), so a reject rule whose JSON omits
`statusCode` deserializes to `null`. In the old code,
`setStatusCode(FORBIDDEN)` ran *before*
`Integer.parseInt(wafHandle.getStatusCode())` for the body — so even when the
parse threw, 403 was already on the response. In the new code
(`WafPlugin.java:67`), `int statusCode =
Integer.parseInt(wafHandle.getStatusCode())` parses *first*, so a
null/non-numeric value throws before any status is set and propagates to the
global error handler (likely 500). For a WAF, a misconfigured reject rule
should fail closed (403), not surface as a 500. Suggested fix:
```java
int statusCode = Optional.ofNullable(wafHandle.getStatusCode())
.filter(NumberUtils::isCreatable)
.map(Integer::parseInt)
.orElse(HttpStatus.FORBIDDEN.value());
```
Also: the two new tests cover 403 and 404 (valid codes) but not the
null/empty/non-numeric `statusCode` case — exactly the path that now 500s. A
test for that case would pin the intended fail-closed behavior.
Minor: `setRawStatusCode` accepts any int, so a user configuring
`statusCode:"200"` makes a WAF reject look like a 200 success to the client.
Restoring configurability is the PR's intent, so this is the operator's
responsibility, but a 4xx/5xx range guard would prevent foot-shooting if
desired.
--
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]
