Closed Bug 241688 Opened 16 years ago Closed 4 years ago

sainsburysbank.co.uk - redirect to invalid URL syntax

Categories

(Web Compatibility :: Desktop, defect, major)

defect
Not set
major

Tracking

(Not tracked)

RESOLVED DUPLICATE of bug 652186

People

(Reporter: dave532, Unassigned)

References

()

Details

(Whiteboard: [contactready] [url] [country-uk])

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
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
Severity: normal → major
Whiteboard: [bug248549notfixed]
Blocks: 124594
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).
Summary: Sainsbury's bank online redirects to invalid URL syntax → sainsburysbank.co.uk - redirect to invalid URL syntax
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)
Called them, left contact details which will apparently be passed on to a suitable person.
(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..)
Patrick, would this be a netlib issue, and has it been considered?
Flags: needinfo?(mcmanus)
404
Mozilla/5.0 (Windows NT 5.1; rv:23.0) Gecko/20100101 Firefox/23.0
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → WORKSFORME
Whiteboard: [bug248549notfixed]
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
Assignee: english-other → hsteen
Status: REOPENED → ASSIGNED
Whiteboard: [sitewait]
(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..
Flags: needinfo?(mcmanus)
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
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?
Flags: needinfo?(mcmanus)
comment 10 is better directed to a docshell peer..
Flags: needinfo?(mcmanus) → needinfo?(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.
Flags: needinfo?(bzbarsky)
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.
> 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
(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..?
> 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.
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.)
Assignee: hsteen → nobody
Status: ASSIGNED → NEW
Component: English Other → Desktop
Depends on: 652186
Still the case. 
This is working in Opera (Blink), but not in Safari and Firefox latest versions.
Switching to contactready again given we didn't get an answer or action. The site needs to be recontacted.
Whiteboard: [sitewait] → [contactready] [url]
Whiteboard: [contactready] [url] → [contactready] [url] [country-uk]
Seems a fix for bug 652186 is underway. Let's stop spending time on this, it will soon work anyway..
Status: NEW → RESOLVED
Closed: 6 years ago4 years ago
Resolution: --- → INVALID
INVALID is probably wrong, it already works in Nightly (fix landed 4 days ago).
Resolution: INVALID → FIXED
But FIXED for an outreach bug means "the site fixed it", not "we fixed it in core"..
Resolution: FIXED → DUPLICATE
Duplicate of bug: 652186
OK, either is better than invalid.
Product: Tech Evangelism → Web Compatibility
You need to log in before you can comment on or make changes to this bug.