Opened 3 weeks ago
Closed 19 hours ago
#36923 closed Bug (fixed)
URLField.to_python() mangles mailto: URLs
| Reported by: | waveywhite | Owned by: | Natalia Bidart |
|---|---|---|---|
| Component: | Forms | Version: | 5.2 |
| Severity: | Normal | Keywords: | mailto urlfield |
| Cc: | waveywhite | Triage Stage: | Ready for checkin |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
Having derived from django.forms.URLField and django.models.URLField to set up a form that can handle mailto URLs, I'm finding that the URL gets mangled. Entering "mailto:test@…" results in "mailto://test@…".
This was encountered while creating models for a Wagtail site. Wagtail uses Django's models and form fields within its framework.
My modified URLFields:
from django import forms
from django.db import models
class ContactUrlFormField(forms.URLField):
default_validators = [
validators.URLValidator(schemes=['http', 'https', 'mailto', 'tel'])
]
class ContactUrlField(models.URLField):
default_validators = [
validators.URLValidator(schemes=['http', 'https', 'mailto', 'tel'])
]
def formfield(self, **kwargs):
return super().formfield(
**{
"form_class": ContactUrlFormField,
**kwargs,
}
)
NB. It would be great to have a way of defining which URL schemes are allowed as an option in the model field and then have it picked up by the form field automatically.
Change History (16)
comment:1 by , 3 weeks ago
| Owner: | set to |
|---|---|
| Status: | new → assigned |
follow-up: 3 comment:2 by , 3 weeks ago
| Triage Stage: | Unreviewed → Accepted |
|---|---|
| Type: | Uncategorized → Bug |
comment:3 by , 3 weeks ago
Replying to Jacob Walls:
The current normalization logic unconditionally assumes a hierarchical structure, forcing mailto:// on opaque schemes like mailto: and tel:.
We could restrict this domain enforcement to an allowlist of standard web schemes (http, https, ftp, ftps) to prevent mangling valid URIs that use data segments. Note that we already default the scheme to http if none is provided.
What's your take on this approach?
follow-up: 6 comment:5 by , 13 days ago
| Owner: | changed from to |
|---|
Hi Vishy. Natalia mentioned she has a local stash that solves this issue. Do you mind if I assign it to her? I don't want you to duplicate any effort.
comment:6 by , 12 days ago
Replying to Jacob Walls:
Hi Vishy. Natalia mentioned she has a local stash that solves this issue. Do you mind if I assign it to her? I don't want you to duplicate any effort.
That's fine Jacob. However, I wanna give a heads up on this:
While verifying scheme in django.core.validators.URLValidator , the delimiter used is :// . I think we should just use a semi-colon (":"), which is standard scheme delimiter according to RFC 3986 Standards.
Please have a look at this, and provide your opinion.
-
django/core/validators.py
--git a/django/core/validators.py b/django/core/validators.py index 534acdd904..18239e3ad4 100644
a b class URLValidator(RegexValidator): 166 166 if self.unsafe_chars.intersection(value): 167 167 raise ValidationError(self.message, code=self.code, params={"value": value}) 168 168 # Check if the scheme is valid. 169 scheme = value.split(": //")[0].lower()169 scheme = value.split(":")[0].lower() 170 170 if scheme not in self.schemes: 171 171 raise ValidationError(self.message, code=self.code, params={"value": value})
Also, the IPv6 netloc part verification under the same URLValidator should only be applied to heirarchical/web-style URIs, but not to opaque URIs. I guess, we could use the above obtained scheme to verify that part.
comment:7 by , 12 days ago
Good question. The URLValidator is out of scope. I only accepted the ticket for removing the mangling inside to_python(), but I didn't clarify that well enough. (We have other tickets, some of which were wontfixed, for extending URLValidator to work with other schemes, e.g. #36054, #25593, and #25594.)
comment:8 by , 12 days ago
| Summary: | URLField mangles mailto: URLs → URLField.to_python() mangles mailto: URLs |
|---|
comment:13 by , 25 hours ago
The security release I just published solves this issue, though I intentionally did not add tests for this case to avoid polluting a focused security patch. This means that the work needed to close this ticket is proper tests for the reported cases, which should all pass without any code changes AFAIU.
comment:15 by , 21 hours ago
| Triage Stage: | Accepted → Ready for checkin |
|---|
Thanks for the report. Looks like the mangling is happening here. We shouldn't assume that "if a domain is not provided, that the path segment contains the domain" for mailto links.
Tentatively accepting, since this is a security-sensitive area, and we have to make sure however we adjust this heuristic can't be fooled.