Allow data urls to have a fragment part.

Allow data urls to have a fragment or query part.

BUG=123004, 120248

[asanka, 2014-06-16 17:45:48]

The bug for this is https://code.google.com/p/chromium/issues/detail?id=123004.
I've assigned the bug to you. Could you run this through the Blink bots to make
sure we aren't breaking layout tests? If we are, then those tests will need to
be fixed before landing this change.

https://codereview.chromium.org/338503006/diff/20001/net/base/data_url.cc
File net/base/data_url.cc (right):

https://codereview.chromium.org/338503006/diff/20001/net/base/data_url.cc#new...
net/base/data_url.cc:27: if (!url.SchemeIs("data"))
Make this a DCHECK instead.

https://codereview.chromium.org/338503006/diff/20001/net/base/data_url_unitte...
File net/base/data_url_unittest.cc (right):

https://codereview.chromium.org/338503006/diff/20001/net/base/data_url_unitte...
net/base/data_url_unittest.cc:213: 
Add a test case for an percent escaped '#'.

https://codereview.chromium.org/338503006/diff/20001/net/base/data_url_unitte...
net/base/data_url_unittest.cc:213: 
Would you mind going through http://greenbytes.de/tech/tc/datauri/ and seeing if
there are any additional test cases we want to incorporate into this list?

[Erik Dahlström (inactive), 2014-06-17 13:10:21]

https://codereview.chromium.org/338503006/diff/20001/net/base/data_url.cc
File net/base/data_url.cc (right):

https://codereview.chromium.org/338503006/diff/20001/net/base/data_url.cc#new...
net/base/data_url.cc:27: if (!url.SchemeIs("data"))
On 2014/06/16 17:45:48, asanka wrote:
> Make this a DCHECK instead.

Done.

https://codereview.chromium.org/338503006/diff/20001/net/base/data_url_unitte...
File net/base/data_url_unittest.cc (right):

https://codereview.chromium.org/338503006/diff/20001/net/base/data_url_unitte...
net/base/data_url_unittest.cc:213: 
On 2014/06/16 17:45:48, asanka wrote:
> Add a test case for an percent escaped '#'.

Done.

https://codereview.chromium.org/338503006/diff/20001/net/base/data_url_unitte...
net/base/data_url_unittest.cc:213: 
On 2014/06/16 17:45:48, asanka wrote:
> Would you mind going through http://greenbytes.de/tech/tc/datauri/ and seeing
if
> there are any additional test cases we want to incorporate into this list?

Will do.

[asanka, 2014-08-13 15:41:41]

Whoa! Apologies if this was waiting for me. Feel free to ping in the future if
you don't hear back within a day or two.

https://codereview.chromium.org/338503006/diff/40001/net/base/data_url.cc
File net/base/data_url.cc (right):

https://codereview.chromium.org/338503006/diff/40001/net/base/data_url.cc#new...
net/base/data_url.cc:28: const std::string path = url.path();
Shall we try to avoid constructing strings? I believe that's why the previous
code was using the iterators over the spec.

i.e. begin = url.possibly_invalid_spec().begin() +
url.parsed_for_possibly_invalid_spec().path.begin;
   end = ....path.end();

You'd also want to DCHECK(url.is_valid()).

[Erik Dahlström (inactive), 2014-08-13 16:01:13]

On 2014/08/13 15:41:41, asanka wrote:
> Whoa! Apologies if this was waiting for me. Feel free to ping in the future if
> you don't hear back within a day or two.

Thanks, will do in the future :) 

I put this aside because of
http://code.google.com/p/chromium/issues/detail?id=123004#c26, do you have any
bright ideas for how to address that? I don't have that data, and having read
the thread on the whatwg list it seemed the proposal for specialized parsing
(essentially to tolerate unescaped # and ? for the cases where these don't mean
fragment or query) was shot down back then. I see no other good way to reconcile
the two. I do think it's a bug that fragments can't be used with data urls and
blobs in blink.

[asanka, 2014-08-13 16:17:40]

On 2014/08/13 16:01:13, Erik Dahlström wrote:
> On 2014/08/13 15:41:41, asanka wrote:
> > Whoa! Apologies if this was waiting for me. Feel free to ping in the future
if
> > you don't hear back within a day or two.
> 
> Thanks, will do in the future :) 
> 
> I put this aside because of
> http://code.google.com/p/chromium/issues/detail?id=123004#c26, do you have any
> bright ideas for how to address that? I don't have that data, and having read
> the thread on the whatwg list it seemed the proposal for specialized parsing
> (essentially to tolerate unescaped # and ? for the cases where these don't
mean
> fragment or query) was shot down back then. I see no other good way to
reconcile
> the two. I do think it's a bug that fragments can't be used with data urls and
> blobs in blink.

That's a valid concern.

One way to go about answering the question would be to collect some UMA data
(carefully) to see what the incidence rate is for unescaped # and ?. That would
reduce the uncertainty somewhat.

[Erik Dahlström (inactive), 2014-08-14 08:52:51]

On 2014/08/13 16:17:40, asanka wrote:
> On 2014/08/13 16:01:13, Erik Dahlström wrote:
> > On 2014/08/13 15:41:41, asanka wrote:
> > > Whoa! Apologies if this was waiting for me. Feel free to ping in the
future
> if
> > > you don't hear back within a day or two.
> > 
> > Thanks, will do in the future :) 
> > 
> > I put this aside because of
> > http://code.google.com/p/chromium/issues/detail?id=123004#c26, do you have
any
> > bright ideas for how to address that? I don't have that data, and having
read
> > the thread on the whatwg list it seemed the proposal for specialized parsing
> > (essentially to tolerate unescaped # and ? for the cases where these don't
> mean
> > fragment or query) was shot down back then. I see no other good way to
> reconcile
> > the two. I do think it's a bug that fragments can't be used with data urls
and
> > blobs in blink.
> 
> That's a valid concern.
> 
> One way to go about answering the question would be to collect some UMA data
> (carefully) to see what the incidence rate is for unescaped # and ?. That
would
> reduce the uncertainty somewhat.

UMA?

Adding some sort of UseCounter might be possible, but it would essentially have
to implement something like the parsing proposal in the WhatWG thread to be able
to count each of these cases properly. That would most likely incur a performace
penalty.