Skip to content
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

Regression: Equality of joined URLs fails to be compared equal with a readymade URL object. #854

Closed
1 task done
gmichaeljaison opened this issue Apr 21, 2023 · 11 comments · Fixed by #857
Closed
1 task done
Labels

Comments

@gmichaeljaison
Copy link

Describe the bug

Equality of joined URLs fails to be compared equal with a readymade URL object.

The following code returns False in the latest release version 1.9.1. It used to return True in previous version.

(yarl.URL("https://hello.com") / "world") == yarl.URL("https://hello.com/world")

To Reproduce

(yarl.URL("https://hello.com") / "world") == yarl.URL("https://hello.com/world")

Expected behavior

Should be equal

Logs/tracebacks

(yarl.URL("https://hello.com") / "world") == yarl.URL("https://hello.com/world")

False

Python Version

$ python --version
3.8+

multidict Version

$ python -m pip show multidict
Name: multidict
Version: 6.0.2
Required-by: httpie, yarl

yarl Version

$ python -m pip show yarl
1.9.1

OS

macOS

Additional context

No response

Code of Conduct

  • I agree to follow the aio-libs Code of Conduct
@enku
Copy link

enku commented Apr 22, 2023

A git bisect reveals the regression was caused by 2dac93f.

enku added a commit to enku/gentoo-build-publisher that referenced this issue Apr 22, 2023
gbp is affected by a bug in yarl-1.9.1.

aio-libs/yarl#854
@dalazx
Copy link

dalazx commented Apr 24, 2023

I am seeing a significant regression as well:

>>> from yarl import __version__, URL
>>> __version__
'1.9.1'
>>> (URL("http://0.0.0.0:1234/this/is/a/path")).raw_path
'/this/is/a/path'
>>> (URL("http://0.0.0.0:1234") / "this/is/a/path").raw_path
'this/is/a/path'

the latter example is missing the leading slash.

@achimnol
Copy link
Member

achimnol commented Apr 25, 2023

I have encountered the same issue:
image

The test code:

import yarl
from pathlib import Path
import aiohttp
import asyncio
import os

async def main(): -> None
    sock_path = Path("/var/run/docker.sock")
    decoded_path = os.fsdecode(sock_path)
    docker_host = yarl.URL("http://localhost")
    connector = aiohttp.UnixConnector(decoded_path)
    async with aiohttp.ClientSession(connector=connector) as sess:
        url = docker_host / "info"
        print(repr(url))
        print(str(url))
        print(bytes(url))
        print(f"{url.host=}", f"{url.path=}", f"{url.raw_path=}")
        async with sess.get(url) as resp:
            data = await resp.text()
            print(data)

if __name__ == "__main__":
    asyncio.run(main())

@achimnol
Copy link
Member

achimnol commented Apr 25, 2023

If I try to mangle path like base_url / "/info", it says ValueError: Appending path '/info' starting from slash is forbidden.
I think the path joining rule should applied slightly differently when it is done via the '/' (truediv) operator, at least.

@selcukcihan
Copy link

I'd like to report an issue we are experiencing. We have yarl dependency through aiohttp and with version 1.9.1 our tests started failing. The problem I'm seeing is with a change in behaviour of update_query method. In the new version, update_query(None) removes the query string completely.

Version 1.8.2

Python 3.9.16 (main, Mar 15 2023, 15:55:54) 
[Clang 14.0.0 (clang-1400.0.29.202)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import yarl
>>> yarl.__version__
'1.8.2'
>>> url = yarl.URL("http://127.0.0.1:3177/?foo=bar")
>>> url.update_query(None) == url
True

Version 1.9.1

Python 3.9.16 (main, Mar 15 2023, 15:55:54) 
[Clang 14.0.0 (clang-1400.0.29.202)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import yarl
>>> yarl.__version__
'1.9.1'
>>> url = yarl.URL("http://127.0.0.1:3177/?foo=bar")
>>> url.update_query(None) == url
False

@webknjaz
Copy link
Member

Hi folks, thanks for reaching out! It would be useful if someone could submit a PR with a failing test for this per https://pganssle-talks.github.io/xfail-lightning/.

I'm at the PyCon Sprints now and might have time to look into this.

@mjpieters is this related to your changes?

@mjpieters
Copy link
Contributor

@mjpieters is this related to your changes?

Looks like it as per git bisect, I'll prioritise this next. Hopefully today.

@mjpieters
Copy link
Contributor

mjpieters commented Apr 25, 2023

In the new version, update_query(None) removes the query string completely.

That was always the intended, documented behaviour, it was a bug that it didn't actually do this before. Use an empty dictionary if you don't want the query to be reset. See #723

@mjpieters
Copy link
Contributor

If I try to mangle path like base_url / "/info", it says ValueError: Appending path '/info' starting from slash is forbidden. I think the path joining rule should applied slightly differently when it is done via the '/' (truediv) operator, at least.

That was already the behaviour before and not a regression; it was an explicit choice to disallow this.

@mjpieters
Copy link
Contributor

I'm cutting release 1.9.2 to get this out asap.

@dalazx
Copy link

dalazx commented Apr 25, 2023

thanks @mjpieters!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants