"headerless" http for ad banner does not load

RESOLVED FIXED in mozilla0.9.9

Status

()

P3
normal
RESOLVED FIXED
17 years ago
17 years ago

People

(Reporter: jrgmorrison, Assigned: darin.moz)

Tracking

Trunk
mozilla0.9.9
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

17 years ago
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').
(Reporter)

Comment 1

17 years ago
Erp.

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

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

Comment 2

17 years ago
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
Status: NEW → ASSIGNED
Priority: -- → P3
Target Milestone: --- → mozilla0.9.6
(Assignee)

Updated

17 years ago
Target Milestone: mozilla0.9.6 → mozilla0.9.7
(Assignee)

Comment 3

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

-> 0.9.9 for now
Target Milestone: mozilla0.9.7 → mozilla0.9.9
(Assignee)

Comment 4

17 years ago
Created attachment 67882 [details] [diff] [review]
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.
(Assignee)

Comment 5

17 years ago
Created attachment 67890 [details] [diff] [review]
v1.0 patch (really!)
Attachment #67882 - Attachment is obsolete: true
(Assignee)

Comment 6

17 years ago
bbaetz: can you r= this patch?
(Assignee)

Comment 7

17 years ago
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 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?
Attachment #67890 - Flags: review+
(Assignee)

Comment 9

17 years ago
bbaetz: perhaps that would be better... hmm...
(Assignee)

Comment 10

17 years ago
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").
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

17 years ago
Yes it does. Thank you.



Comment 13

17 years ago
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=/;
 
(Assignee)

Comment 14

17 years ago
vinay: ok, i'll take a look at that.  thx for testing this patch!
(Assignee)

Comment 15

17 years ago
i verified that this patch fixes the testcases at:

  http://jrgm/bugs/72382/page.html
(Assignee)

Comment 16

17 years ago
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.
(Assignee)

Comment 17

17 years ago
*** Bug 120628 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 18

17 years ago
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

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

sr=mscott
Attachment #67890 - Flags: superreview+
(Assignee)

Comment 20

17 years ago
fixed-on-trunk
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.