Fix non-ASCII url_prefix never matching the request path - #495
Closed
agu2347 wants to merge 1 commit into
Closed
Conversation
HTTPTask.execute() compares the configured url_prefix directly against the request path. Per PEP 3333, WSGI represents PATH_INFO/SCRIPT_NAME as "native strings" holding the raw request bytes decoded as latin-1 (each byte becomes the code point of the same value), regardless of the request's actual encoding. The configured url_prefix, however, was left as the ordinary Unicode string the user wrote it as, with no equivalent conversion applied. For an ASCII-only url_prefix this happens to work, since UTF-8-encoding pure ASCII text and decoding it back as latin-1 is a no-op. For a non-ASCII url_prefix, the two representations are entirely different strings (verified: a 3-character Japanese prefix produces two visually and byte-wise unrelated strings), so the comparison in execute() could never succeed, making it impossible to mount an application behind any non-ASCII url_prefix at all. Apply the same "UTF-8 bytes decoded as latin-1" conversion to url_prefix at config-parsing time (in the slash_fixed_str converter, the only place it's used), matching the exact fix suggested in the issue. This only affects non-ASCII url_prefix values; ASCII values, the empty string, and the existing slash-normalization behavior are completely unaffected, since the added encode/decode round-trip is a no-op for pure ASCII input. Verified directly: constructed an Adjustments instance with a non-ASCII url_prefix and confirmed it now exactly matches the latin-1-decoded UTF-8 bytes representation that HTTPTask.execute() would compare it against for an incoming request to that path -- the exact mismatch described in the issue is resolved. Also confirmed ASCII url_prefix values, the empty string, and slash-normalization (e.g. "///foo/" -> "/foo") are unaffected. Added regression tests: one confirming a non-ASCII url_prefix is correctly converted to match the WSGI path-encoding convention, and one confirming ASCII url_prefix values are unaffected. Confirmed the non-ASCII test fails with the original code (produces two unrelated strings) and passes with the fix. Ran the full existing test_adjustments.py and test_task.py suites: 124 passed (122 baseline + 2 new); the 3 remaining failures (socket-binding tests) are pre-existing sandbox limitations, confirmed identical on a clean checkout of main. Fixes Pylons#492
Member
|
This smells like it was generated by an LLM. I'm not sure what the Pylon project's stance is on LLM-generated contributions, and this duplicates #493. Because it duplicates that PR, I'm closing this. |
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.
Fixes #492.
HTTPTask.execute()compares the configuredurl_prefixdirectly against the request path. Per PEP 3333, WSGI representsPATH_INFO/SCRIPT_NAMEas "native strings" holding the raw request bytes decoded as latin-1 (each byte becomes the code point of the same value), regardless of the request's actual encoding. The configuredurl_prefix, however, was left as the ordinary Unicode string the user wrote it as, with no equivalent conversion applied.For an ASCII-only
url_prefixthis happens to work, since UTF-8-encoding pure ASCII text and decoding it back as latin-1 is a no-op. For a non-ASCIIurl_prefix, the two representations are entirely different strings (verified: a 3-character Japanese prefix produces two visually and byte-wise unrelated strings), so the comparison inexecute()could never succeed, making it impossible to mount an application behind any non-ASCIIurl_prefixat all -- exactly as described in the issue.Fix: apply the same "UTF-8 bytes decoded as latin-1" conversion to
url_prefixat config-parsing time (in theslash_fixed_strconverter, the only place it's used), matching the exact fix suggested in the issue. This only affects non-ASCIIurl_prefixvalues; ASCII values, the empty string, and the existing slash-normalization behavior are completely unaffected, since the added encode/decode round-trip is a no-op for pure ASCII input.Testing: verified directly: constructed an
Adjustmentsinstance with a non-ASCIIurl_prefixand confirmed it now exactly matches the latin-1-decoded UTF-8 bytes representation thatHTTPTask.execute()would compare it against for an incoming request to that path -- the exact mismatch described in the issue is resolved. Also confirmed ASCIIurl_prefixvalues, the empty string, and slash-normalization (e.g."///foo/"->"/foo") are unaffected.Added regression tests: one confirming a non-ASCII
url_prefixis correctly converted to match the WSGI path-encoding convention, and one confirming ASCIIurl_prefixvalues are unaffected. I confirmed the non-ASCII test fails with the original code (produces two unrelated strings) and passes with the fix. Ran the full existingtest_adjustments.pyandtest_task.pysuites: 124 passed (122 baseline + 2 new); the 3 remaining failures (socket-binding tests) are pre-existing sandbox limitations, confirmed identical on a clean checkout ofmain.