130545 - Servers running on non-default port cannot set cookies for domain

Status: VERIFIED FIXED

(Core :: Networking: Cookies, defect)

Description

[Hrvoje Niksic]

This occurs in in 0.9.9.  The previous version I was using, 0.9.8, did not
exhibit the problem.

When a server running on a non-default port tries to set the cookie for the
domain, Mozilla dismisses the cookie, i.e. does not store it or send it back.  I
have prepared a test case that demonstrates the problem:

1. Visit http://muc.arsdigita.com:2005/set-cookie.tcl
This is a page that prints "done", but also sends out a cookie "testfoo" with
the value "bar", for the entire ".arsdigita.com" domain.

2. Go to the cookie manager and see that no cookie has been recorded for the
domain ".arsdigita.com".  (Two cookies are recorded under "muc.arsdigita.com",
but those are different.)

Now repeat the same with Mozilla 0.9.8, and you'll see that step #2 shows you
that the cookie has been recorded.  Furthermore, the same problem does not occur
in 0.9.9 when the cookie-issuing server runs on port 80.  (I don't have a test
case for that, but it's pretty obvious that it works, because otherwise a huge
number of Internet sites would be rendered useless.)

I am not sure if this change is accidental or intentional, but the
discrimination of servers running on non-default ports sure breaks many web
applications.  For example, a company-wide single sign-on system is running its
own dedicated server on a non-default port.  When the user needs authorization,
he is redirected to the single sign-on server which authorizes the user, issues
him a cookie for the entire domain, and finally redirects him back to the
regular site.  With Mozilla 0.9.9 this no longer works because the domain-wide
cookie from the single sign-on server comes from a non-default port.

Comment 1

[Stephen P. Morse]

This could be a consequence of our finally checking correctly for third-party 
cookies (that code was just added recently).  Do you have your pref set to 
reject third-party cookies?  If so, can you try it with accept-all and see if 
you get the same results.

Comment 2

[Hrvoje Niksic]

My prefs are set to "Enable all cookies," and I have not blocked any sites.  (As
demonstrated by the fact that the cookies are correctly set when the server is
running on port 80.)  Also, the same problem was noticed by several other people
in my office who attempted to use 0.9.9.

Have you tried visiting the provided URL?  I'd be interested to know if you can
actually repeat the problem and whether you agree that what I see is a bug.

Comment 3

[Stephen P. Morse]

I wonder if this could be a dup of bug 130583.

Comment 4

[Hrvoje Niksic]

Since the URL in bug 130583 is also on a non-default port, I would strongly
suspect that we're seeing the same problem, i.e. that 130583 is a dup of this bug.

Comment 5

[Brent]

I too can confirm this behaviour in 0.9.9 (in fact, it starting doing this in
the nightly builds about 1.5 weeks ago). Yes Stephen, i believe bugs 130583 &
130545 are one-and-the-same.

My company's Oracle Single-Signon system stopped working with the 0.9.9 release,
but worked great through 0.9.8.  I have "Enable all cookies" set in my prefs,
and no sites blocked, but i still see no cookies being set in the Cookie Manager
(i can see them set when using 0.9.8).

Here is what happens on our Intranet:

1) i go to dsun4.mycompany.com:8088/plsql/... and enter my name/pass
2) the PL/SQL procedure that handles the signin screen calls a remore procedure
(RPC) on another database (on a different Sun box - dsun5.mycompany.com:8088)
and that procedure then tries to set a cookie.
3) i should then be redirected to the URL that i just authenticated for, but i
am bounced right back to the signin page, and no cookies are set at all!

Here is a post at mozillazine of another case where a user is locked out of
Novell's portal for the same reason:
http://mozillazine.org/talkback.html?article=2164&message=62#62

It appears that our signin page, and all of the other reporters, are probably
calling around to other machines/DBs/RPCs and this is preventing cookies from
being set.

I added some code in the signin PL/SQL procedure to set a cookie (since it is
still on dsun4.mycompany.com:8088) and it worked fine.

Are there any other tests or bits of information i can provide you that would be
helpful in tracking this issue down?

Comment 6

[Stephen P. Morse]

See also bug 130638.  Probably related also.

Comment 7

[Leo H]

I find that if I try to set the domain of the cookie, it fails (mozilla ignores
the cookie).  If I leave the domain to be default, mozilla does accept it.  This
behaviour started in 0.9.9.
Of course, I'm talking about sites that have a specified port number.

Comment 8

[Olivier Cahagne]

Might also explain bug 129342 which doesn't set cookies on port 443 (for this
site, Mozilla can still set cookies on other SSL sites).

Comment 9

[Michael Schout]

This occurs on the mtrunk builds as well.  The last working linux-gnu-sea build
that works for me is in the 2002-03-05-21-trunk directory.  I tried a build from
one of the 2002-03-06-??-trunk directories and that build doesnt work.  Cookies
get set fine as long as the port number is default, but on sites using
specified, non-default ports at least, the cookies are completely ignored.  The
2002-03-05-21-trunk directory contains the last working build for me, so
hopefully this will point someone in the right direction on this.

Comment 10

[Michiel van Leeuwen (email: mvl+moz@)]

Looking at the time this problem is a regression from bug 124042

Some debugging told me that in nsCookies.cpp when removing the port part from
the host, the length of the string is not calculated properly. I will attach a
path that fixis this here, but I don't know if this is the right thing to do.
Maybe the Length funvction should be fixed.

Comment 11

[Stephen P. Morse]

cc'ing darin in case this is a regression from bug 124042 as the above comment 
suggests

Comment 12

[Michiel van Leeuwen (email: mvl+moz@)]

Attachment: Change way lenth of host withous port is calculated

Comment 13

[Stephen P. Morse]

Michiel, could you please explain why there is a difference in the two ways of 
calculating the cur_host_length?  What value is obtained in each case?

Thanks.

Comment 14

[Michiel van Leeuwen (email: mvl+moz@)]

I put the following debug-prints in nsCookies.cpp (Starting at line 1142, where
my patch is):
printf("host: %s\n",cur_host.get());
printf("cur_host.Length(): %d\n",cur_host.Length());
printf("PL_strlen: %d\n",PL_strlen(cur_host.get()));

When I vist www.test.editengine.nl:8000 I get (If you want to check: press the
demo button):

host: www.test.editengine.nl
cur_host.Length(): 27
PL_strlen: 22

the first length cearly is wrong, and still counts the :8000 part. Why that
happens, I don't know.

Comment 15

[Michael Schout]

Length() appears to merely tell you how long the string is.  In this case
though, whats happening is that if there is a ':' character in the cur_host
string, then that character gets replaced with the NULL character ('\0'). 
So if we had "foo.com:8000" the new value is "foo.com\08000" which still has
the same length, but there is a \0 where the ':' was before.

Maybe instead of clobbering the ':' it would have been better to use setLength()
or just to use PL_strlen() instead.  I'm not familiar enough with the rest of
the mozilla code to know what the ramifications of fixing Length() to look
for a \0 would do (e.g.: if its used on binary strings somewhere else in the
code, then looking for \0 in Lengh() is certainly the wrong thing to do)

Comment 16

[Darin Fisher]

michael: by design, the string classes support embedded nulls.  IOW, the string
class is correct.  if you need to, you can call SetLength to update the length
or call strlen as you've said.

another approach is this:

nsACString::const_iterator begin, colon, end;
str.BeingReading(begin);
str.EndReading(end);
if (FindCharInReadable(':', colon = begin, end))
    len_of_host = Distance(begin, colon);

see nsReadableUtils.h for FindCharInReadable() and Distance().

Comment 17

[Stephen P. Morse]

Thanks, schout, for explaining this.  You and van Leeuwen both save me debugging 
time.

Now that I understand what is happening, I agree with this patch.  And, if I'm 
reading darin's comment correctly, he does also.

r=morse

darin, please sr.

Comment 18

[Darin Fisher]

Comment on attachment 74373 [details] [diff] [review]
Change way lenth of host withous port is calculated

sr=darin

Comment 19

[Darin Fisher]

Comment on attachment 74373 [details] [diff] [review]
Change way lenth of host withous port is calculated

sr=darin

Comment 20

[Asa Dotzler [:asa]]

Comment on attachment 74373 [details] [diff] [review]
Change way lenth of host withous port is calculated

a=asa (on behalf of drivers) for checkin to the 1.0 trunk

Comment 21

[Stephen P. Morse]

Fix checked in.  Thanks Michiel and Michael.

Comment 22

[Stephen P. Morse]

*** Bug 129342 has been marked as a duplicate of this bug. ***

Comment 23

[Stephen P. Morse]

*** Bug 130583 has been marked as a duplicate of this bug. ***

Comment 24

[Stephen P. Morse]

*** Bug 134194 has been marked as a duplicate of this bug. ***

Comment 25

[Tom Everingham]

verified:  04/09/02 tr, win NT4, Mac osX, linux rh6