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

wtforms.validators.URL does not allow query parameters in the URL #523

Closed
laurihy opened this issue Nov 5, 2019 · 1 comment · Fixed by #524
Closed

wtforms.validators.URL does not allow query parameters in the URL #523

laurihy opened this issue Nov 5, 2019 · 1 comment · Fixed by #524

Comments

@laurihy
Copy link
Contributor

@laurihy laurihy commented Nov 5, 2019

Using wtforms.validators.URL to validate URL with query parameters incorrectly returns False.

import wtforms
 
class URLForm(wtforms.Form):
     url = wtforms.StringField('MyLink', validators=[wtforms.validators.URL()])
 
URLForm(url='https://example.com').validate()
# -> True

URLForm(url='https://example.com?this=fails').validate()
# -> False, but this should be True as it's a valid URL

Seems like this is because the regex used in URL validation incorrectly includes query parameters to the hostname, and then HostnameValidation fails.

https://github.com/wtforms/wtforms/blob/master/src/wtforms/validators.py#L498

import re
from wtforms.validators import HostnameValidation
url_regex = re.compile(r"^[a-z]+://(?P<host>[^/:]+)(?P<port>:[0-9]+)?(?P<path>\/.*)?$")
match  = url_regex.match('https://example.com?query=params')
print(match.group('host'))
# -> example.com?query=params
HostnameValidation(match.group('host'))
# -> False

Instead, the regex should separate hostname and query parameters, and only use the actual hostname for HostnameValidation.

I'll cook up a PR to fix this later this week, unless someone is faster than that :)

@laurihy
Copy link
Contributor Author

@laurihy laurihy commented Nov 5, 2019

#524 should fix this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

1 participant