Skip to content

Normalize URL string ports per RFC 3986 section 3.2.3#1033

Merged
webknjaz merged 19 commits intoaio-libs:masterfrom
commonism:rfc3986-port-normalization
Jul 17, 2024
Merged

Normalize URL string ports per RFC 3986 section 3.2.3#1033
webknjaz merged 19 commits intoaio-libs:masterfrom
commonism:rfc3986-port-normalization

Conversation

@commonism
Copy link
Copy Markdown
Contributor

@commonism commonism commented Jul 3, 2024 •
edited
Loading

What do these changes do?

The changes add a missing step in rfc3986 normalization - strip default port from URL str representation.

Are there changes in behavior for the user?

Yes

Related issue number

#1023 (comment)

In the future, with another PR, I think the only normalisation step currently missing is https://datatracker.ietf.org/doc/html/rfc3986.html#section-6.2.3
Which would include (for HTTP/HTTPS schemes) setting the path to / when empty and removing the port number when matching the default 80/443.

Checklist

  • I think the code is well written
  • Unit tests for the changes exist
  • Documentation reflects the changes

@commonism commonism force-pushed the rfc3986-port-normalization branch from f60c4db to 11a7499 Compare July 5, 2024 06:16
@codecov
Copy link
Copy Markdown

codecov Bot commented Jul 5, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 85.71429% with 7 lines in your changes missing coverage. Please review.

Project coverage is 62.34%. Comparing base (4bededd) to head (4e255da).
Report is 234 commits behind head on master.

Files with missing lines Patch % Lines
yarl/_url.py 80.76% 5 Missing ⚠️
tests/test_url.py 80.00% 1 Missing ⚠️
tests/test_url_update_netloc.py 94.44% 1 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #1033      +/-   ##
==========================================
+ Coverage   62.06%   62.34%   +0.28%     
==========================================
  Files          38       38              
  Lines        6550     6616      +66     
  Branches      352      357       +5     
==========================================
+ Hits         4065     4125      +60     
- Misses       2457     2463       +6     
  Partials       28       28
Flag Coverage Δ
CI-GHA 62.31% <85.71%> (+0.28%) ⬆️
MyPy 25.56% <67.34%> (+0.54%) ⬆️
OS-Linux 99.25% <100.00%> (+<0.01%) ⬆️
OS-Windows 99.58% <100.00%> (+<0.01%) ⬆️
OS-macOS 99.01% <100.00%> (+0.01%) ⬆️
Py-3.10.11 98.90% <95.00%> (-0.02%) ⬇️
Py-3.10.14 99.10% <95.00%> (-0.02%) ⬇️
Py-3.11.9 99.10% <95.00%> (-0.02%) ⬇️
Py-3.12.4 99.10% <95.00%> (-0.02%) ⬇️
Py-3.8.10 98.87% <100.00%> (+0.01%) ⬆️
Py-3.8.18 99.07% <100.00%> (+0.01%) ⬆️
Py-3.9.13 98.87% <100.00%> (+0.01%) ⬆️
Py-3.9.19 99.07% <100.00%> (+0.01%) ⬆️
Py-pypy7.3.11 99.39% <100.00%> (+<0.01%) ⬆️
Py-pypy7.3.16 99.42% <100.00%> (+<0.01%) ⬆️
VM-macos-latest 99.01% <100.00%> (+0.01%) ⬆️
VM-ubuntu-latest 99.25% <100.00%> (+<0.01%) ⬆️
VM-windows-latest 99.58% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@commonism commonism changed the title WIP RFC3986 port normalization RFC3986 port normalization Jul 5, 2024
@commonism
Copy link
Copy Markdown
Contributor Author

commonism commented Jul 6, 2024

@webknjaz @Dreamsorcerer - please review
In addition to getting more RFC3986 compliant by normalizing ports, this is part of a series (1/3) to implement URL.join without using urljoin.
Currently URL.join is implemented using using urllib.parse.urljoin, urljoin strips. empty segments in violation of rfc3985

@webknjaz
Copy link
Copy Markdown
Member

webknjaz commented Jul 6, 2024

Remind me on Monday evening if I forget, not earlier, plz.

Dreamsorcerer
Dreamsorcerer reviewed Jul 6, 2024
View reviewed changes
Comment thread yarl/_url.py
Dreamsorcerer
Dreamsorcerer reviewed Jul 6, 2024
View reviewed changes
Comment thread yarl/_url.py Outdated
Dreamsorcerer
Dreamsorcerer reviewed Jul 6, 2024
View reviewed changes
Comment thread yarl/_url.py
commonism and others added 4 commits July 7, 2024 08:47
@commonism @Dreamsorcerer
Update yarl/_url.py
d477fe1 
Co-authored-by: Sam Bull <aa6bs0@sambull.org>
@commonism @Dreamsorcerer
Update yarl/_url.py
4c823fc 
Co-authored-by: Sam Bull <aa6bs0@sambull.org>
@commonism
segments - later
d417f7e
@commonism
URL.port - use URL._get_default_port
92744f5
@commonism
Copy link
Copy Markdown
Contributor Author

commonism commented Jul 16, 2024

Remind me on Monday evening if I forget, not earlier, plz.

Remind

webknjaz
webknjaz reviewed Jul 16, 2024
View reviewed changes
Comment thread yarl/_url.py Outdated
webknjaz
webknjaz reviewed Jul 16, 2024
View reviewed changes
Comment thread yarl/_url.py Outdated
webknjaz
webknjaz reviewed Jul 16, 2024
View reviewed changes
Comment thread yarl/_url.py Outdated
webknjaz
webknjaz reviewed Jul 16, 2024
View reviewed changes
Comment thread yarl/_url.py
webknjaz
webknjaz reviewed Jul 16, 2024
View reviewed changes
Comment thread tests/test_url_update_netloc.py Outdated
webknjaz
webknjaz reviewed Jul 16, 2024
View reviewed changes
Comment thread CHANGES/1033.bugfix.rst Outdated
commonism and others added 4 commits July 17, 2024 08:36
@commonism @Dreamsorcerer
Update yarl/_url.py
990071d 
Co-authored-by: Sam Bull <aa6bs0@sambull.org>
@commonism @webknjaz
Update yarl/_url.py
d312f42 
Co-authored-by: Sviatoslav Sydorenko (Святослав Сидоренко) <wk.cvs.github@sydorenko.org.ua>
@commonism @webknjaz
Update yarl/_url.py
3ea31ed 
Co-authored-by: Sviatoslav Sydorenko (Святослав Сидоренко) <wk.cvs.github@sydorenko.org.ua>
@commonism @webknjaz
Update CHANGES/1033.bugfix.rst
4f59281 
Co-authored-by: Sviatoslav Sydorenko (Святослав Сидоренко) <wk.cvs.github@sydorenko.org.ua>
webknjaz
webknjaz reviewed Jul 17, 2024
View reviewed changes
Comment thread yarl/_url.py Outdated
commonism and others added 3 commits July 17, 2024 13:25
@commonism @webknjaz
Update yarl/_url.py
d17a51b 
Co-authored-by: Sviatoslav Sydorenko (Святослав Сидоренко) <wk.cvs.github@sydorenko.org.ua>
@commonism
split tests
dd19cec
@commonism
comment for normalization during __str__
ca6b3ee
webknjaz
webknjaz reviewed Jul 17, 2024
View reviewed changes
Comment thread CHANGES/1033.bugfix.rst Outdated
webknjaz
webknjaz reviewed Jul 17, 2024
View reviewed changes
Comment thread tests/test_url.py Outdated
webknjaz
webknjaz reviewed Jul 17, 2024
View reviewed changes
Comment thread tests/test_url_update_netloc.py Outdated
webknjaz
webknjaz reviewed Jul 17, 2024
View reviewed changes
Comment thread tests/test_url_update_netloc.py Outdated
webknjaz
webknjaz reviewed Jul 17, 2024
View reviewed changes
Comment thread tests/test_url_update_netloc.py Outdated
webknjaz
webknjaz reviewed Jul 17, 2024
View reviewed changes
Comment thread yarl/_url.py Outdated
webknjaz and others added 2 commits July 17, 2024 15:38
@webknjaz
Specify types in all newly added functions
73e2861
@commonism @webknjaz
Update CHANGES/1033.bugfix.rst
f344a3e 
Co-authored-by: Sviatoslav Sydorenko (Святослав Сидоренко) <wk.cvs.github@sydorenko.org.ua>
webknjaz
webknjaz reviewed Jul 17, 2024
View reviewed changes
Comment thread CHANGES/1033.bugfix.rst Outdated
@webknjaz webknjaz added bug bot:chronographer:skip This PR does not need to include a change note labels Jul 17, 2024
@webknjaz webknjaz changed the title RFC3986 port normalization Normalize URL string ports per RFC 3986 section 3.2.3 Jul 17, 2024
@webknjaz webknjaz enabled auto-merge (squash) July 17, 2024 17:12
@webknjaz webknjaz merged commit d5c5ab3 into aio-libs:master Jul 17, 2024
@webknjaz webknjaz removed the bot:chronographer:skip This PR does not need to include a change note label Jul 17, 2024
@commonism commonism deleted the rfc3986-port-normalization branch July 18, 2024 06:43
This was referenced Sep 1, 2024
Merged
Merged
@Dreamsorcerer Dreamsorcerer mentioned this pull request Sep 2, 2024
Closed
1 task
@bdraco bdraco mentioned this pull request Sep 9, 2024
Open
@bdraco bdraco mentioned this pull request Sep 26, 2024
Merged
@bdraco bdraco mentioned this pull request Oct 8, 2024
Merged
@purplezimmermann purplezimmermann mentioned this pull request Jul 3, 2025
Open
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Dreamsorcerer Dreamsorcerer Dreamsorcerer approved these changes

@webknjaz webknjaz webknjaz approved these changes

Assignees

No one assigned

Labels

bug

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@commonism @webknjaz @Dreamsorcerer