103757 - "headerless" http for ad banner does not load

Status: RESOLVED FIXED

(Core :: Networking: HTTP, defect, P3)

Description

[John Morrison]

From http://bugzilla.mozilla.org/show_bug.cgi?id=72382

  ------- Additional Comments From John Morrison 2001-10-07 22:45 -------
  
  Well this loads (mostly) fine in current trunk builds on Mac/linux/win32.
  
  There is one "problem": there are three iframes that serve up ad banners.
  They do not load in mozilla, but they do in Nav4.x. The problem is that
  these pages return nothing but the HTML for that URL; there is absolutely no
  header information preceding the HTML.
  
  Gonna mark this worksforme for the original problem, but I have one
  question.
  
  darin: is a headerless stream something we are trying to handle?
  
  
  ------- Additional Comments From Bradley Baetz 2001-10-08 08:26 -------
  
  HTTP/0.9 doesn't have any headers. We attempt to do content-type sniffing,
  but the html in ads tend to be low quality, and so its hard to detect. We
  should get most ads though, and that page loads for me in 0.9.4.
  
  
  ------- Additional Comments From Darin Fisher 2001-10-08 14:15 -------
  
  jrgm: ads change, and so if you notice an ad not loading, please save a copy
  of the HTML source... we can then add appropriate code to the sniffer to
  handle the ad.  thx!

---

So, for the test page at http://jrgm/bugs/72382/page.html, the contents of 
the three iframes on that page do not load, nor do those URL's load when
you click on the link at the bottom of the page.

I used a small program using perl LWP to masquerade as 'mozilla/5.0...', and
setting "Connection: Keep-Alive". It appears that those URL's are returning
the following as a response.  Note: this is the raw buffer that is returned on
the socket. ['\n' is used to note the character 0x10].

The response to, e.g., http://ads1.ad-flow.com/?DC=AMDzone-rst&TARGET=_blank
is (excuse the bugzilla-introduced linebreaks; the following should be a 
linebreak followed by a single line of HTML):

\n<a href="http://ads.ad-flow.com/?RC=32&AI=584&RANDOM=98382584418282" 
target="_blank"><img 
src="http://ads.ad-flow.com/?AI=584&SN=32&FN=web-crucial-1ID14-125-CPM&RANDOM=9
8382584418282&AN=Y" alt=" " height="125" width="125" border="0"></a>

In other words, '\n' followed by the HTML, with absolutely no other information
passed back. (The actual contents returned change with each call, but never 
have any headers whatsoever, just an initial '\n').

Comment 1

[John Morrison]

Erp.

> ['\n' is used to note the character 0x10].

['\n' is used to note the character 0x0A].

Comment 2

[Darin Fisher]

hmm.. nsUnknownDecoder.cpp does sniff for "<a href" ... i wonder why it's not
working in this case... maybe something to do with it being an iframe?!?

-> perhaps for 0.9.6

Comment 3

[Darin Fisher]

iirc, bbaetz mentioned that the leading \n is probably tripping up the
status/header line parsing somehow.

-> 0.9.9 for now

Comment 4

[Darin Fisher]

Attachment: v1.0 patch

was being too forgiving in the detection of 'HTTP' at the start of the
response...
detected 'http' portion of '<a href="http:' as start of status line.  allowing
for 'HTTP' within the first 8 bytes seems more reasonable than 32! ...what was
i thinking?!?  also, fixes a potential crash/dataloss bug.  count -= *countRead
is unnecessary/incorrect.

Comment 5

[Darin Fisher]

Attachment: v1.0 patch (really!)

Comment 6

[Darin Fisher]

bbaetz: can you r= this patch?

Comment 7

[Darin Fisher]

vinay: looks like this bug may be a dupe of bug 120628.  i've attached a
patch... mind taking a look and seeing if it solves bug 120628 as well?

Comment 8

[Bradley Baetz (:bbaetz)]

Comment on attachment 67890 [details] [diff] [review]
v1.0 patch (really!)

r=bbaetz

Why can't we just require that "HTTP" be the first non-whitespace?

Comment 9

[Darin Fisher]

bbaetz: perhaps that would be better... hmm...

Comment 10

[Darin Fisher]

actually, bbaetz: iirc, i've seen responses that look like '0 HTTP/1.1 200 OK'

the leading '0' is obviously wrong, but i suspect it is residual from improper
chunking in a persistent connection.  this problem may actually have been due to
problems on our end because i recall fixing a bug in the chunked decoder when
adding support for parsing trailers.  maybe it would be reasonable then to only
ignore whitespace (" \r\n\t").

Comment 11

[Bradley Baetz (:bbaetz)]

We're talking about a real edge case anyway, really. Can you check what ns4 did,
and we'll just copy it? That seems the most sensible way to go.

Comment 12

[Vinay Badami]

Yes it does. Thank you.

Comment 13

[Vinay Badami]

oops - take it back wrt  bug 120628
Prevents the crash. But now either the browser is till witing for data or
displays the http headers on the screen.
HTTP/1.1 200 OK Server: Microsoft-IIS/5.0 Date: Wed, 06 Feb 2002 10:33:31 GMT
Connection: close Content-type: text/html Page-Completion-Status: Normal
Page-Completion-Status: Normal Set-Cookie: LOCALITY_ID=1000; path=/; Set-Cookie:
SITE_ID=1022; path=/;
 

Comment 14

[Darin Fisher]

vinay: ok, i'll take a look at that.  thx for testing this patch!

Comment 15

[Darin Fisher]

i verified that this patch fixes the testcases at:

  http://jrgm/bugs/72382/page.html

Comment 16

[Darin Fisher]

this patch also seems to fix loading:

  https://www.enthusian.com/default.cfm?site_name=936&secure=1

which is the URL in bug 120628.

vinay: i cannot reproduce the problem you describe in comment #13.  i'm going to
go ahead and mark 120628 as a dupe of this bug.

Comment 17

[Darin Fisher]

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

Comment 18

[Darin Fisher]

bbaetz: i'm going to go ahead with this patch since it fixes the dupe'd crash
and solves this bug... later on i'll revisit this bug to see if we can't come up
with a smarter status line sniffer.

Comment 19

[Scott MacGregor]

Comment on attachment 67890 [details] [diff] [review]
v1.0 patch (really!)

sr=mscott

Comment 20

[Darin Fisher]

fixed-on-trunk