gh-76960: Fix urljoin() and urldefrag() for URIs with empty components #123273
+139
−52
Conversation
This file contains 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
…ponents * urljoin() with relative reference "?" sets empty query and removes fragment. * Preserve empty components (authority, params, query, fragment) in urljoin(). * Preserve empty components (authority, params, query) in urldefrag(). Also refactor the code and get rid of double _coerce_args() and _coerce_result() calls in urljoin(), urldefrag(), urlparse() and urlunparse().
Comment on lines
-529
to
-530
| self.checkJoin(SIMPLE_BASE, 'http:g','http://a/b/c/g') | ||
| self.checkJoin(SIMPLE_BASE, 'http:','http://a/b/c/d') |
There was a problem hiding this comment.
Duplicated below.
Comment on lines
-551
to
-552
| self.checkJoin(SIMPLE_BASE, 'http:','http://a/b/c/d') | ||
| self.checkJoin(SIMPLE_BASE, 'http:?y','http://a/b/c/d?y') |
There was a problem hiding this comment.
Duplicated in new tests below.
| v = SplitResult(scheme or '', netloc or '', url, query or '', fragment or '') | ||
| return _coerce_result(v) | ||
|
|
||
| def _urlsplit(url, scheme=None, allow_fragments=True): |
Comment on lines
+592
to
+593
| if fragment is None: | ||
| fragment = bfragment |
There was a problem hiding this comment.
This contradicts RFC 3986 according to which the target fragment is always set to the relative reference fragment. But this means that an empty string (which has no defined fragment) should remove fragment. I think this is an error in RFC 3986.
|
|
||
| if scheme is None: | ||
| scheme = bscheme | ||
| if scheme != bscheme or scheme not in uses_relative: | ||
| return _coerce_result(url) | ||
| if scheme in uses_netloc: | ||
| if netloc: |
There was a problem hiding this comment.
According to RFC 3986, this should be netlog is not None (undefined). But this gives results too different from Ruby and Go (they may be wrong) and from the current Python code. I leave this for a separate issue.
|
@orsenthil, please take a look at this PR. |
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.
Also refactor the code and get rid of double _coerce_args() and _coerce_result() calls in urljoin(), urldefrag(), urlparse() and urlunparse().