ChihYungHu / google-url

Automatically exported from code.google.com/p/google-url
Other
0 stars 0 forks source link

url_util.h has decoding functions but no encoding functions #24

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
In url_util.h, we have DecodeURLEscapeSequences, but no corresponding function 
to do the encoding.  I'd like to add an encoder that would be equivalent to 
encodeURIComponent [the global JS method].

This would initially be for the benefit of the Chromium project [in order to 
make KURLGoogle.cpp's EncodeWithURLEscapeSequences actually do what it says it 
does], but I think it makes sense in general for the library to have both 
directions available.

Original issue reported on code.google.com by ericu@chromium.org on 8 Jun 2011 at 9:14

GoogleCodeExporter commented 9 years ago
What's the bug you're trying to fix with this? Where in Chrome will we use this 
function. We have a whole lot of different escaping functions for different 
cases which cases will be handled by your proposed addition?

Original comment by bre...@google.com on 10 Jun 2011 at 2:08

GoogleCodeExporter commented 9 years ago
It involves a long chain of yak-shaving.

The original bug is http://code.google.com/p/chromium/issues/detail?id=78860.
That results from not properly escaping the path of a URL somewhere under 
WebKit/chromium/src [sorry, I'm away from my desk; if you want the actual file, 
I can get it for you later].
The right way to do escaping there seems to be to use KURL's 
EncodeWithURLEscapeSequences.  However, chromium's implementation of that 
function is basically a no-op as far as actual escape sequences go, due to a 
use of the function that no longer exists.  I'd like to fix that function, but 
since it's in WebKit, most of the obvious functions [such as those in 
chromium's escape.h] aren't accessible.  KURLGoogle.cpp uses other functions 
from googleurl to do its decoding, but the library doesn't expose encoding 
functions, so that's why I want to add this one.

Abarth@ and I discussed it for a while, and this looked cleaner than 
duplicating a lot of code from KURL.cpp or trying to factor out a bunch of 
hidden static table data into a shared header file for just KURL.cpp and 
KURLGoogle.cpp.  Plus having an encoder and a decoder in the same library seems 
like a win--it'll be easy to make sure they agree on things.

What do you think?

Original comment by ericu@chromium.org on 10 Jun 2011 at 2:19