Re: URI-escape cookies (addresses external issue 4365)

[kpr...@google.com]

All done, thanks. Committed at r7354.

On 2009/12/27 03:29:04, Dan Rice wrote:
> LGTM with minor nits.

> http://gwt-code-reviews.appspot.com/128801/diff/1/3
> File user/src/com/google/gwt/user/client/Cookies.java (right):

> http://gwt-code-reviews.appspot.com/128801/diff/1/3#newcode112
> Line 112: if (uriEncoding) {
> Prettier and less duplicated code to do:

> if (uriEncoding) {
> name = uriEncode(name);
> }
> removeCookieNative(name);

> http://gwt-code-reviews.appspot.com/128801/diff/1/3#newcode115
> Line 115: else {
> } else {

> on same line

> http://gwt-code-reviews.appspot.com/128801/diff/1/3#newcode128
> Line 128: if (uriEncoding) {
> Ditto

> http://gwt-code-reviews.appspot.com/128801/diff/1/3#newcode184
> Line 184: throw new IllegalArgumentException("Illegal cookie
format.");
> Include name and value in exception message, maybe distinguish between
bad name
> and bad value to make debugging easier.

> http://gwt-code-reviews.appspot.com/128801/diff/1/2
> File user/test/com/google/gwt/user/client/CookieTest.java (right):

> http://gwt-code-reviews.appspot.com/128801/diff/1/2#newcode166
> Line 166: // Make sure cookie values are URI encoded
> Probably best to add 'Cookies.setUriEncode(true);' here (redundantly)
to avoid
> dependency between sections of this method.

http://gwt-code-reviews.appspot.com/128801