Skip to content

bpo-39799: Never return base's fragment from urllib.parse.urljoin #18710

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 6 commits into from
Closed

bpo-39799: Never return base's fragment from urllib.parse.urljoin #18710

wants to merge 6 commits into from

Conversation

openandclose
Copy link

@openandclose openandclose commented Feb 29, 2020

According to RFC3986 5.2.2.,
target fragment is always reference fragment.

https://bugs.python.org/issue39799

https://bugs.python.org/issue39799

According to RFC3986 5.2.2.,
target fragment is always reference fragment.

https://bugs.python.org/issue39799
@codecov
Copy link

codecov bot commented Feb 29, 2020

Codecov Report

Merging #18710 into master will increase coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #18710     +/-   ##
=========================================
  Coverage   82.13%   82.13%             
=========================================
  Files        1956     1955      -1     
  Lines      589974   584599   -5375     
  Branches    44484    44483      -1     
=========================================
- Hits       484572   480160   -4412     
+ Misses      95750    94791    -959     
+ Partials     9652     9648      -4     
Impacted Files Coverage Δ
Lib/distutils/tests/test_bdist_rpm.py 30.00% <0.00%> (-65.00%) ⬇️
Lib/distutils/command/bdist_rpm.py 7.63% <0.00%> (-56.88%) ⬇️
Modules/_decimal/libmpdec/umodarith.h 80.76% <0.00%> (-19.24%) ⬇️
Lib/test/test_urllib2net.py 76.92% <0.00%> (-13.85%) ⬇️
Lib/test/test_smtpnet.py 78.57% <0.00%> (-7.15%) ⬇️
Lib/ftplib.py 63.85% <0.00%> (-6.06%) ⬇️
Lib/test/test_ftplib.py 87.11% <0.00%> (-4.72%) ⬇️
Tools/scripts/db2pickle.py 17.82% <0.00%> (-3.97%) ⬇️
Tools/scripts/pickle2db.py 16.98% <0.00%> (-3.78%) ⬇️
Lib/test/test_socket.py 71.94% <0.00%> (-3.77%) ⬇️
... and 334 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1f0cd3c...9cfe7c0. Read the comment docs.

@@ -449,6 +449,9 @@ def test_urljoins(self):
# issue 23703: don't duplicate filename
self.checkJoin('a', 'b', 'b')

# issue 39799: no base fragment (not compatible with RFC1808)
self.checkJoin('http://a/b#f', '', 'http://a/b')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you move this test to be under test_RFC3986?

@@ -449,6 +449,9 @@ def test_urljoins(self):
# issue 23703: don't duplicate filename
self.checkJoin('a', 'b', 'b')

# issue 39799: no base fragment (not compatible with RFC1808)
self.checkJoin('http://a/b#f', '', 'http://a/b')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a test for self.checkJoin('http://a/b/#f', 'g', 'http://a/b/g')?

@@ -0,0 +1,3 @@
Fix ``urllib.parse.urljoin``
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you mention that this is in order to conform with RFC 3986?

@openandclose
Copy link
Author

@epicfaace I edited accordingly.

@serhiy-storchaka serhiy-storchaka self-requested a review November 11, 2024 12:04
@openandclose openandclose closed this by deleting the head repository Feb 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants