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

fix #664: validate.URL: allow hostnames with no dots #665

Closed

Conversation

sduthil
Copy link
Contributor

@sduthil sduthil commented Jul 31, 2017

reason: a hostname with no dots is valid, and useful in some cases,
e.g. a host in the same subdomain than the client, a shorter alias to
a longer hostname, a local network without TLD

reason: a hostname with no dots is valid, and useful in some cases,
e.g. a host in the same subdomain than the client, a shorter alias to
a longer hostname, a local network without TLD
@sduthil sduthil changed the base branch from dev to 2.x-line July 31, 2017 17:30
@sloria
Copy link
Member

sloria commented Aug 6, 2017

Thanks for the PR @sduthil .

Both Django's and WTForms' URL validators disallow URLs with no periods, which makes me hesitant to merge this. Your use cases sound valid, though, so I'll keep this open for discussion for the time being.

@sduthil
Copy link
Contributor Author

sduthil commented Aug 7, 2017

From what I read, WTForms allows dotless URL, with a specific require_tld=False option (see the interface and the implementation)

About Django, there is a opened feature request and another quite old ticket closed.

There's also a recent discussion about email validation, where some comments are relevant to URLs, some aren't.

@sloria
Copy link
Member

sloria commented Aug 8, 2017

Thanks @sduthil for doing that research.

It appears from both those Django tickets that there is still an open discussion on whether to make the validator more permissive:

https://code.djangoproject.com/ticket/25418#comment:2

I believe the idea of URLValidator is to recognize URLs that usually work without some special DNS setup. I feel like this proposal has come up before and been rejected. If so, we should document the restriction (and how to lift it in your own validation) to try to prevent it from being proposed again and again.

https://code.djangoproject.com/ticket/25418#comment:8

Tim, the fact that you've had a duplicate from me suggests that the regex is wrong. ​https://tools.ietf.org/html/rfc3986#section-3.2.2 does not specify the need for either a domain or a tld.
That's the technicality, the practicality is that most intranet urls are left in short form, without a domain or tld. These are not a "special" DNS setup, and I can't see a reason to simplify the regex.

https://code.djangoproject.com/ticket/9202#comment:1

I'm not sure we want to be so permissive for URLField when a RegexField with a custom regex does the same thing. The point of the URLField is to validate "common" URLs; for the vast majority of users http://django/ is a typo.

https://code.djangoproject.com/ticket/9202#comment:4

So this regexp, while technically correct, isn't appropriate to validate the contents of URLField; it's too permissive.

So I'm still hesitant to merge this. I would be more likely to merge something along the lines of WTForms' approach, i.e. strict validation by default, with an option to be more permissive.

@sduthil
Copy link
Contributor Author

sduthil commented Aug 8, 2017

I agree that the WTForms approach is better, and backward compatible. I'll open another PR when I have some time.

Thanks for the discussion, I learned a few things in the process :)

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

Successfully merging this pull request may close these issues.

None yet

2 participants