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 URL validation for unicode support #1182

Merged

Conversation

Projects
None yet
2 participants
@noirbizarre
Copy link
Member

commented Oct 2, 2017

This PR improves URL fields validation (both model and form level) by supporting more use cases:

  • new tlds
  • unicode domains
  • unicode paths
  • allows to accept only public URLs

@noirbizarre noirbizarre added this to the 1.2.0 milestone Oct 2, 2017

@noirbizarre noirbizarre requested a review from opendatateam/etalab Oct 2, 2017

@noirbizarre noirbizarre force-pushed the noirbizarre:improve-url-validation branch from 9eaffd9 to ac73479 Oct 2, 2017

@abulte
Copy link
Member

left a comment

On a general note: where is the public=True feature used?

@@ -11,6 +11,7 @@
- Switch to [Crowdin](https://crowdin.com) to manage translations [#1171](https://github.com/opendatateam/udata/pull/1171)
- **BREAKING** Switch to `Flask-Security`. `Flask-Security-Fork` should be uninstalled before installing the new requirements [#958](https://github.com/opendatateam/udata/pull/958)
- Added a `udata info` command for diagnostic purpose [#1179](https://github.com/opendatateam/udata/pull/1179)
- Improve URLs validation (support new tlds, unciode URLs...) [#1182](https://github.com/opendatateam/udata/pull/1182)

This comment has been minimized.

Copy link
@abulte

abulte Oct 2, 2017

Member

s/unciode/unicode/

Public URL can be enforced with `public=True`
URL_REGEX has been extracted and adapted from:
https://github.com/kvesteri/validators/blob/master/validators/url.py

This comment has been minimized.

Copy link
@abulte

abulte Oct 2, 2017

Member

What is different versus the validators.url implementation? Public vs private?

This comment has been minimized.

Copy link
@noirbizarre

noirbizarre Oct 2, 2017

Author Member

Scheme validation is handled separately (MongoEngine implementation allows to list allowed schemes).
Raw strings are used instead of unicode

This comment has been minimized.

Copy link
@noirbizarre

noirbizarre Oct 2, 2017

Author Member

I added it in the comment

Fake, FakeForm = self.factory()

fake = Fake()
url = url = 'https://www.somewhère.com/with/accënts/'

This comment has been minimized.

Copy link
@abulte

abulte Oct 2, 2017

Member

url =

@noirbizarre

This comment has been minimized.

Copy link
Member Author

commented Oct 2, 2017

I didn't activated the public=True on any model yet because I think it requires a little debate.
I think this is interesting to have public=True on reuses and resources URL and on org and user websites.
But, this is in the case of a public portal.

@noirbizarre noirbizarre force-pushed the noirbizarre:improve-url-validation branch from ac73479 to a541857 Oct 2, 2017

@abulte

This comment has been minimized.

Copy link
Member

commented Oct 2, 2017

@noirbizarre Yes I agree, it's debatable because some users may want to link to internal resources.

@abulte

This comment has been minimized.

Copy link
Member

commented Oct 2, 2017

@noirbizarre Maybe a config flag? ALLOW_PRIVATE_RESOURCES?

@noirbizarre

This comment has been minimized.

Copy link
Member Author

commented Oct 2, 2017

Yep, maybe in a separate PR. This one fix unicode and lands the fact that we can filter out private URL and another PR will provide the ability to configure this

@abulte

abulte approved these changes Oct 2, 2017

@noirbizarre noirbizarre force-pushed the noirbizarre:improve-url-validation branch from a541857 to fccf66c Oct 2, 2017

@noirbizarre noirbizarre force-pushed the noirbizarre:improve-url-validation branch from fccf66c to 6136be9 Oct 2, 2017

@noirbizarre noirbizarre merged commit 93beba5 into opendatateam:master Oct 2, 2017

3 checks passed

ci/circleci: assets Your tests passed on CircleCI!
Details
ci/circleci: dist Your tests passed on CircleCI!
Details
ci/circleci: python Your tests passed on CircleCI!
Details

@noirbizarre noirbizarre removed the in progress label Oct 2, 2017

@noirbizarre noirbizarre deleted the noirbizarre:improve-url-validation branch Oct 2, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.