New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 829873 link

Starred by 2 users
Status: Available
Owner: ----
Cc:
█ xunji...@chromium.org
mmenke@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Make UnescapeURLWithAdjustmentsImpl use icu

Project Member Reported by mmenke@chromium.org, Apr 6 2018 
UnescapeURLWithAdjustmentsImpl has rules based on characters belonging to certain unicode classes.  Unfortunately, the code can't depend on icu currently, since net/ needs to work with and without icu enabled, so has to have its own character tables.  We should look into resolving this - we can make a special version of the method that unescapes everything unconditionally for net/ consumers that want that behavior (data URLs), and see if those that need it for display can either be made not to use it (Like QUIC logging code), or be excluded from the binary for non-icu builds (Like FTP directory listings, which Cronet probably doesn't need).

Comment 1 by mmenke@chromium.org, Apr 6 2018 

The interesting places UnescapeURLWithAdjustmentsImpl is used in net/:

* QueryIterator (Only used by embedded_test_server)
* filename_util (Used in file URLs, and by URLRequestMockHttpJob, possibly elsewhere as well)
* http_content_disposition (Not used in net/, looks like)
* FtpNetworkTranaction
* embedded_test_server (unescapes everything)
* data_url (unescapes everything)

Of those, the QueryIterator and filename_util look most problematic, as http_content_disposition  and FtpNetworkTranaction can be excluded when icu alternative are enabled, I assume, and we can unescape everything without icu.  Only 5 downloads browser_tests and a nacl test depend on the use of QueryIterator in embedded test server, so we could likely rework things there.

There doesn't look to be a platform android alternative for this method.  We could also just not unescape UTF-8 in Cronet, and while that may work, I'm concerned that subtle behavior differences would go unnoticed.

Comment 2 by mmenke@chromium.org, Apr 17 2018

Cc: mmenke@chromium.org
Labels: -Pri-2 Pri-3
Owner: ----
Status: Available (was: Assigned)
I'm going to punt on this for now - spent too much time on this as-is.

Comment 3 by mmenke@chromium.org, Apr 18 2018

Labels: Network-Triaged
►

Sign in to add a comment