Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Fixed #21242 -- Allowed more IANA schemes in URLValidator #2099

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
2 participants
Member

claudep commented Dec 21, 2013

Thanks Sascha Peilicke for the report and initial patch, and
Tim Graham for the review.

@timgraham timgraham and 1 other commented on an outdated diff Dec 27, 2013

django/core/validators.py
@@ -52,14 +52,26 @@ class URLValidator(RegexValidator):
r'(?::\d+)?' # optional port
r'(?:/?|[/?]\S+)$', re.IGNORECASE)
message = _('Enter a valid URL.')
+ schemes = ['http', 'https', 'ftp', 'ftps']
+
+ def __init__(self, schemes=None):
@timgraham

timgraham Dec 27, 2013

Owner

I think we should include *args, **kwargs and pass them to the super __init__

@claudep

claudep Dec 28, 2013

Member

Absolutely about **kwargs, not sure about *args...

@timgraham timgraham commented on an outdated diff Dec 27, 2013

docs/ref/validators.txt
@@ -87,11 +87,23 @@ to, or in lieu of custom ``field.clean()`` methods.
``URLValidator``
----------------
-.. class:: URLValidator()
+.. class:: URLValidator([schemes=None])
@timgraham

timgraham Dec 27, 2013

Owner

this should include the arguments of RegexValidator unless we are specifically excluding them (which would be backwards incompatible so I don't think we want that)

@timgraham timgraham commented on an outdated diff Dec 27, 2013

docs/ref/validators.txt
A :class:`RegexValidator` that ensures a value looks like a URL, and raises
an error code of ``'invalid'`` if it doesn't.
+ .. attribute:: schemes
+
+ URL / URI scheme list to validate against. If not provided, the default
@timgraham

timgraham Dec 27, 2013

Owner

so space before/after /

@timgraham timgraham commented on an outdated diff Dec 27, 2013

docs/ref/validators.txt
A :class:`RegexValidator` that ensures a value looks like a URL, and raises
an error code of ``'invalid'`` if it doesn't.
+ .. attribute:: schemes
+
+ URL / URI scheme list to validate against. If not provided, the default
+ list is ``['http', 'https', 'ftp', 'ftps']``. As a reference, the IANA
+ Web site provides a full list of `valid URI schemes`_.
+
+ .. _valid URI schemes: https://www.iana.org/assignments/uri-schemes/uri-schemes.xhtml
+
+ .. versionchanged:: 1.7
+
+ The ``schemes`` optional attribute was added.
@timgraham

timgraham Dec 27, 2013

Owner

ok as is, but I'd suggest "optional schemes attribute" -- I feel like that's more consistent with wording elsewhere in the docs

@timgraham timgraham commented on an outdated diff Dec 27, 2013

docs/ref/validators.txt
A :class:`RegexValidator` that ensures a value looks like a URL, and raises
an error code of ``'invalid'`` if it doesn't.
+ .. attribute:: schemes
@timgraham

timgraham Dec 27, 2013

Owner

Add to release notes

@timgraham timgraham commented on an outdated diff Dec 28, 2013

docs/releases/1.7.txt
@@ -306,6 +306,13 @@ Cache
thread-safe any more, as :data:`django.core.cache.caches` now yields
different instances per thread.
+Core
@timgraham

timgraham Dec 28, 2013

Owner

It may be too specific in that I'm not sure if they'll be any other changes in the section, but I suggest "Validators" -- I think "core" is overly broad (e.g. signals are also in "core" but have their own section)

@timgraham timgraham commented on an outdated diff Dec 28, 2013

docs/releases/1.7.txt
@@ -306,6 +306,13 @@ Cache
thread-safe any more, as :data:`django.core.cache.caches` now yields
different instances per thread.
+Core
+^^^^
+
+* :class:`~django.core.validators.URLValidator` now accepts an optional
+ ``schemes`` argument which allows to customize the accepted URI schemes
@timgraham

timgraham Dec 28, 2013

Owner

to customization -> customization of

@timgraham timgraham commented on an outdated diff Dec 28, 2013

docs/releases/1.7.txt
@@ -306,6 +306,13 @@ Cache
thread-safe any more, as :data:`django.core.cache.caches` now yields
different instances per thread.
+Core
+^^^^
+
+* :class:`~django.core.validators.URLValidator` now accepts an optional
+ ``schemes`` argument which allows to customize the accepted URI schemes
+ (instead of the defaults http(s) and ftp(s)).
@timgraham

timgraham Dec 28, 2013

Owner

`` around http/ftp

@timgraham timgraham commented on an outdated diff Dec 28, 2013

docs/ref/validators.txt
A :class:`RegexValidator` that ensures a value looks like a URL, and raises
- an error code of ``'invalid'`` if it doesn't.
+ an error code of ``'invalid'`` if it doesn't. In addition to the optional
+ arguments of its parent :class:`RegexValidator` class, ``URLValidator``
+ accepts an extra optional attribute:
+
+ .. attribute:: schemes
+
+ URL/URI scheme list to validate against. If not provided, the default
+ list is ``['http', 'https', 'ftp', 'ftps']``. As a reference, the IANA
+ Web site provides a full list of `valid URI schemes`_.
+
+ .. _valid URI schemes: https://www.iana.org/assignments/uri-schemes/uri-schemes.xhtml
@timgraham

timgraham Dec 28, 2013

Owner

I think this should be indented at the same level as the text above it

Fixed #21242 -- Allowed more IANA schemes in URLValidator
Thanks Sascha Peilicke for the report and initial patch, and
Tim Graham for the review.

@claudep claudep closed this Dec 28, 2013

@claudep claudep deleted the claudep:21242 branch Dec 28, 2013

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