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

Explicit port 0 replaced with default port #1408

Open
5 tasks done
gmacon opened this issue Nov 7, 2024 · 1 comment
Open
5 tasks done

Explicit port 0 replaced with default port #1408

gmacon opened this issue Nov 7, 2024 · 1 comment

Comments

@gmacon
Copy link
Contributor

gmacon commented Nov 7, 2024

Please confirm the following

  • I understand this is open source software provided for free and that I might not receive a timely response.
  • I am positive I am NOT reporting a (potential) security
    vulnerability, to the best of my knowledge. (These must be shared by
    submitting this report form instead, if
    any hesitation exists.)
  • I am willing to submit a pull request with reporoducers as xfailing test cases or even entire fix. (Assign this issue to me.)

Describe the bug

If a URL has an explicit port of 0 and a scheme with a default port (such as http), then the port and explicit_port attributes disagree on what the port is.

To Reproduce

import yarl
u = yarl.URL("http://a.co:0")
assert u.port == u.explicit_port

Expected behavior

If explicit_port is not None, then port should be the same as explicit_port.

Logs/tracebacks

Python 3.10.12 (main, Sep 11 2024, 15:47:36) [GCC 11.4.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import yarl
>>> u = yarl.URL("http://a.co:0")
>>> assert u.port == u.explicit_port
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AssertionError
>>> u.port
80
>>> u.explicit_port
0

Python Version

$ python --version
Python 3.10.12

multidict Version

$ python -m pip show multidict
Name: multidict
Version: 6.1.0
Location: $PWD/.venv/lib/python3.10/site-packages
Requires: typing-extensions
Required-by: yarl

propcache Version

$ python -m pip show propcache
Name: propcache
Version: 0.2.0
Location: $PWD/.venv/lib/python3.10/site-packages
Requires:
Required-by: yarl

yarl Version

$ python -m pip show yarl
Name: yarl
Version: 1.17.2.dev0
Location: $PWD/.venv/lib/python3.10/site-packages
Editable project location: $PWD
Requires: idna, multidict, propcache
Required-by:
$ git describe
v1.17.1-1-gdfc41db

OS

Ubuntu 22.04

Additional context

I discovered this because Hypothesis can generate such a URL from its hypothesis.provisional.urls strategy. I think yarl's implementation of port (as self.explicit_port or self._default_port) is wrong because the URL grammar permits the port to be 0 (the production is just port = *DIGIT), but I can't think of a situation where a URL with port 0 would be useful.

@gmacon gmacon added the bug label Nov 7, 2024
@webknjaz
Copy link
Member

webknjaz commented Nov 7, 2024

Thanks for the report! Outside of URLs, port 0 may have different semantics assigned. For example, it can be a so-called "ephemeral port" that is passed into the socket bind syscalls and then the kernel assigns an unoccupied port that nobody claimed, at which point it turns into a concrete port with a non-zero number.

I can imagine that someone may want to represent a pre-binding URL for logging purposes. Or for when an interactive app doesn't bind to a port until a user clicks the button.

I can also imagine 0 having different semantics for non-HTTP URLs (yarl is agnostic, after all).

So I'd say — send us an xfailing test and perhaps some Hypothesis tests, for starters.

No rush, though — we're leading an aiohttp sprint day on Saturday and this might be a good issue for the participants to look into.

gmacon pushed a commit to gmacon/yarl that referenced this issue Nov 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

No branches or pull requests

2 participants