Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(1454)

Issue 8586043: http issue 182: URL inline authentication removed (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
6 years, 5 months ago by peleyal
Modified:
6 years, 5 months ago
Reviewers:
yanivi
CC:
ngmiceli
Base URL:
https://code.google.com/p/google-http-java-client/
Visibility:
Public.

Description

http issue 182: URL inline authentication removed

Patch Set 1 #

Total comments: 8

Patch Set 2 : Yanivi comments + escapeUriUserInfo #

Patch Set 3 : minor #

Total comments: 12

Patch Set 4 : Yanivi comments 2 #

Patch Set 5 : change to ref http://tools.ietf.org/html/rfc3986 in all places #

Patch Set 6 : minor #

Total comments: 4

Patch Set 7 : before pushing #

Unified diffs Side-by-side diffs Delta from patch set Stats (+143 lines, -29 lines) Patch
M google-http-client/src/main/java/com/google/api/client/http/GenericUrl.java View 1 2 3 9 chunks +43 lines, -9 lines 0 comments Download
M google-http-client/src/main/java/com/google/api/client/util/escape/CharEscapers.java View 1 2 3 4 8 chunks +62 lines, -12 lines 0 comments Download
M google-http-client/src/main/java/com/google/api/client/util/escape/PercentEscaper.java View 1 2 3 4 5 3 chunks +15 lines, -4 lines 0 comments Download
M google-http-client/src/test/java/com/google/api/client/http/GenericUrlTest.java View 1 2 3 4 5 6 5 chunks +23 lines, -4 lines 0 comments Download

Messages

Total messages: 7
peleyal
6 years, 5 months ago (2013-04-09 17:32:29 UTC) #1
yanivi
https://codereview.appspot.com/8586043/diff/1/google-http-client/src/main/java/com/google/api/client/http/GenericUrl.java 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/java/com/google/api/client/http/GenericUrl.java#newcode59 google-http-client/src/main/java/com/google/api/client/http/GenericUrl.java:59: public class GenericUrl extends GenericData { add a warning ...
6 years, 5 months ago (2013-04-11 03:30:55 UTC) #2
peleyal
https://codereview.appspot.com/8586043/diff/1/google-http-client/src/main/java/com/google/api/client/http/GenericUrl.java 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/java/com/google/api/client/http/GenericUrl.java#newcode59 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, ...
6 years, 5 months ago (2013-04-11 18:08:13 UTC) #3
yanivi
https://codereview.appspot.com/8586043/diff/10002/google-http-client/src/main/java/com/google/api/client/http/GenericUrl.java 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/java/com/google/api/client/http/GenericUrl.java#newcode53 google-http-client/src/main/java/com/google/api/client/http/GenericUrl.java:53: * Upgrade warning: in prior version 1.14 user info ...
6 years, 5 months ago (2013-04-11 21:03:57 UTC) #4
peleyal
https://codereview.appspot.com/8586043/diff/10002/google-http-client/src/main/java/com/google/api/client/http/GenericUrl.java 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/java/com/google/api/client/http/GenericUrl.java#newcode53 google-http-client/src/main/java/com/google/api/client/http/GenericUrl.java:53: * Upgrade warning: in prior version 1.14 user info ...
6 years, 5 months ago (2013-04-12 13:14:07 UTC) #5
yanivi
LGTM https://codereview.appspot.com/8586043/diff/27001/google-http-client/src/test/java/com/google/api/client/http/GenericUrlTest.java 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/java/com/google/api/client/http/GenericUrlTest.java#newcode161 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 ...
6 years, 5 months ago (2013-04-12 13:47:27 UTC) #6
peleyal
6 years, 5 months ago (2013-04-12 15:24:10 UTC) #7
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)
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b