241688 - sainsburysbank.co.uk - redirect to invalid URL syntax

Status: RESOLVED DUPLICATE of bug 652186

(Web Compatibility :: Site Reports, defect)

Description

[David Hallowell]

http://online.sainsburysbank.co.uk/ should redirect to
https://online.sainsburysbank.co.uk/ just like http://www.halifax-online.co.uk/
does (the Halifax do the credit cards for Sainsbury's and the online system is
very similar).

However going to online.sainsburysbank.co.uk produces the error:
\\online.sainsburysbank.co.uk could not be found.

Looking at the HTTP headers (using the livehttpheaders extension) be see that
the server tries to redirect to https:\\online.sainsburysbank.co.uk which no
doubt works in IE

http://online.sainsburysbank.co.uk/



GET / HTTP/1.1

Host: online.sainsburysbank.co.uk

User-Agent: Mozilla/5.0 (BeOS; U; BeOS BePC; en-US; rv:1.7b) Gecko/20040424

Accept:
text/xml,application/xml,application/xhtml+xml,text/html;q=0.9,text/plain;q=0.8,image/png,*/*;q=0.5

Accept-Language: en-us,en;q=0.5

Accept-Encoding: gzip,deflate

Accept-Charset: ISO-8859-1,utf-8;q=0.7,*;q=0.7

Keep-Alive: 300

Connection: keep-alive

Cookie: SQI=0; sainsburyscouk=sbid=26042004-1785829775&adsrc=&adsrcexpire=;
GTSessionID838440871016=83844087101644535347



HTTP/1.x 302 Object moved

Location: https:\\online.sainsburysbank.co.uk



As a customer I will contact them - the Halifax were quick at fixing a bug in
their system when I informed them, so hopefully they'll be the same with the
Sainsbury's one

Comment 1

[David Hallowell]

Here's two replies I've got from them so far (usual standard replies):
> Thank you for your e-mail.
> 
> Please accept my apologies for the delay in my response. 
> 
> We are experiencing an unusually high numbers of e-mails received and
> are doing everything possible to reduce the backlog.
> 
> Thank you in advance for your patience.
> 
> To use the Sainsburys Bank Online Service your browser will need to
> be
version 5.5 (or higher) of Microsoft Internet Explorer or Netscape 6.0
Navigator/Communicator and also be capable of supporting 128 bit
encryption.
> 
> Please note that most recent web browsers by Microsoft and Netscape
(version 4 and higher) will automatically support 128 bit encryption.


After explaining that this bug will also affect Netscape 6 and 7 I got the
following also canned reply:
> Thank you for your feedback regarding our online service.
> 
> We are constantly looking to improve our service and comments of this
> nature are very much appreciated.
> 
> Unfortunately the only browsers that we currently support are
> Netscape and Internet Explorer.
> 
> I can confirm that your comments will be fed into our development
> team for further consideration.
> 
> Once again thank you for your feedback and taking the time to contact
> us.
> 

So I've reiterated again that this is also a problem for Netscape 7 and
mentioned that this is a one line fix

Comment 2

[Chris Lawson (gone)]

David, did you ever get anywhere with them? I notice it's still a problem (and will continue to be for any browser that is not IE on Windows).

Comment 3

[Hallvord R. M. Steen [:hallvors]]

wow.. still a problem!

GET / HTTP/1.1
Host: online.sainsburysbank.co.uk
User-Agent: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en; rv:1.9.0.19) Gecko/2011032020 Camino/2.0.7 (like Firefox/3.0.19)
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8
Accept-Language: nn-no,nn;q=0.9,no-no;q=0.8,no;q=0.6,nb-no;q=0.5,nb;q=0.4,en-us;q=0.3,en;q=0.1
Accept-Encoding: gzip, deflate
DNT: 1
Cookie: Calling_URL=http://online.sainsburysbank.co.uk:81/
Connection: keep-alive

HTTP/1.1 302 Object moved
Location: https:\\online.sainsburysbank.co.uk
Connection: close

Fails in Firefox and old Opera, works in Chrome (and presumably IE)

Comment 4

[Hallvord R. M. Steen [:hallvors]]

Called them, left contact details which will apparently be passed on to a suitable person.

Comment 5

[Hallvord R. M. Steen [:hallvors]]

(Duly noting that if both WebKit/Blink-based browsers and Trident handle this error, Firefox is by now the only major browser not to. IMO we should consider whether to handle it gracefully too..)

Comment 6

[Hallvord R. M. Steen [:hallvors]]

Patrick, would this be a netlib issue, and has it been considered?

Comment 7

[Hallvord R. M. Steen [:hallvors]]

404
Mozilla/5.0 (Windows NT 5.1; rv:23.0) Gecko/20100101 Firefox/23.0

Comment 8

[Patrick McManus [:mcmanus]]

(In reply to Hallvord R. M. Steen from comment #6)
> Patrick, would this be a netlib issue, and has it been considered?

I would probably take a patch to increase web compatibility here..

Comment 9

[Karl Dubost💡 :karlcow]

I wonder which part of the code where it should be fixed.
This has something about backslash but is restricted to Windows family.

https://github.com/mozilla/mozilla-central/blob/b619e60f534b91507319fd4eb33ac3f1a4f9a63e/docshell/base/nsDefaultURIFixup.cpp

Comment 10

[Hallvord R. M. Steen [:hallvors]]

We do indeed get to nsDefaultURIFixup::CreateFixupURI with the malformed URL. The data the method is trying to clean up is:

-		aStringURI	{mData=0x1961a168 "https://\\\\online.sainsburysbank.co.uk/" mLength=38 mFlags=65541 }	const nsACString_internal &

(Aside: it doesn't make sense to me to only clean up backslashes in URLs if you're running Windows.. So sites that use the wrong slash in a URL will work on Windows and fail on other platforms?!? That's just weird..)

The problem is that the algorithm here stops at the first instance of a forward slash:

http://dxr.mozilla.org/mozilla-central/source/docshell/base/nsDefaultURIFixup.cpp#195

Simply removing 

|| *start == '/'

from that line ensures that the cleanup will reach the backslashes after the https:// - the cleanup will now run its course and the site load fine.

Patrick, would that be an acceptable fix?

Comment 11

[Patrick McManus [:mcmanus]]

comment 10 is better directed to a docshell peer..

Comment 12

[Boris Zbarsky [:bzbarsky]]

The docshell code there is generally used for things like input to the URL bar, which would end up in here as http:\\whatever.

But this case has necko creating a URI from the string http:\\whatever first, which changes it into http://\\whatever.  We purposefully don't mess with that in the fixup code, and the comment explains why.

The reason this presumably works in Trident and Blink is that they treat backslash as an alias for '/' in URIs in general.  We should probably just give up and do that too, in necko.

Comment 13

[Hallvord R. M. Steen [:hallvors]]

Note that this docshell code does *not* run just for input to the URL bar in this case, it also runs for the data in the Location: response header. If it's no longer intended to run just for user input, it seems like it should be moved to necko to be applied everywhere.

Comment 14

[Boris Zbarsky [:bzbarsky]]

> it also runs for the data in the Location: response header.

Does it, in general?  Or are you running into the fact that docshell will run it if a load attempt ends up NS_ERROR_UNKNOWN_HOST in some cases?

Generic code to treat '\\' as '/' would presumably apply to all URIs, not just those being loaded in a docshell, all backslashes, not just those that are in what looks like a hostname, and live in netwerk/base/src/nsURLParsers.cpp

Comment 15

[Hallvord R. M. Steen [:hallvors]]

(In reply to Boris Zbarsky [:bz] from comment #14)
> > it also runs for the data in the Location: response header.

> Does it, in general? 

No, it did in this case - on a closer look it was indeed due to the NS_ERROR_UNKNOWN_HOST condition.

(I wonder why I see createFixupURI calls with something that looks like bogus data:
spec	{mStorage=0x00ead074 "ñ\x12\a˜Òê" }	nsAutoCString
apparently coming from mouse event processing?)

> Generic code to treat '\\' as '/' would presumably apply to all URIs, not
> just those being loaded in a docshell, all backslashes, not just those that
> are in what looks like a hostname, and live in
> netwerk/base/src/nsURLParsers.cpp

Basically move nsDefaultURIFixup::CreateFixupURI to nsURLParsers.cpp and call it before parsing? 

Will such a change be controversial at this day and age?

I guess one can argue that it is a good approach to do the URL fixup stuff if and only if a URL fails to load. So it currently cleans up a hostname if it fails to resolve, this could also be extended to 404 loads I guess - and the semi-mythical case referred to here:

// The forward slash test is to stop before trampling over
// URIs which legitimately contain a mix of both forward and
// backward slashes.

is presumably not much of a concern if we try to load the URL first and only mangle it on failure..?

Comment 16

[Boris Zbarsky [:bzbarsky]]

> Basically move nsDefaultURIFixup::CreateFixupURI to nsURLParsers.cpp and call it before
> parsing? 

No. CreateFixupURI does all sorts of fixup that's not appropriate in general (e.g. on the values of <a href>).  Its primary goal is to fix up whatever crud the user typed into the url bar.

I'm talking about implementing the rules for backslash from <http://url.spec.whatwg.org/#parsing>, since it seems like we lost on our attempt to keep that insanity out of the web platform.

Comment 17

[Hallvord R. M. Steen [:hallvors]]

In other words, this site will probably start working when bug 652186 is fixed. There's hope :)

(We can of course keep nagging the bank if we have bandwidth. I had a very nice phone conversation with one of their technical ppl last autumn, he did seem to understand the problem - it just hasn't been a priority I guess.)

Comment 18

[Karl Dubost💡 :karlcow]

Still the case. 
This is working in Opera (Blink), but not in Safari and Firefox latest versions.

Comment 19

[Karl Dubost💡 :karlcow]

Switching to contactready again given we didn't get an answer or action. The site needs to be recontacted.

Comment 20

[Hallvord R. M. Steen [:hallvors]]

Seems a fix for bug 652186 is underway. Let's stop spending time on this, it will soon work anyway..

Comment 21

[Mike Taylor [:miketaylr]]

INVALID is probably wrong, it already works in Nightly (fix landed 4 days ago).

Comment 22

[Hallvord R. M. Steen [:hallvors]]

But FIXED for an outreach bug means "the site fixed it", not "we fixed it in core"..

Comment 23

[Mike Taylor [:miketaylr]]

OK, either is better than invalid.