Skip to content
/ django Public

Update validators.py to allow for (more) correct URL Validation#19095

Closed
ludwigkraatz wants to merge 1 commit intodjango:mainfrom
ludwigkraatz:patch-1
Closed

Update validators.py to allow for (more) correct URL Validation#19095
ludwigkraatz wants to merge 1 commit intodjango:mainfrom
ludwigkraatz:patch-1

Conversation

@ludwigkraatz
Copy link

@ludwigkraatz ludwigkraatz commented Jan 23, 2025

fully allowing for URLs as per rfc3986#section-3.2.2 - with a regex solution for localhost (and whatever else is possible) instead of a hardcoded < "magicnumber"-80%-"solution" >

what is not said in rfc3986, dealing with URI/URLs:

  • that a hostname requires multiple labels
  • thus - no "." is required to seperate them
  • actually, length limit of 63 is also not a given if we're not in a DNS context. but for simplistical reasons, i chose to keep it aligned to DNS-Labels, as provided via DomainNameValidator.hostname_re

Why this is necessary & usefull:
Single-label URLs might be used

  • in intranet situations
  • for URLs that represent services / schemes that do not comply to DNS naming conventions
  • for local testing (local DNS resolution that is not based on FQDN)
  • mDNS [RFC 6762] solutions, operating under .local TLD (which as of that RFC can be ommitted in a local context)
  • the validator is name URLValidator, not DNSURLValidator

Trac ticket number

ticket-36131

Branch description

Provide a concise overview of the issue or rationale behind the proposed changes.

Checklist

  • This PR targets the main branch.
  • The commit message is written in past tense, mentions the ticket number, and ends with a period.
  • I have checked the "Has patch" ticket flag in the Trac system.
  • I have added or updated relevant tests.
  • I have added or updated relevant docs, including release notes if applicable.
  • I have attached screenshots in both light and dark modes for any UI changes.

fully allowing for URLs as per rfc3986#section-3.2.2 - with a regex solution for localhost (and whatever else is possible) instead of a hardcoded < "magicnumber"-80%-"solution" >

what is not said in rfc3986, dealing with URI/URLs:
- that a hostname requires multiple labels
- thus - no "." is required to seperate them
- actually, length limit of 63 is also not a given if we're not in a DNS context. but for simplistical reasons, i chose to keep it aligned to DNS-Labels, as provided via DomainNameValidator.hostname_re

Why this is necessary & usefull:
Single-label URLs might be used
- in intranet situations
- for URLs that represent services / schemes that do not comply to DNS naming conventions
- for local testing (local DNS resolution that is not based on FQDN)
- mDNS [RFC 6762] solutions, operating under .local TLD (which as of that RFC can be ommitted in a local context)
- the validator is name URLValidator, not DNSURLValidator
@github-actions github-actions bot added the no ticket Based on PR title, no linked Trac ticket label Jan 23, 2025
@ludwigkraatz ludwigkraatz changed the title Update validators.py Update validators.py to allow for (more correct) URL Validation Jan 23, 2025
@ludwigkraatz ludwigkraatz changed the title Update validators.py to allow for (more correct) URL Validation Update validators.py to allow for (more) correct URL Validation Jan 23, 2025
@ludwigkraatz
Copy link
Author

ludwigkraatz commented Jan 23, 2025

EDIT: the following is to be ignored. its because the URLField is tested, which has lazy-scheme support

btw:

the test - test_urlfield_clean_invalid, seems to be a false negative outside of this merge request.

because even with my changes, this should not be valid - and as such there seems to be a broader issue.
In other words - if tested with "localhost" instead of "https://localhost" the URLValidator would probably not raise an Issue. Even though it should.

I tested it: i was correct: there is a deeper issue

see this pull request which tested for 'localhost' => URL field

@ludwigkraatz
Copy link
Author

the issue is more complex than just to fix a regex. its the whole concept of URLValidator thats off - compared to what a valid URL really is. It has been called a design decision - which they never seem to have considered (and probably not going to) a '(semantic) bug' of sorts.

as such - this issue does not seem to be tackled any time soon, if ever.

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

Labels

no ticket Based on PR title, no linked Trac ticket

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant