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: Relax check to ignore semicolons in URLs #116

Merged
merged 1 commit into from
Jan 28, 2025

Conversation

max-moser
Copy link
Contributor

Closes #115

@tmorrell
Copy link
Contributor

Yup, another piece of evidence this change is needed. Could you add in the test from https://github.com/inveniosoftware/idutils/pull/92/files, and then we can get this merged finally.

@max-moser
Copy link
Contributor Author

Actually, your PR has more goodies than this one. Closing in favor of yours.

@max-moser max-moser closed this Jan 28, 2025
@max-moser max-moser reopened this Jan 28, 2025
@max-moser
Copy link
Contributor Author

@tmorrell I tried rebasing your PR on master, but was denied permissions to push to the branch.
As such, I'll update & merge this PR instead; I interpret your PR being basically the same as an implicit approval :)

* according to RFC 1738 section 2.2, semicolons are reserved but valid
  characters in URIs
* `urllib.parse.urlparse()` interprets semicolons as `params`
* thus, the check for no params in `is_url()` returns `False` for some
  valid URLs
@max-moser max-moser merged commit ffd5a6b into inveniosoftware:master Jan 28, 2025
2 checks passed
@max-moser max-moser deleted the mm/is_url branch January 28, 2025 13:02
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 this pull request may close these issues.

URLs with semicolons are reported as invalid
2 participants