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

Status: VERIFIED FIXED

(Core :: General, defect)

Description

[Stephen Donner [:stephend] Not actively reading bugmail]

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]

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 [:bzbarsky]]

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]

(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]

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

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

Comment 5

[Julian Reschke]

Attachment: Proposed patch, incl test case

Comment 6

[Julian Reschke]

Attachment: Proposed patch, incl test case

Comment 7

[Julian Reschke]

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 [:bzbarsky]]

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

r=me

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Green on Try.
https://tbpl.mozilla.org/?tree=Try&rev=029800a06435

https://hg.mozilla.org/integration/mozilla-inbound/rev/5f0e24568cbb

Comment 10

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/5f0e24568cbb

Comment 11

[Stephen Donner [:stephend] Not actively reading bugmail]

Verified FIXED; thanks!

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