bug: /#/ in URLs is failing validation #288

Activity

1. nandgator commented on Aug 17, 2023

   

   nandgator

   on Aug 17, 2023

   Collaborator

   More actions

   The /#/ in there is probably the thing making it match as invalid, sadly it is a common side-effect of some JS UI libraries.

   Hi @adrienthiery, indeed, /#/ was causing trouble, but this was not an intentional breaking change, unlike the case in #285 (comment). It is an edge case which was neither present in the tests before nor after v0.20.0. But since the validation algorithm changed, the edge case popped up.

   Currently src/validators/url.py has:

   validators/src/validators/url.py

   Lines 118 to 119 in ee088fa

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

   When the URL https://exchange.jetswap.finance/#/swap is passed, fragment = /swap, which makes optional_segments = false since / is present in the fragment, which is (an) unusual (side-effect) as you suggest.

   ---

   Here's the patch.

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

   This will handle all the URLs with /#/some-value fragment in them, but I think there will be more edge cases, only time will tell. PRs are welcome!

   ❤️React with ❤️1Reacted by Adrien Thiery
2. nandgator

   changed the title [-]Breaking change between 0.20 and latest version (URLs including `/#/`)[/-] [+]bug: `/#/` in URLs is failing validation[/+] on Aug 17, 2023
3. nandgator

   added

   bugIssue: Works not as designedIssue: Works not as designed

   on Aug 17, 2023
4. nandgator

   closed this as completedin #289on Aug 18, 2023
5. nandgator

   assigned

   nandgator

   and

   adrienthiery

   and unassigned

   nandgator

   on Sep 2, 2023
6. cfi-gb commented on Sep 6, 2023

   

   cfi-gb

   on Sep 6, 2023

   More actions

   but I think there will be more edge cases, only time will tell.

   It seems a few additional ones have been found so far and #296 has been raised as a follow-up with additional different examples.