yarl icon indicating copy to clipboard operation
yarl copied to clipboard

Keep absolute paths absolute when normalising.

Open mjpieters opened this issue 2 years ago • 1 comments

What do these changes do?

Keep absolute paths absolute when normalising.

This is is consistent with RFC 3986, and just about all other URL libraries in general use.

The added tests are based on two URL tests from the web-platform-tests project (see test 1 and test 2), a reference also used by other projects such as rust-url.

Are there changes in behavior for the user?

Yes, previously you'd get a ValueError when .. segments reach beyond the root path:

>>> URL('https://example.com/alice/../../eve')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/var/www/openid/mj/_h/_venv/lib/python3.9/site-packages/yarl/_url.py", line 183, in __new__
    cls._validate_authority_uri_abs_path(host=host, path=path)
  File "/var/www/openid/mj/_h/_venv/lib/python3.9/site-packages/yarl/_url.py", line 679, in _validate_authority_uri_abs_path
    raise ValueError(
ValueError: Path in a URL with authority should start with a slash ('/') if set

With this change, no exception is throw, instead .. segments are simply dropped if they'd remove the / root element:

>>> URL('https://example.com/alice/../../eve')
URL('https://example.com/eve')

This is what just about all other URL libraries do.

Note: there are no changes to the documentation because the old behaviour was never documented.

Related issue number

Resolves #536

Checklist

  • [X] I think the code is well written
  • [X] Unit tests for the changes exist
  • [ ] Documentation reflects the changes
  • [X] Add a new news fragment into the CHANGES folder
    • name it <issue_id>.<type> (e.g. 588.bugfix)
    • if you don't have an issue_id change it to the pr id after creating the PR
    • ensure type is one of the following:
      • .feature: Signifying a new feature.
      • .bugfix: Signifying a bug fix.
      • .doc: Signifying a documentation improvement.
      • .removal: Signifying a deprecation or removal of public API.
      • .misc: A ticket has been closed, but it is not of interest to users.
    • Make sure to use full sentences with correct case and punctuation, for example: Fix issue with non-ascii contents in doctest text files.

mjpieters avatar Jun 14 '22 20:06 mjpieters

Codecov Report

Merging #741 (8b6628a) into master (8080779) will increase coverage by 0.00%. The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #741   +/-   ##
=======================================
  Coverage   99.73%   99.73%           
=======================================
  Files           4        4           
  Lines         755      757    +2     
  Branches      211      213    +2     
=======================================
+ Hits          753      755    +2     
  Misses          2        2           
Flag Coverage Δ
unit 99.60% <100.00%> (+<0.01%) :arrow_up:

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

Impacted Files Coverage Δ
yarl/_url.py 99.66% <100.00%> (+<0.01%) :arrow_up:

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

codecov[bot] avatar Jun 14 '22 20:06 codecov[bot]