The default bug view has changed. See this FAQ.

Limelight networks logo (data: URL) fails to load because it contains errors

VERIFIED FIXED in mozilla17

Status

()

Core
General
--
critical
VERIFIED FIXED
5 years ago
5 years ago

People

(Reporter: stephend, Assigned: Julian Reschke)

Tracking

({regression})

Trunk
mozilla17
regression
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox17-)

Details

(URL)

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

5 years ago
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"
Component: General → General
Keywords: regressionwindow-wanted
Product: Firefox → Core

Comment 1

5 years ago
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
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?
Blocks: 774240
Keywords: regressionwindow-wanted → regression
(Assignee)

Comment 3

5 years ago
(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.
Version: unspecified → Trunk
(Assignee)

Comment 4

5 years ago
Test case: http://greenbytes.de/tech/tc/datauri/#base64extwrongpos

So this is an invalid URI, but the regression was unintentional.
(Assignee)

Updated

5 years ago
Assignee: nobody → julian.reschke
(Assignee)

Comment 5

5 years ago
Created attachment 653169 [details] [diff] [review]
Proposed patch, incl test case
Attachment #653169 - Flags: review?(bzbarsky)
(Assignee)

Comment 6

5 years ago
Created attachment 653170 [details] [diff] [review]
Proposed patch, incl test case
Attachment #653169 - Attachment is obsolete: true
Attachment #653169 - Flags: review?(bzbarsky)
Attachment #653170 - Flags: review?(bzbarsky)
(Assignee)

Comment 7

5 years ago
Comment on attachment 653170 [details] [diff] [review]
Proposed patch, incl test case

Try results: https://tbpl.mozilla.org/?tree=Try&rev=029800a06435
Comment on attachment 653170 [details] [diff] [review]
Proposed patch, incl test case

r=me
Attachment #653170 - Flags: review?(bzbarsky) → review+
tracking-firefox17: --- → ?
(Assignee)

Updated

5 years ago
Keywords: checkin-needed
Green on Try.
https://tbpl.mozilla.org/?tree=Try&rev=029800a06435

https://hg.mozilla.org/integration/mozilla-inbound/rev/5f0e24568cbb
Flags: in-testsuite+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/5f0e24568cbb
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
(Reporter)

Comment 11

5 years ago
Verified FIXED; thanks!

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

Updated

5 years ago
tracking-firefox17: ? → -
You need to log in before you can comment on or make changes to this bug.