193728 - data: Base64-encoded URIs don't support %-encoded characters

Status: RESOLVED FIXED

(Core :: Networking, defect, P2)

Description

[Hixie (not reading bugmail)]

If a data: URI contains a %-encoded character and is base64 encoded, then it
plays dead, not doing anything, not even giving error messages. For example:

data:text/plain;base64,UEFTUw%3D%3D
data:text/plain;base64,%56%47%56%7A%64%43%42%6F%59%58%4D%67%63%47%46%7A%63%32%56%6B

Comment 1

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: patch

Comment 2

[benc]

let me guess, we don't unescape the URL before we start processing it.

I thought I had written a testcase for this, but maybe I didn't do it right.

Thanks for the bug, hixie.

Comment 3

[Christian :Biesinger (don't email me, ping me on IRC)]

benc: That is true but only happens if the url is base64-encoded.
With my patch, we always unescape.

Comment 4

[Bradley Baetz (:bbaetz)]

This bug is invalid. RFC2397 says:

   The appearance of ";base64" means that the data
   is encoded as base64. Without ";base64", the data (as a sequence of
   octets) is represented using ASCII encoding for octets inside the
   range of safe URL characters and using the standard %xx hex encoding
   of URLs for octets outside that range. 

which implies that the code is correct. Does ns4 behave differently? This makes
sense; with base64, we can have % in the url, but no 'forbidden' characters.
Actually, can we have % in the url? If not, then this is OK. I can't recall the
base64 spec, though.

(According to the spec, the ";base64" bit should also be at teh beginning)

Comment 5

[Christian :Biesinger (don't email me, ping me on IRC)]

ok, netscape 4 loads none of the urls from this bug. it just does nothing when I
try to load it (sounds familiar? :) )

so is this INVALID?

Comment 6

[Christian :Biesinger (don't email me, ping me on IRC)]

(I tested version 4.8 on linux)

Comment 7

[Christian :Biesinger (don't email me, ping me on IRC)]

sigh. this came up once, already:
http://bugzilla.mozilla.org/show_bug.cgi?id=160221#c3

I knew that this sounded familiar...

Comment 8

[Andreas Otte]

I think we should not unescape data uris in base 64 encoding, we should also not
encode them with url-encoding. Those data urls have their own escaping rules we
should not touch them. 

Should this example urls work?

Comment 9

[benc]

data       := *urlchar

in RFC 2396, the term is actually "uric"...

uric          = reserved | unreserved | escaped

I don't think anything that is corrrectly base64 encoded needs to be escaped
(that's the point of using base64 encoding right?), but there is always the case
that someone could gratuitously encode characters that do not need encoding.

RFC 2396:

   Normally, the only time
   escape encodings can safely be made is when the URI is being created
   from its component parts; each component may have its own set of
   characters that are reserved, so only the mechanism responsible for
   generating or interpreting that component can determine whether or
   not escaping a character will change its semantics. Likewise, a URI
   must be separated into its components before the escaped characters
   within those components can be safely decoded.

   In some cases, data that could be represented by an unreserved
   character may appear escaped; for example, some of the unreserved
   "mark" characters are automatically escaped by some systems.  If the
   given URI scheme defines a canonicalization algorithm, then
   unreserved characters may be unescaped according to that algorithm.
   For example, "%7e" is sometimes used instead of "~" in an http URL
   path, but the two are equivalent for an http URL.

The problem, to me, is that the data: URL RFC does not clearly state if there
are reserved characters or not. 

Base64 includes "+/=", which are potentially considered "reserved"

If the data for a URI component would conflict with the
   reserved purpose, then the conflicting data must be escaped before
   forming the URI.

      reserved    = ";" | "/" | "?" | ":" | "@" | "&" | "=" | "+" |
                    "$" | ","

   The "reserved" syntax class above refers to those characters that are
   allowed within a URI, but which may not be allowed within a
   particular component of the generic URI syntax; they are used as
   delimiters of the components described in Section 3.

   Characters in the "reserved" set are not reserved in all contexts.
   The set of characters actually reserved within any given URI
   component is defined by that component. In general, a character is
   reserved if the semantics of the URI changes if the character is
   replaced with its escaped US-ASCII encoding.

So, I think it is possible someone who did not look at these two RFC's carefully
would escape some octets in their URLs.

If you want to interpret the data: RFC as having zero reserved characters
because none are explicitly reserved, then we probably do not ned to unescape in
base64.

Comment 10

[Andreas Otte]

The data url is a simple one, beyond "data:" there are no more special url
characters, no need to do any escaping regarding that.

Comment 11

[Darin Fisher]

Comment on attachment 114695 [details] [diff] [review]
patch

if the RFC says not to try to un-URL-escape the base64 encoded data: URL, then
why do we want to bother?  just for increased compatibility?  huh?  data: URLs
are not very common (because IE doesn't support them), so  why don't we just
keep our code lean here?  stick to the standard.

my vote is INVALID or WONTFIX.

Comment 12

[Hixie (not reading bugmail)]

This isn't invalid. The spec is clear that the data: URI content is "urlchar"
regardless of whether the data is base64 encoded or not.

This should be fixed because it is perfectly legitimate to over-escape "uric"
characters in URIs, in order to maximise the likelyhood that the data will pass
through correctly, and the spec allows this.

Comment 13

[Andreas Otte]

I took a look at base64 encoding and it does not include a %, so there is no
danger of destroying the data while unescaping. I now have to agree with hixi,
lets unescape in every case, we can not destroy anything, but possibly fix urls
that got escaped when they shouldn't have.

Comment 14

[Bradley Baetz (:bbaetz)]

Comment on attachment 114695 [details] [diff] [review]
patch

darin, was there a final decision on this?

Comment 15

[Christian :Biesinger (don't email me, ping me on IRC)]

darin: per comment 13, could you maybe reconsider your superreview-?

Comment 16

[Darin Fisher]

Comment on attachment 114695 [details] [diff] [review]
patch

sr=darin (sorry, didn't catch any of the last bugmails... been so behind... but
you guys definitely convinced me!)

Comment 17

[Bradley Baetz (:bbaetz)]

Comment on attachment 114695 [details] [diff] [review]
patch

oops, sorry - I missed darin's sr/moa on this.

Comment 18

[Christian :Biesinger (don't email me, ping me on IRC)]

Checking in netwerk/protocol/data/src/nsDataChannel.cpp;
/cvsroot/mozilla/netwerk/protocol/data/src/nsDataChannel.cpp,v  <-- 
nsDataChannel.cpp
new revision: 1.64; previous revision: 1.63
done

Comment 19

[benc]

are the provided links supposed to work now?

I'm using 20030617, 1.4, Mach-O, and it doesn't work for me.

Comment 20

[Christian :Biesinger (don't email me, ping me on IRC)]

benc: Well, they are supposed to work; however, the patch has not been checked
in on the 1.4 branch, so this won't work in 1.4.

Comment 21

[benc]

good. I haven't added this to the tests I'm running for 1.4 branch anyhow.

sorry for the confusion on my part.