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

URL equality/passing None to URL.build #275

Closed
PromyLOPh opened this issue Dec 25, 2018 · 7 comments
Closed

URL equality/passing None to URL.build #275

PromyLOPh opened this issue Dec 25, 2018 · 7 comments
Labels

Comments

@PromyLOPh
Copy link
Contributor

The equality operator (==, __eq__) does not work properly when passing None to URL.build. Example:

a, b = URL.build(scheme='http', host='0'), URL.build(scheme='http', host='0', fragment=None)
assert a == b # this is False
assert repr (a) == repr (b) # this is True, because "URL('http://0')" == "URL('http://0')"

Passing an empty string works as expected. This is very confusing, because repr produces exactly the same representation.

@aio-libs-bot
Copy link

GitMate.io thinks possibly related issues are #158 (Support encoded parameter in URL.build), #156 (URL.build doesn't url encode credentials), #242 (Handle path argument of URL.build, which doesn't start from /), #33 (Parse URL parameter), and #24 (TypeError in URL.with_query).

@asvetlov
Copy link
Member

That's interesting.
URL.build declares fragment as non-optional str parameter with default to empty string.
Why do you pass None? None for the argument is a grey zone: sometimes it works but there is no any commitment for this value type.

@webknjaz
Copy link
Member

I think None should correspond to missing fragment with no hash # symbol while empty string would be trailing hash in URL. http://some.com/url vs. http://some.com/url#

@asvetlov
Copy link
Member

urllib.parse.urlparse doesn't distinguish http://some.com/url and http://some.com/url#.
It returns an empty string fragment for both cases.
We can return to the question if will implement a custom HTTP parser but now there is nothing to do.

@PromyLOPh
Copy link
Contributor Author

PromyLOPh commented Dec 26, 2018

I’m using hypothesis to generate test data. More specifically a dict which is then passed to yarl using URL.build(**dict). And I was simply assuming None would be mapped to the default value, which is apparently wrong (judging from https://yarl.readthedocs.io/en/latest/api.html#yarl.URL.fragment). This
is fine for me, however the behavior of == and repr() in this case is very confusing and thus None should either be rejected explicitly (TypeError) or silently converted to '' to prevent this situation.

@asvetlov
Copy link
Member

TypeError sounds good to me.
Would you prepare a pull request?

@asvetlov
Copy link
Member

Fixed by #276

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

No branches or pull requests

4 participants