Issue #11495 - Add UriCompliance rules that follow the HTTP / URI / Servlet specs for illegal & suspicious characters #11496
Conversation
…ervlet specs for illegal & suspicious characters
joakime
added
Bug
|
See https://jakarta.ee/specifications/servlet/6.0/jakarta-servlet-spec-6.0.html#uri-path-canonicalization for list of Servlet rules. (point 10) |
jetty-core/jetty-http/src/test/java/org/eclipse/jetty/http/HttpURITest.java
Outdated
Show resolved
Hide resolved
Check for illegal and suspicious characters as we are parsing the path. Only look for ambiguous paths if we know there are dots or encodings.
…backslash' into fix/12.0.x/uricompliance-reject-backslash # Conflicts: # jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/DefaultServletTest.java
|
@joakime status of this? |
…icompliance-reject-backslash
|
@gregw this PR is green and ready for review |
| _canonicalPath = null; | ||
| String param = _param; | ||
| _param = null; | ||
| parse(State.PATH, path); |
There was a problem hiding this comment.
I'm cautious about this, as this now allows for a query and fragment to be passed in via this method. Can this be done without a call to parse? perhaps we should check for ? and # characters before parsing... or does the isPathValid already do that?
Note we should javadoc if the path is raw or encoded.
There was a problem hiding this comment.
There's a fair number of places that uses this HttpURI.Mutable.path(String) API.
Interestingly, we actually need to parse the path in some of those places to satisfy the Servlet 6.0 "suspicious characters" rule. (eg: sendRedirect, getRequestDispatcher, etc)
I'm not sure what isPathValid you are mentioning here.
It would be great if we had a parsePath that only did the path, and if it encountered something outside of the scope of PATH (eg: query / fragment / etc) that would also be an error.
There was a problem hiding this comment.
Note we should javadoc if the path is raw or encoded.
As far as I'm concerned, this behavior didn't change for HttpURI.Mutable.path(String).
But since it wasn't originally documented, I'm not sure the intent of this method.
There was a problem hiding this comment.
@joakime I've pushed a fix from my concern.... if you don't like it, we can revert and I'll OK this anyway.
This reverts commit fed10f7.
…ervlet specs for illegal & suspicious characters (#11496) * Issue #11495 - Add UriCompliance rules that follow the HTTP / URI / Servlet specs for illegal & suspicious characters * more illegalPathCharacterData test cases * Correcting SUSPICIOUS_PATH_CHARACTERS semantic (encoded and decoded) * Check for illegal and suspicious characters as we are parsing the path. * Only look for ambiguous paths if we know there are dots or encodings. --------- Co-authored-by: gregw <[email protected]>
Servlet spec 6.0 requires HTTP 400 when the URI contains suspicious characters https://jakarta.ee/specifications/servlet/6.0/jakarta-servlet-spec-6.0.html#uri-path-canonicalization If suspicious sequences are discovered during the prior processing steps, the request must be rejected with a 400 bad request rather than dispatched to the target servlet. Windows absolute path targetTmpPath.toAbsolutePath() includes backslash that is a suspicious character. Included in Jetty 12.0.8 and later. * jetty/jetty.project#11495 * jetty/jetty.project#11496
Servlet spec 6.0 requires HTTP 400 when the URI contains suspicious characters https://jakarta.ee/specifications/servlet/6.0/jakarta-servlet-spec-6.0.html#uri-path-canonicalization If suspicious sequences are discovered during the prior processing steps, the request must be rejected with a 400 bad request rather than dispatched to the target servlet. Windows absolute path targetTmpPath.toAbsolutePath() includes backslash that is a suspicious character. Included in Jetty 12.0.8 and later. * jetty/jetty.project#11495 * jetty/jetty.project#11496
Servlet spec 6.0 requires HTTP 400 when the URI contains suspicious characters https://jakarta.ee/specifications/servlet/6.0/jakarta-servlet-spec-6.0.html#uri-path-canonicalization If suspicious sequences are discovered during the prior processing steps, the request must be rejected with a 400 bad request rather than dispatched to the target servlet. Windows path r.jenkins.getRootDir() includes backslash that is a suspicious character. Included in Jetty 12.0.8 and later. * jetty/jetty.project#11495 * jetty/jetty.project#11496
Servlet spec 6.0 requires HTTP 400 when the URI contains suspicious characters https://jakarta.ee/specifications/servlet/6.0/jakarta-servlet-spec-6.0.html#uri-path-canonicalization If suspicious sequences are discovered during the prior processing steps, the request must be rejected with a 400 bad request rather than dispatched to the target servlet. Windows absolute path targetTmpPath.toAbsolutePath() includes backslash that is a suspicious character. Included in Jetty 12.0.8 and later. * jetty/jetty.project#11495 * jetty/jetty.project#11496
Servlet spec 6.0 requires HTTP 400 when the URI contains suspicious characters https://jakarta.ee/specifications/servlet/6.0/jakarta-servlet-spec-6.0.html#uri-path-canonicalization If suspicious sequences are discovered during the prior processing steps, the request must be rejected with a 400 bad request rather than dispatched to the target servlet. Windows path r.jenkins.getRootDir() includes backslash that is a suspicious character. Included in Jetty 12.0.8 and later. * jetty/jetty.project#11495 * jetty/jetty.project#11496
Adds 2 more UriCompliance.Violations
ILLEGAL_PATH_CHARACTERSfor both the URI and HTTP spec rules about illegal (not encoded) charactersSUSPICIOUS_PATH_CHARACTERSfor Servlet spec rules on suspicious (encoded or not) charactersFixes: #11495