Repository navigation
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the
 |
Port of the multipart part of owasp-modsecurity#3657 to 2.x. The Content-Disposition parser returned bare negative numbers that only showed up as "Invalid Content-Disposition header (-15)" in the logs, and -15 was shared by the duplicate filename and duplicate filename* cases. The codes are now an enum with a message table, the log line reads "(-20: duplicate filename parameter)", and the two duplicate cases get their own codes (-20, -21). Existing numeric values are unchanged because tests and log matching pin them. The m.setvar() return value fix of the original version of this change landed separately in owasp-modsecurity#3736. Related to owasp-modsecurity#3655 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0181jBv5JdNQviwDzfnkf1Gy
The previous commit changed the REQBODY_ERROR_MSG text from "(-N): header" to "(-N: description): header" and split the shared -15 into -20 and -21, but left six existing expectations on the old text. Update them to the new codes and descriptions. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Tz1sULBvqAwnRreKh5eC9h
multipart_parse_content_disposition() still returned bare numbers on all paths except the two duplicate filename cases, so the enum and the code it was meant to document could drift apart. Return the enum constants everywhere, as the 3.x parser does. The numeric values do not change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XeLG4vp4DU9WxVbQHwgWT3
fzipi
force-pushed
the
v2/issue-3655-error-codes
branch
from
November 10, 2026 22:45
ae3ad9b to
8301a11
Compare
|
fzipi
changed the title
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
2.x counterpart of the internal part of #3657:
apache2/msc_multipart.c: the Content-Disposition parser's bare negative return codes become an enum with a message table, and every error path now returns the named code. The error text now readsInvalid Content-Disposition header (-20: duplicate filename parameter): ...instead of(-15). The two duplicate cases that shared-15get their own codes (-20filename,-21filename*). All other numeric values are unchanged, since tests and log matching pin them;-9stays reserved (the tolerated IE escaping case) and-15is retired.The Lua
m.setvarreturn value fix that was originally part of this PR is dropped: #3736 already landed it onv2/master. The branch is rebased onto currentv2/masterand no longer touchesapache2/msc_lua.c.Why
The numbers alone were useless in the logs and
-15was ambiguous. Same reasoning as #3655, applied to 2.x.Tests
New
tests/regression/misc/70-multipart-cd-errors.t: duplicatefilenameand duplicatefilename*parts, asserting the new-20/-21text throughREQBODY_ERROR_MSG.Verified in an Ubuntu 24.04 container (apache2 2.4) after the rebase:
misc/70-multipart-cd-errors.t(2),target/10-multipart_filename.t(8),target/10-variable-multipart_strict_error.t(14),misc/10-multipart-filenamestar.t(7) andmisc/00-multipart-parser.t(42) all pass.References
Related to #3655, #3657
🤖 Generated with Claude Code