{{ message }}
Repository navigation
FEAT: Add configurable safe parameter to UrlConverter - #3402
Open
Vardhman Gupta (Kaap10) wants to merge 2 commits into
Open
Vardhman Gupta (Kaap10) wants to merge 2 commits into
Vardhman Gupta (Kaap10) wants to merge 2 commits into
Conversation
Richard Lundeen (richlundeen)
left a comment
Richard Lundeen (richlundeen)
left a comment
Contributor
There was a problem hiding this comment.
This changes the behavior of every existing UrlConverter() call. urllib.parse.quote currently defaults to safe="/", but the new default is safe="", so inputs with slashes now produce different prompts without any caller opting in.
If the configurable option is needed, please preserve safe="/" as the default and let callers request safe="" explicitly. The forwarding option and identifier are enough; the extra property and manual string-type check are not needed for this change. Otherwise, I recommend closing rather than changing the default behavior for this use case.
Reviewed with GitHub Copilot.
Keep slash preservation as the default and make full encoding opt-in. Remove redundant validation and the safe property while retaining behavioral identifiers and reconstruction. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Richard Lundeen (richlundeen)
approved these changes
Oct 25, 2026
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.
Description
Closes #3401
Adds a configurable
safeparameter toUrlConverter:safe="": Enforces complete URL encoding of reserved characters by default, including forward slashes (/->%2F), ensuring path traversal (../../etc/passwd) and SSRF payloads are properly encoded for evasion testing.safeSupport: Callers wishing to preserve specific characters (such as/or/:) can configureUrlConverter(safe="/")or any custom string._build_identifier()to serializeparams={"safe": self._safe}, adhering to the converter contract.safeis a string, raisingTypeErrorotherwise.Tests and Documentation
test_url_converter_default_encodes_forward_slashverifying that path traversal and SSRF payloads have slashes encoded as%2Fby default.test_url_converter_custom_safetestingsafe="/",safe="/:", and query formattingsafe="?=".test_url_converter_property_and_identifiertesting the property and identifier parameter serialization.test_url_converter_invalid_safe_typeverifyingTypeErroron invalid arguments.test_url_converter_reconstructionverifying reconstruction parameters.