Skip to content

url raising validation error on a valid url #299

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

Closed
kooshkya opened this issue Sep 18, 2023 · 7 comments · Fixed by #297 or #305
Closed

url raising validation error on a valid url #299

kooshkya opened this issue Sep 18, 2023 · 7 comments · Fixed by #297 or #305
Labels
bug Issue: Works not as designed

Comments

@danherbriley
Copy link
Contributor

Hi!

This was brought up in Issue #296 and fixed in PR #297. I assume it should be patched when a new release comes out. Until then I recommend using version 0.20.0.

@yozachar
Copy link
Collaborator

This has been fixed in the latest commit (960b48b). Test it out:

$ mkdir test-validators
$ cd test-validators
$ python -m venv .venv
$ . ./.venv/bin/activate
$ pip install pip install --upgrade --force-reinstall git+https://github.com/python-validators/validators.git@$(git ls-remote https://github.com/python-validators/validators.git | head -1 | awk '{print $1;}')
Collecting git+https://github.com/python-validators/validators.git@960b48b8a548fa5b104f1999ef0c18923a232c52
  Cloning https://github.com/python-validators/validators.git (to revision 960b48b8a548fa5b104f1999ef0c18923a232c52) to /tmp/pip-req-build-cz1oh581
  Running command git clone --filter=blob:none --quiet https://github.com/python-validators/validators.git /tmp/pip-req-build-cz1oh581
  Running command git rev-parse -q --verify 'sha^960b48b8a548fa5b104f1999ef0c18923a232c52'
  Running command git fetch -q https://github.com/python-validators/validators.git 960b48b8a548fa5b104f1999ef0c18923a232c52
  Resolved https://github.com/python-validators/validators.git to commit 960b48b8a548fa5b104f1999ef0c18923a232c52
  Installing build dependencies ... done
  Getting requirements to build wheel ... done
  Installing backend dependencies ... done
  Preparing metadata (pyproject.toml) ... done
Building wheels for collected packages: validators
  Building wheel for validators (pyproject.toml) ... done
  Created wheel for validators: filename=validators-0.22.0-py3-none-any.whl size=26226 sha256=029ae863c44edb8f18750ba9dccd272a126c687e7a9be406c59eecdba9addf16
  Stored in directory: /home/us-er/.cache/pip/wheels/91/80/11/5d58427b2e4261aa5f50c8c8d358bc865a220cdd5dcd853383
Successfully built validators
Installing collected packages: validators
  Attempting uninstall: validators
    Found existing installation: validators 0.22.0
    Uninstalling validators-0.22.0:
      Successfully uninstalled validators-0.22.0
Successfully installed validators-0.22.0
$ python
Python 3.11.5 (main, Sep  2 2023, 14:16:33) [GCC 13.2.1 20230801] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import validators
>>> validators.url("https://iphonechi.com/iphone-11-used/9115-11860-apple-iphone-11-128gb-purple-dual-sim.html#/61-part_number-za_a/130-body_condition-%D8%AE%D8%B1%D8%A7%D8%B4%DB%8C_%D9%86%D8%AF%D8%A7%D8%B1%D8%AF/136-overall_condition-%DA%A9%D8%A7%D8%B1%DA%A9%D8%B1%D8%AF%D9%87/138-overall_lcd_condition-%D9%81%D8%A7%D8%A8%D8%B1%DB%8C%DA%A9/142-lcd_appearance-%D8%AE%D8%B1%D8%A7%D8%B4%DB%8C_%D9%86%D8%AF%D8%A7%D8%B1%D8%AF/146-battery_condition-%D8%A7%D8%B5%D9%84%DB%8C/163-battery_percentage-86/180-impact-%D9%86%D8%AF%D8%A7%D8%B1%D8%AF/191-%D9%84%D9%88%D8%A7%D8%B2%D9%85_%D8%AC%D8%A7%D9%86%D8%A8%DB%8C-%DA%A9%D8%A7%D9%85%D9%84/244-%D9%88%D8%B6%D8%B9%DB%8C%D8%AA_%DA%A9%D8%A7%D8%B1%D8%AA%D9%86_%D8%AF%D8%B3%D8%AA%DA%AF%D8%A7%D9%87-%D8%AF%D8%A7%D8%B1%D8%AF")
True

@yozachar yozachar added bug Issue: Works not as designed addressed labels Sep 18, 2023
@surkum
Copy link

surkum commented Sep 25, 2023

I have the same problem with email verification in a flask app after upgrading dependencies;

With version 0.22.0 all emails got rejected, 0.20.0 works fine

@yozachar
Copy link
Collaborator

yozachar commented Sep 26, 2023

With version 0.22.0 all emails got rejected, 0.20.0 works fine

It is usual, as the latter release made sure that most, if not all the test cases, for email addresses were met.

@surkum, is it possible to provide those emails or samples, for testing before next release?

@darkdragon-001
Copy link
Contributor

darkdragon-001 commented Oct 3, 2023

@yozachar
Copy link
Collaborator

yozachar commented Oct 4, 2023

@darkdragon-001 it fails because of

if fragment:
fragment = fragment.lstrip("/") if fragment.startswith("/") else fragment
optional_segments &= all(char_to_avoid not in fragment for char_to_avoid in ("?",))

But if I put this back (from 9c35f18) instead:

# fragment
r"(?:#\S*)?"

like so

diff --git a/src/validators/url.py b/src/validators/url.py
index 00df3d6..8db36b3 100644
--- a/src/validators/url.py
+++ b/src/validators/url.py
@@ -111,8 +111,7 @@ def _validate_optionals(path: str, query: str, fragment: str, strict_query: bool
     if query and parse_qs(query, strict_parsing=strict_query):
         optional_segments &= True
     if fragment:
-        fragment = fragment.lstrip("/") if fragment.startswith("/") else fragment
-        optional_segments &= all(char_to_avoid not in fragment for char_to_avoid in ("?",))
+        optional_segments &= bool(re.match(r"(?:\S*)?", fragment, re.IGNORECASE))
     return optional_segments
 
 
diff --git a/tests/test_url.py b/tests/test_url.py
index 558d50c..89ae678 100644
--- a/tests/test_url.py
+++ b/tests/test_url.py
@@ -85,6 +85,8 @@ from validators import ValidationError, url
         "http://-.~_!$&'()*+,;=:%40:80%2f::::::@example.com",
         "https://exchange.jetswap.finance/#/swap",
         "https://www.foo.com/bar#/baz/test",
+        "https://matrix.to/#/!BSqRHgvCtIsGittkBG:talk.puri.sm/$1551464398"
+        + "853539kMJNP:matrix.org?via=talk.puri.sm&via=matrix.org&via=disroot.org",
         # when simple_host=True
         # "http://localhost",
         # "http://localhost:8000",

it works. So there's your patch. PR is welcome.

@darkdragon-001
Copy link
Contributor

@yozachar Thanks! I created a PR based on the allowed characters in the fragment part in the standard based off your code pointers!

@yozachar yozachar linked a pull request Oct 5, 2023 that will close this issue
@yozachar yozachar closed this as completed Oct 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue: Works not as designed
Projects
None yet
5 participants