Opened 6 years ago

Closed 8 months ago

#9791 closed Uncategorized (duplicate)

URLField's validation does not support in-URL basic AUTH

Reported by: django-nolan@… Owned by: nobody
Component: Forms Version: 1.0
Severity: Normal Keywords:
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

URLs of the form:
http://user:password@host/path
are not supported.

It appears that this is a urllib2 limitation.

Change History (8)

comment:1 follow-up: Changed 6 years ago by jacob

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Resolution set to wontfix
  • Status changed from new to closed

I'm reluctant to allow this simply on the principle that storing other people's passwords in plaintext is bad juju. Indeed, it's trivial to use RegexField with a custom pattern and/or custom validation logic of this is something you need. So unless someone can give a great reason why this ought to be in core, I don't see much use in fixing this.

comment:2 follow-up: Changed 6 years ago by django-nolan@…

Well, I have users entering URLs of this form, and so I needed to not choke on them.

That said, I've already turned off validation and hard coded my own inline, so I no longer especially care about this. If this is deemed a rare enough need that it should be reimplemented each time, I see no problem with that.

comment:3 in reply to: ↑ 2 Changed 6 years ago by ubernostrum

Replying to django-nolan@sigbus.net:

If this is deemed a rare enough need that it should be reimplemented each time, I see no problem with that.

(this is the part where I point out that we're all programmers here, and if you find yourself needing this often the thing to do is not "reimplement each time", but rather whip up a quick little library, put this in it, reuse it over and over as needed, and then maybe even offering that library to others in case they need it too)

comment:4 Changed 6 years ago by anonymous

Unfortunately, I suspect my quick hack is insufficiently general to be a library function, but here it is in case someone else finds this bug report:

        u = urlsplit(in_url)

        if u.scheme.lower() != "http":
            raise forms.ValidationError("Only http is supported")

        passman = urllib2.HTTPPasswordMgrWithDefaultRealm()
        if u.netloc.find('@') >= 0:
            user = u.username
            passwd = u.password
            u = list(u)
            u[1] = u[1].split('@')[1]
            url = urlunsplit(u)
            passman.add_password(None, url.split["//"][1], user, passwd)
        else:
            url = in_url
        authhandler = urllib2.HTTPBasicAuthHandler(passman)
        opener = urllib2.build_opener(authhandler)
        urllib2.install_opener(opener)

        #XXX Add timeout param when we can use python 2.6...
        try:
            req = urllib2.Request(url)
            f = urllib2.urlopen(req)
        except urllib2.HTTPError, e:
            raise forms.ValidationError("Couldn't get URL %s: %s" %
                                        (in_url, e))
        except urllib2.URLError, e:
            raise forms.ValidationError("Couldn't connect to %s: %s" %
                                        (u[1], e.reason))

        if f.code != 200 and:
            raise forms.ValidationError("URL did not return 200 OK")

comment:5 in reply to: ↑ 1 Changed 3 years ago by anonymous

  • Easy pickings unset
  • Severity set to Normal
  • Type set to Uncategorized
  • UI/UX unset

Replying to jacob:

I'm reluctant to allow this simply on the principle that storing other people's passwords in plaintext is bad juju. Indeed, it's trivial to use RegexField with a custom pattern and/or custom validation logic of this is something you need. So unless someone can give a great reason why this ought to be in core, I don't see much use in fixing this.

There's no way to determine what use case this may be or what level of "juju" it implies.
I use it frequently for wget, cadaver and some xmlrpc cli utilities.
It's certainly not worse than FTP or login forms over HTTP, both are quite commonplace in the real world.

Let's just follow the standards and let the users worry about their specific use cases and/or security issues.

comment:6 Changed 3 years ago by aaugustin

anonymous, you must be aware that the "standards" regexp provided in the RFC for parsing URLs matches absolutely anything, so it's useless for validation. Just "following the standards" doesn't mean much here. Also it isn't Django's policy to let users worry about security issues.

comment:7 Changed 8 months ago by nemesisdesign

  • Resolution wontfix deleted
  • Status changed from closed to new

Suppose this case: ssh://git@git.repo.com/my-fantastic-privately-hosted-project.git. Isn't this a URL? With the current URLField it is not possible to have this URL pass the validation without some additional work.

If the field is called a URLField it should do what it says, at least according to the principle of least astonishment.

In my opinion, making us work more to achieve such simple things is against the goals of Django.

Therefore I reopen this ticket.

comment:8 Changed 8 months ago by timgraham

  • Resolution set to duplicate
  • Status changed from new to closed

Duplicate of #20003 which is accepted.

Note: See TracTickets for help on using tickets.
Back to Top