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 in URL.build(…,port="") #883

Closed
1 task done
commonism opened this issue Jun 7, 2023 · 1 comment · Fixed by #959
Closed
1 task done

regression in URL.build(…,port="") #883

commonism opened this issue Jun 7, 2023 · 1 comment · Fixed by #959
Labels

Comments

@commonism
Copy link
Contributor

Describe the bug

Input filtering for with_port #793 changed the logic to render the URL to compare the port to None.
94b8679#r116844287
as it is possible to create a URL with a port="" via URL.build, this results in urls created with an empty port.

To Reproduce

import yarl
u = yarl.URL.build(scheme="http", host="api.com", port="", path="/v1")
assert str(u) == "http://api.com/v1", str(u)

Expected behavior

Filtering port="" in URL.build.
I'd even prefer to allow "" and convert to None to stay backwards compatible.

Logs/tracebacks

AssertionError: http://api.com:/v1

Python Version

$ python --version
Python 3.10.6

multidict Version

$ python -m pip show multidict
Name: multidict
Version: 6.0.2

yarl Version

$ python -m pip show yarl
Name: yarl
Version: 1.9.2

OS

Linux

Additional context

No response

Code of Conduct

  • I agree to follow the aio-libs Code of Conduct
@commonism commonism added the bug label Jun 7, 2023
commonism added a commit to commonism/aiopenapi3 that referenced this issue Jun 7, 2023
commonism added a commit to commonism/aiopenapi3 that referenced this issue Jun 8, 2023
commonism added a commit to commonism/aiopenapi3 that referenced this issue Jun 22, 2023
commonism added a commit to commonism/aiopenapi3 that referenced this issue Jul 13, 2023
commonism added a commit to commonism/aiopenapi3 that referenced this issue Jul 13, 2023
commonism added a commit to commonism/aiopenapi3 that referenced this issue Aug 8, 2023
@webknjaz
Copy link
Member

Hello, please contribute regression tests via PR to move things along.

commonism added a commit to commonism/yarl that referenced this issue Nov 21, 2023
commonism added a commit to commonism/yarl that referenced this issue Nov 21, 2023
webknjaz added a commit that referenced this issue Nov 28, 2023
PR #959
Fixes #883

Co-authored-by: Sviatoslav Sydorenko <webknjaz@redhat.com>
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.

2 participants