http issue 182: URL inline authentication removed

http issue 182: URL inline authentication removed

[yanivi, 2013-04-11 03:30:55]

https://codereview.appspot.com/8586043/diff/1/google-http-client/src/main/jav...
File google-http-client/src/main/java/com/google/api/client/http/GenericUrl.java
(right):

https://codereview.appspot.com/8586043/diff/1/google-http-client/src/main/jav...
google-http-client/src/main/java/com/google/api/client/http/GenericUrl.java:59:
public class GenericUrl extends GenericData {
add a warning in the JavaDoc with respect to the change in behavior from 1.14
about not dropping the userinfo from the URL.

https://codereview.appspot.com/8586043/diff/1/google-http-client/src/main/jav...
google-http-client/src/main/java/com/google/api/client/http/GenericUrl.java:348:
buf.append(userInfo + "@");
buf.append(userInfo).append("@") for minor efficiency gain

https://codereview.appspot.com/8586043/diff/1/google-http-client/src/main/jav...
google-http-client/src/main/java/com/google/api/client/http/GenericUrl.java:348:
buf.append(userInfo + "@");
what are the character encoding rules for userinfo, e.g. if username has a ':'
or if password has a '@'?  how about arbitrary UTF-8 characters?  see for
example how we handle path and query params.

https://codereview.appspot.com/8586043/diff/1/google-http-client/src/test/jav...
File
google-http-client/src/test/java/com/google/api/client/http/GenericUrlTest.java
(right):

https://codereview.appspot.com/8586043/diff/1/google-http-client/src/test/jav...
google-http-client/src/test/java/com/google/api/client/http/GenericUrlTest.java:221:
assertEquals("user:password2", url.getUserInfo());
add a case where getUserInfo() is null

[peleyal, 2013-04-11 18:08:13]

https://codereview.appspot.com/8586043/diff/1/google-http-client/src/main/jav...
File google-http-client/src/main/java/com/google/api/client/http/GenericUrl.java
(right):

https://codereview.appspot.com/8586043/diff/1/google-http-client/src/main/jav...
google-http-client/src/main/java/com/google/api/client/http/GenericUrl.java:59:
public class GenericUrl extends GenericData {
On 2013/04/11 03:30:55, yanivi wrote:
> add a warning in the JavaDoc with respect to the change in behavior from 1.14
> about not dropping the userinfo from the URL.

Done.

https://codereview.appspot.com/8586043/diff/1/google-http-client/src/main/jav...
google-http-client/src/main/java/com/google/api/client/http/GenericUrl.java:348:
buf.append(userInfo + "@");
On 2013/04/11 03:30:55, yanivi wrote:
> buf.append(userInfo).append("@") for minor efficiency gain

Done.

https://codereview.appspot.com/8586043/diff/1/google-http-client/src/main/jav...
google-http-client/src/main/java/com/google/api/client/http/GenericUrl.java:348:
buf.append(userInfo + "@");
Done.
I added decode on constructor and escapeUriUserInfo(new!) in buildAuthority.

On 2013/04/11 03:30:55, yanivi wrote:
> what are the character encoding rules for userinfo, e.g. if username has a ':'
> or if password has a '@'?  how about arbitrary UTF-8 characters?  see for
> example how we handle path and query params.

https://codereview.appspot.com/8586043/diff/1/google-http-client/src/test/jav...
File
google-http-client/src/test/java/com/google/api/client/http/GenericUrlTest.java
(right):

https://codereview.appspot.com/8586043/diff/1/google-http-client/src/test/jav...
google-http-client/src/test/java/com/google/api/client/http/GenericUrlTest.java:221:
assertEquals("user:password2", url.getUserInfo());
On 2013/04/11 03:30:55, yanivi wrote:
> add a case where getUserInfo() is null

Done.

[yanivi, 2013-04-11 21:03:57]

https://codereview.appspot.com/8586043/diff/10002/google-http-client/src/main...
File google-http-client/src/main/java/com/google/api/client/http/GenericUrl.java
(right):

https://codereview.appspot.com/8586043/diff/10002/google-http-client/src/main...
google-http-client/src/main/java/com/google/api/client/http/GenericUrl.java:53:
* Upgrade warning: in prior version 1.14 user info part in the URL was dropped,
but starting with
insert "the" before "user info"

https://codereview.appspot.com/8586043/diff/10002/google-http-client/src/main...
google-http-client/src/main/java/com/google/api/client/http/GenericUrl.java:130:
uri.getFragment(),
getRawFragment() because we want the encoded form here

https://codereview.appspot.com/8586043/diff/10002/google-http-client/src/main...
google-http-client/src/main/java/com/google/api/client/http/GenericUrl.java:132:
uri.getUserInfo());
similarly getRawUserInfo()

https://codereview.appspot.com/8586043/diff/10002/google-http-client/src/main...
google-http-client/src/main/java/com/google/api/client/http/GenericUrl.java:163:
this.fragment = fragment;
need to decode the fragment?

https://codereview.appspot.com/8586043/diff/10002/google-http-client/src/main...
File
google-http-client/src/main/java/com/google/api/client/util/escape/CharEscapers.java
(right):

https://codereview.appspot.com/8586043/diff/10002/google-http-client/src/main...
google-http-client/src/main/java/com/google/api/client/util/escape/CharEscapers.java:126:
* escaping URIs, see section 2.4 of <a
href="http://www.ietf.org/rfc/rfc3986.txt">RFC 3986</a>.
http://tools.ietf.org/html/rfc3986#section-2.4

https://codereview.appspot.com/8586043/diff/10002/google-http-client/src/main...
File
google-http-client/src/main/java/com/google/api/client/util/escape/PercentEscaper.java
(right):

https://codereview.appspot.com/8586043/diff/10002/google-http-client/src/main...
google-http-client/src/main/java/com/google/api/client/util/escape/PercentEscaper.java:24:
* href="http://www.ietf.org/rfc/rfc3986.txt">RFC 3986</a>.
and http://tools.ietf.org/html/rfc3986#appendix-A

[peleyal, 2013-04-12 13:14:07]

https://codereview.appspot.com/8586043/diff/10002/google-http-client/src/main...
File google-http-client/src/main/java/com/google/api/client/http/GenericUrl.java
(right):

https://codereview.appspot.com/8586043/diff/10002/google-http-client/src/main...
google-http-client/src/main/java/com/google/api/client/http/GenericUrl.java:53:
* Upgrade warning: in prior version 1.14 user info part in the URL was dropped,
but starting with
On 2013/04/11 21:03:57, yanivi wrote:
> insert "the" before "user info"

Done.

https://codereview.appspot.com/8586043/diff/10002/google-http-client/src/main...
google-http-client/src/main/java/com/google/api/client/http/GenericUrl.java:130:
uri.getFragment(),
On 2013/04/11 21:03:57, yanivi wrote:
> getRawFragment() because we want the encoded form here

Done.

https://codereview.appspot.com/8586043/diff/10002/google-http-client/src/main...
google-http-client/src/main/java/com/google/api/client/http/GenericUrl.java:132:
uri.getUserInfo());
On 2013/04/11 21:03:57, yanivi wrote:
> similarly getRawUserInfo()

Done.

https://codereview.appspot.com/8586043/diff/10002/google-http-client/src/main...
google-http-client/src/main/java/com/google/api/client/http/GenericUrl.java:163:
this.fragment = fragment;
On 2013/04/11 21:03:57, yanivi wrote:
> need to decode the fragment?

Done.

https://codereview.appspot.com/8586043/diff/10002/google-http-client/src/main...
File
google-http-client/src/main/java/com/google/api/client/util/escape/CharEscapers.java
(right):

https://codereview.appspot.com/8586043/diff/10002/google-http-client/src/main...
google-http-client/src/main/java/com/google/api/client/util/escape/CharEscapers.java:126:
* escaping URIs, see section 2.4 of <a
href="http://www.ietf.org/rfc/rfc3986.txt">RFC 3986</a>.
On 2013/04/11 21:03:57, yanivi wrote:
> http://tools.ietf.org/html/rfc3986#section-2.4

Done (here and in other places as well)

https://codereview.appspot.com/8586043/diff/10002/google-http-client/src/main...
File
google-http-client/src/main/java/com/google/api/client/util/escape/PercentEscaper.java
(right):

https://codereview.appspot.com/8586043/diff/10002/google-http-client/src/main...
google-http-client/src/main/java/com/google/api/client/util/escape/PercentEscaper.java:24:
* href="http://www.ietf.org/rfc/rfc3986.txt">RFC 3986</a>.
On 2013/04/11 21:03:57, yanivi wrote:
> and http://tools.ietf.org/html/rfc3986#appendix-A

Done.

[yanivi, 2013-04-12 13:47:27]

LGTM

https://codereview.appspot.com/8586043/diff/27001/google-http-client/src/test...
File
google-http-client/src/test/java/com/google/api/client/http/GenericUrlTest.java
(right):

https://codereview.appspot.com/8586043/diff/27001/google-http-client/src/test...
google-http-client/src/test/java/com/google/api/client/http/GenericUrlTest.java:161:
"https://user:password2@www.google.com:223/m8/feeds/contacts/someone=%23%25&%20%3F%3Co%3E%7B%7D@gmail.com/"
please add some %-encoded characters into userinfo and fragment, e.g. with @
symbol

https://codereview.appspot.com/8586043/diff/27001/google-http-client/src/test...
google-http-client/src/test/java/com/google/api/client/http/GenericUrlTest.java:179:
url.setUserInfo("user:password2");
make a USER_INFO constant so don't have to repeat it twice below

[peleyal, 2013-04-12 15:24:10]

Done.
I'm going to push this to repository now.

https://codereview.appspot.com/8586043/diff/27001/google-http-client/src/test...
File
google-http-client/src/test/java/com/google/api/client/http/GenericUrlTest.java
(right):

https://codereview.appspot.com/8586043/diff/27001/google-http-client/src/test...
google-http-client/src/test/java/com/google/api/client/http/GenericUrlTest.java:161:
"https://user:password2@www.google.com:223/m8/feeds/contacts/someone=%23%25&%20%3F%3Co%3E%7B%7D@gmail.com/"
On 2013/04/12 13:47:27, yanivi wrote:
> please add some %-encoded characters into userinfo and fragment, e.g. with @
> symbol

Done.

https://codereview.appspot.com/8586043/diff/27001/google-http-client/src/test...
google-http-client/src/test/java/com/google/api/client/http/GenericUrlTest.java:179:
url.setUserInfo("user:password2");
On 2013/04/12 13:47:27, yanivi wrote:
> make a USER_INFO constant so don't have to repeat it twice below

Done. (and also for fragment)