127870 - URL: "@" in password treated as delimiter in <userinfo>@<host>:<port>

Status: VERIFIED FIXED

(Core :: Networking, defect, P3)

Description

[[e-mail]]

Mozilla 0.9.8:

Try to load ftp://anonymous:[e-mail]@findus.dhs.org/

Mozilla says Alert: "[e-mail] could not be found. Please check the
name and try again".

I am sure this is a duplicate, but I couldn't find it in Query (Bugzilla badly
needs an improved Query... how to improve it I do not know :/)

Comment 1

[Boris Zbarsky [:bzbarsky]]

The url is invalid, the "@" in the password needs to be escaped as %40

Comment 2

[Matthias Versen [:Matti]]

I agree :

-> invalid

Comment 3

[[e-mail]]

Apparently according to some person it is a valid FTP RFC URL.

Anyhow, Netscape 4 handles it.

How is Mozilla going to handle sending e-mail addresses as URLs? If it is
possible to set it in the preferences as FTP password, the users probably thinks
the client can handle it in the URL field, as well.

If I type in ftp://anonymous:[e-mail]/, the index page shows:

ftp://anonymous:[e-mail]@findus.dhs.org/


Very confusing -- what if I want to copy and paste that address?

This is ugly.

Comment 4

[Matthias Versen [:Matti]]

CC [e-mail] : He should know if this URL is valid...

Comment 5

[Andreas Otte]

This is a nasty problem. To not confuse the urlparser a username/password should
not contain a @, it has to be escaped, because in the urlsyntax it delimits
username/password from the host.

And the title in the index page should not unescape it, at least that's my
opinion. I remember there is a pref somewhere inside mozilla to control it
inside mail/news, allow or not allow @ inside username/password. 

However there is no such thing for ftp or http urls. If we send the email
address as password to the ftp server we do it inside the ftp protocol.

But I will take a look at the urlparser if it is possible to be better ...
reopening for now 

Comment 6

[Andreas Otte]

assigning to me

Comment 7

[Andreas Otte]

rfc 2396 says:

   URL schemes that involve the direct use of an IP-based protocol to a
   specified server on the Internet use a common syntax for the server
   component of the URI's scheme-specific data:

      <userinfo>@<host>:<port>

   where <userinfo> may consist of a user name and, optionally, scheme-
   specific information about how to gain authorization to access the
   server.  The parts "<userinfo>@" and ":<port>" may be omitted.

      server        = [ [ userinfo "@" ] hostport ]

   The user information, if present, is followed by a commercial at-sign
   "@".

      userinfo      = *( unreserved | escaped |
                         ";" | ":" | "&" | "=" | "+" | "$" | "," )

@ is not part of unreserved or escaped ... so technically mozilla does nothing
wrong when parsing the url. However since the hostname also does not include a @
(I hope that is still true for iDNS) it might be possible to go after the
rightmost @ in the authority component. 

Comment 8

[Andreas Otte]

Attachment: patch to look for the last @ in the authority section to delimit userinfo from host

With this patch we look for the last @ in the authority section not the first
one to delimit userinfo from host. Doing so is not necessary according to RFC
2396, but this will fixup urls that have not escaped the @ inside username
and/or password. The escape of @ inside username/password however still
happens.

Comment 9

[Darin Fisher]

be careful.. what if authLen is zero?  i think nsURLParsers really needs a
reverse memchr impl, but that can wait.

Comment 10

[Andreas Otte]

Hmmmm ... valid point, revised version coming ...

Comment 11

[Andreas Otte]

Attachment: revised patch, check for authLen == 0

bail out with NS_ERROR_MALFORMED_URI if authLen == 0, after all we are in the
AuthURLParser!

Comment 12

[Darin Fisher]

i wouldn't say that's necessarily an error.  imagine an protocol in which the
authority is optional.  not that i can think of such an example off the top of
my head, but whatever.  seems like you should just do this instead:

  if (authLen == 0) {
      SET_RESULT(username, 0, -1);
      SET_RESULT(password, 0, -1);
      SET_RESULT(hostname, 0, 0);
      if (port)
          *port = -1;
      return NS_OK;
  }

looks like ParseServerInfo needs a similar check :-(

Comment 13

[Darin Fisher]

fwiw: this is what nsBaseURLParser::ParseAuthority does when it gets an empty
string.

Comment 14

[Andreas Otte]

Hmmm ... in the old urlparser world the AuthParser expected to find an
authority, that's what that parser was for. The current one does something
similar ... in a way. If it gets fed with http:///path which looks like a http
url without a host it just ignores the third slash and uses path as host. Kind
of enforcing a host :) 

Anyway, your suggestion is surely more consistent with the current
implementation, next version coming up including check in ParseServerInfo.

Comment 15

[Andreas Otte]

Attachment: revised version, now simply set segments to 0, also includes check for ParseServerInfo

Comment 16

[Darin Fisher]

Comment on attachment 71755 [details] [diff] [review]
revised version, now simply set segments to 0, also includes check for ParseServerInfo 

sr=darin

andreas: the point is that anyone could instantiate a URL parser, and
ParseAuthority is a public method.

Comment 17

[Doug Turner (:dougt)]

Comment on attachment 71755 [details] [diff] [review]
revised version, now simply set segments to 0, also includes check for ParseServerInfo 

r=dougt

Comment 18

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Comment on attachment 71755 [details] [diff] [review]
revised version, now simply set segments to 0, also includes check for ParseServerInfo 

a=dbaron for trunk checkin

Comment 19

[Andreas Otte]

fix checked in on trunk

Comment 20

[benc]

quick extension of summary... if someone has something more meaningful, please
change this...

Comment 21

[benc]

So what is the effect of the fix? Is it to always use the last "@"?

What is the fate of the previous "@"'s? 

At first I thought you had implemented something else, so I did a packet trace
of a  FTP (HTTP proxy) request to see if that was wrong, and I found that the
ftp URL is escaped before sending to the proxy server.

reporter: can you confirm if this works for you now? also, can you tell us if
that ULR is supposed to work (or just not give the DNS error anymore?)

Comment 22

[Andreas Otte]

We always go for the last unescaped @ in the authority section as delimiter
between host and username/password. Other @ are part of the username and/or
password. 
Although we do this now @ is still not allowed in usernames and passwords by RFC
2396, so it gets escaped as it should be.

Comment 23

[[e-mail]]

Yes, this seems to work now.

Comment 24

[benc]

VERIFIED: mozilla 1.1, Win98.
It's going into the ftp testcases, so I'll open if it doesn't work for linux|macOSX.

Comment 25

[benc]

VERIFIED: Mozilla 1.4b/Linux