Last Comment Bug 781693 - Limelight networks logo (data: URL) fails to load because it contains errors
: Limelight networks logo (data: URL) fails to load because it contains errors
Status: VERIFIED FIXED
: regression
Product: Core
Classification: Components
Component: General (show other bugs)
: Trunk
: All All
: -- critical (vote)
: mozilla17
Assigned To: Julian Reschke
:
Mentors:
http://www.limelight.com/
Depends on:
Blocks: 774240
  Show dependency treegraph
 
Reported: 2012-08-09 16:49 PDT by Stephen Donner [:stephend]
Modified: 2012-08-24 16:28 PDT (History)
9 users (show)
ryanvm: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
-


Attachments
Proposed patch, incl test case (2.88 KB, patch)
2012-08-19 05:35 PDT, Julian Reschke
no flags Details | Diff | Review
Proposed patch, incl test case (2.87 KB, patch)
2012-08-19 05:39 PDT, Julian Reschke
bzbarsky: review+
Details | Diff | Review

Description Stephen Donner [:stephend] 2012-08-09 16:49:53 PDT
Build identifier: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/17.0 Firefox/17.0

This works just fine in Firefox 14.

STR:

1. Load http://www.limelight.com/
2. Look at the logo

Expected Results:

Logo displays fine

Actual Results:

No logo in the upper-left side

"The image cannot be displayed because it contains errors"
Comment 1 Alice0775 White 2012-08-09 17:56:52 PDT
Regression window(m-c)
Good:
http://hg.mozilla.org/mozilla-central/rev/c918ff2f0994
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/17.0 Firefox/17.0 ID:20120716112421
Bad:
http://hg.mozilla.org/mozilla-central/rev/ba8463beab13
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/17.0 Firefox/17.0 ID:20120717020522
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=c918ff2f0994&tochange=ba8463beab13


Regression window(m-i)
Good:
http://hg.mozilla.org/integration/mozilla-inbound/rev/2958d924acc6
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/17.0 Firefox/17.0 ID:20120716172023
Bad:
http://hg.mozilla.org/integration/mozilla-inbound/rev/8d22bd6c2fe8
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/17.0 Firefox/17.0 ID:20120716173927
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=2958d924acc6&tochange=8d22bd6c2fe8
Comment 2 Boris Zbarsky [:bz] (Out June 25-July 6) 2012-08-09 18:34:46 PDT
It's an invalid data: URL.  It starts with:

  data:image/jpeg;base64;origUrl=http://www.limelight.com/assets/images/limelight-content-delivery.jpg,

This used to work because we treated this as a "Content-Type: image/jpeg;base64;origUrl=http://www.limelight.com/assets/images/limelight-content-delivery.jpg" which is perfectly reasonable for a Content-Type.  

But that behavior got changed in bug 774240, and now we look for the ',' right after the ";base64" bit to treat the data as base64.  And this so-called data: URI certainly doesn't have a ',' there.  So we don't treat this as base64 data, and then it fails, of course.

We could consider changing the check for isBase64 to look for ',' or ';', if that makes sense.  Julian?
Comment 3 Julian Reschke 2012-08-10 03:02:57 PDT
(In reply to Boris Zbarsky (:bz) [In and out Aug 1 - 10, out Aug 11-20] from comment #2)
> It's an invalid data: URL.  It starts with:
> 
>  
> data:image/jpeg;base64;origUrl=http://www.limelight.com/assets/images/
> limelight-content-delivery.jpg,
> 
> This used to work because we treated this as a "Content-Type:
> image/jpeg;base64;origUrl=http://www.limelight.com/assets/images/limelight-
> content-delivery.jpg" which is perfectly reasonable for a Content-Type.  

Almost; media types do not have value-less parameters.
 
> But that behavior got changed in bug 774240, and now we look for the ','
> right after the ";base64" bit to treat the data as base64.  And this
> so-called data: URI certainly doesn't have a ',' there.  So we don't treat
> this as base64 data, and then it fails, of course.
> 
> We could consider changing the check for isBase64 to look for ',' or ';', if
> that makes sense.  Julian?

Well, the point was to reject invalid URIs, as I thought that Chrome and Opera do that as well (see http://greenbytes.de/tech/tc/datauri/#base64param). Will have a look next week.
Comment 4 Julian Reschke 2012-08-19 02:39:35 PDT
Test case: http://greenbytes.de/tech/tc/datauri/#base64extwrongpos

So this is an invalid URI, but the regression was unintentional.
Comment 5 Julian Reschke 2012-08-19 05:35:54 PDT
Created attachment 653169 [details] [diff] [review]
Proposed patch, incl test case
Comment 6 Julian Reschke 2012-08-19 05:39:28 PDT
Created attachment 653170 [details] [diff] [review]
Proposed patch, incl test case
Comment 7 Julian Reschke 2012-08-19 05:58:03 PDT
Comment on attachment 653170 [details] [diff] [review]
Proposed patch, incl test case

Try results: https://tbpl.mozilla.org/?tree=Try&rev=029800a06435
Comment 8 Boris Zbarsky [:bz] (Out June 25-July 6) 2012-08-21 21:59:15 PDT
Comment on attachment 653170 [details] [diff] [review]
Proposed patch, incl test case

r=me
Comment 10 Ed Morley [:emorley] 2012-08-23 03:51:30 PDT
https://hg.mozilla.org/mozilla-central/rev/5f0e24568cbb
Comment 11 Stephen Donner [:stephend] 2012-08-24 08:36:44 PDT
Verified FIXED; thanks!

Build identifier: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:17.0) Gecko/17.0 Firefox/17.0

Note You need to log in before you can comment on or make changes to this bug.