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 due to Python change in urllib #2377

Closed
kamil-certat opened this issue Jun 14, 2023 · 0 comments · Fixed by #2378
Closed

Regression due to Python change in urllib #2377

kamil-certat opened this issue Jun 14, 2023 · 0 comments · Fixed by #2378
Milestone

Comments

@kamil-certat
Copy link
Contributor

Python security update changed how the urlib.parse treats the leading spaces in URLs. They are currently stripped, and the URL is successfully processed: https://github.com/python/cpython/pull/102508/files#

This breaks our test cases if running on the system with patched Python, e.g. https://github.com/certtools/intelmq/actions/runs/5266316737/jobs/9520090889

Broken test's assert:

self.assertFalse(harmonization.URL.is_valid(' http://example.com'))

In addition, the official documentation now clearly says, that parse is not intended to be used as validation.

I think that we should keep the previous validation by rejecting URLs starting from empty chars as it may be important for some services.

kamil-certat added a commit to kamil-certat/intelmq that referenced this issue Jun 14, 2023
The gh-102153 change in CPython modified how
urllib.pare handles URLs with the leading
spaces. To ensure previous behaviour, additional
check was added.

Closes: certtools#2377
@sebix sebix added this to the 3.2.0 milestone Jun 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants