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

Domain validator allows invalid characters in some cases #366

Closed
hagenrd opened this issue Apr 17, 2024 · 4 comments · Fixed by #367
Closed

Domain validator allows invalid characters in some cases #366

hagenrd opened this issue Apr 17, 2024 · 4 comments · Fixed by #367
Labels
bug Issue: Works not as designed

Comments

@hagenrd
Copy link

hagenrd commented Apr 17, 2024

It appears that, currently, any character is valid for the final character in the gTLD if rfc1034 is True, for example:

>>> domain('example.com?', rfc_1034=True, rfc_2782=False)
True
>>> domain('example.com!', rfc_1034=True, rfc_2782=False)
True

I believe the '.' just needs to be escaped in the pattern string (link):

+ rf"[a-z]{r'.?$' if rfc_1034 else r'$'}",
             ^

Also, it appears question marks are allowed when rfc_2782 is True for domain validation:

>>> from validators import domain
>>> domain('example?.com', rfc_1034=False, rfc_2782=True)
True

This appears to be from the use of '?' after the '_' inside of a character class:

rf"^(?:[a-z0-9{r'_?'if rfc_2782 else ''}]"
                  ^

Presumably, this is to make the '_' optional, but since metacharacters aren't active in character classes (link), this is interpreted as a literal '?' instead.

@yozachar yozachar added the bug Issue: Works not as designed label Apr 18, 2024
@yozachar
Copy link
Collaborator

yozachar commented Apr 18, 2024

Hey @hagenrd thanks! Do you mind reviewing: #367, before it's merged?

@hagenrd
Copy link
Author

hagenrd commented Apr 19, 2024

Thanks for the quick turnaround! Any chance you have some time to provide a patch that includes those changes?

@yozachar
Copy link
Collaborator

@hagenrd
Copy link
Author

hagenrd commented Apr 19, 2024

Sorry, I meant patch in terms of a semantic version, i.e. a patch-release (0.28.1).

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
Development

Successfully merging a pull request may close this issue.

2 participants