8000 Http Issue 201: Incorrect relative redirect location for URL with emp… · robccan/google-http-java-client@72aa772 · GitHub
[go: up one dir, main page]
Skip to content

Commit 72aa772

Browse files
peleyalpeleyal
committed
Http Issue 201: Incorrect relative redirect location for URL with empty path
https://codereview.appspot.com/7393060/
1 parent 371d2fc commit 72aa772

File tree

3 files changed

+156
-16
lines changed

3 files changed

+156
-16
lines changed

‎google-http-client/src/main/java/com/google/api/client/http/GenericUrl.java

Lines changed: 79 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,10 @@
2121
import com.google.api.client.util.escape.Escaper;
2222
import com.google.api.client.util.escape.PercentEscaper;
2323

24+
import java.net.MalformedURLException;
2425
import java.net.URI;
2526
import java.net.URISyntaxException;
27+
import java.net.URL;
2628
import java.util.ArrayList;
2729
import java.util.Collection;
2830
import java.util.Collections;
@@ -105,17 +107,48 @@ public GenericUrl(String encodedUrl) {
105107
}
106108

107109
/**
110+
* Constructs from a URI.
111+
*
108112
* @param uri URI
109113
*
110114
* @since 1.14
111115
*/
112116
public GenericUrl(URI uri) {
113-
scheme = uri.getScheme().toLowerCase();
114-
host = uri.getHost();
115-
port = uri.getPort();
116-
pathParts = toPathParts(uri.getRawPath());
117-
fragment = uri.getFragment();
118-
String query = uri.getRawQuery();
117+
this(uri.getScheme(),
118+
uri.getHost(),
119+
uri.getPort(),
120+
uri.getRawPath(),
121+
uri.getFragment(),
122+
uri.getRawQuery());
123+
}
124+
125+
/**
126+
* Constructs from a URL.
127+
*
128+
* @param url URL
129+
*
130+
* @since 1.14
131+
*/
132+
public GenericUrl(URL url) {
133+
this(url.getProtocol(),
134+
url.getHost(),
135+
url.getPort(),
136+
url.getPath(),
137+
url.getRef(),
138+
url.getQuery());
139+
}
140+
141+
private GenericUrl(String scheme,
142+
String host,
143+
int port,
144+
String path,
145+
String fragment,
146+
String query) {
147+
this.scheme = scheme.toLowerCase();
148+
this.host = host;
149+
this.port = port;
150+
this.pathParts = toPathParts(path);
151+
this.fragment = fragment;
119152
if (query != null) {
120153
UrlEncodedParser.parse(query, this);
121154
}
@@ -335,6 +368,46 @@ public final URI toURI() {
335368
return toURI(build());
336369
}
337370

371+
/**
372+
* Constructs the URL based on the string representation of the URL from {@link #build()}.
373+
*
374+
* <p>
375+
* Any {@link MalformedURLException} is wrapped in an {@link IllegalArgumentException}.
376+
* </p>
377+
*
378+
* @return new URL instance
379+
*
380+
* @since 1.14
381+
*/
382+
public final URL toURL() {
383+
try {
384+
return new URL(build());
385+
} catch (MalformedURLException e) {
386+
throw new IllegalArgumentException(e);
387+
}
388+
}
389+
390+
/**
391+
* Constructs the URL based on {@link URL#URL(URL, String)} with this URL representation from
392+
* {@link #toURL()} and a relative url.
393+
*
394+
* <p>
395+
* Any {@link MalformedURLException} is wrapped in an {@link IllegalArgumentException}.
396+
* </p>
397+
*
398+
* @return new URL instance
399+
*
400+
* @since 1.14
401+
*/
402+
public final URL toURL(String relativeUrl) {
403+
try {
404+
URL url = toURL();
405+
return new URL(url, relativeUrl);
406+
} catch (MalformedURLException e) {
407+
throw new IllegalArgumentException(e);
408+
}
409+
}
410+
338411
/**
339412
* Returns the first query parameter value for the given query parameter name.
340413
*

‎google-http-client/src/main/java/com/google/api/client/http/HttpRequest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1227,7 +1227,7 @@ public boolean handleRedirect(int statusCode, HttpHeaders responseHeaders) {
12271227
if (getFollowRedirects() && HttpStatusCodes.isRedirect(statusCode)
12281228
&& redirectLocation != null) {
12291229
// resolve the redirect location relative to the current location
1230-
setUrl(new GenericUrl(url.toURI().resolve(redirectLocation)));
1230+
setUrl(new GenericUrl(url.toURL(redirectLocation)));
12311231
// on 303 change method to GET
12321232
if (statusCode == HttpStatusCodes.STATUS_CODE_SEE_OTHER) {
12331233
setRequestMethod(HttpMethods.GET);

‎google-http-client/src/test/java/com/google/api/client/http/GenericUrlTest.java

Lines changed: 76 additions & 9 deletions
< 3BCE tr class="diff-line-row">
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,10 @@
1919
import junit.framework.TestCase;
2020
import org.junit.Assert;
2121

22+
import java.net.MalformedURLException;
23+
import java.net.URI;
24+
import java.net.URISyntaxException;
25+
import java.net.URL;
2226
import java.util.ArrayList;
2327
import java.util.Arrays;
2428
import java.util.Collection;
@@ -150,8 +154,8 @@ public TestUrl(String encodedUrl) {
150154

151155
private static String FULL =
152156
"https://www.google.com:223/m8/feeds/contacts/someone=%23%25&%20%3F%3Co%3E%7B%7D@gmail.com/"
153-
+ "full?" + "foo=bar&" + "alt=json&" + "max-results=3&" + "prettyprint=true&"
154-
+ "q=Go%3D%23/%25%26%20?%3Co%3Egle";
157+
+ "full?" + "foo=bar&" + "alt=json&" + "max-results=3&" + "prettyprint=true&"
158+
+ "q=Go%3D%23/%25%26%20?%3Co%3Egle#DOWNLOADING";
155159

156160
private static List<String> FULL_PARTS =
157161
Arrays.asList("", "m8", "feeds", "contacts", "someone=#%& ?<o>{}@gmail.com", "full");
@@ -162,14 +166,47 @@ public void testBuild_full() {
162166
url.setHost("www.google.com");
163167
url.setPort(223);
164168
url.setPathParts(FULL_PARTS);
165-
url.set("alt", "json").set("max-results", 3).set("prettyprint", true).set("q", "Go=#/%& ?<o>gle");
169+
url.set("alt", "json")
170+
.set("max-results", 3).set("prettyprint", true).set("q", "Go=#/%& ?<o>gle");
166171
url.foo = "bar";
167172
url.hidden = "invisible";
173+
url.setFragment("DOWNLOADING");
168174
assertEquals(FULL, url.build());
169175
}
170176

171177
public void testParse_full() {
172178
TestUrl url = new TestUrl(FULL);
179+
subtestFull(url);
180+
assertNull(url.hidden);
181+
assertEquals("bar", url.get("foo"));
182+
assertEquals("bar", url.foo);
183+
}
184+
185+
public void testConstructor_url() throws MalformedURLException {
186+
GenericUrl url = new GenericUrl(new URL(FULL));
187+
subtestFull(url);
188+
}
189+
190+
public void testConstructor_uri() throws URISyntaxException {
191+
GenericUrl url = new GenericUrl(new URI(FULL));
192+
subtestFull(url);
193+
}
194+
195+
public void testConstructor_string() {
196+
GenericUrl url = new GenericUrl(FULL);
197+
subtestFull(url);
198+
}
199+
200+
public void testConstructor_schemeToLowerCase() throws URISyntaxException, MalformedURLException {
201+
GenericUrl url = new GenericUrl("HTTps://www.google.com:223");
202+
assertEquals("https", url.getScheme());
203+
url = new GenericUrl(new URI("HTTPS://www.google.com:223"));
204+
assertEquals("https", url.getScheme());
205+
url = new GenericUrl(new URL("hTTPs://www.google.com:223"));
206+
assertEquals("https", url.getScheme());
207+
}
208+
209+
private void subtestFull(GenericUrl url) {
173210
assertEquals("https", url.getScheme());
174211
assertEquals("www.google.com", url.getHost());
175212
assertEquals(223, url.getPort());
@@ -178,10 +215,8 @@ public void testParse_full() {
178215
assertEquals("3", url.getFirst("max-results"));
179216
assertEquals("true", url.getFirst("prettyprint"));
180217
assertEquals("Go=#/%& ?<o>gle", url.getFirst("q"));
181-
assertNull(url.hidden);
182-
assertEquals("bar", url.foo);
183-
assertEquals("bar", url.get("foo"));
184218
assertEquals("bar", url.getFirst("foo"));
219+
assertEquals("DOWNLOADING", url.getFragment());
185220
}
186221

187222
public static class FieldTypesUrl extends GenericUrl {
@@ -303,7 +338,8 @@ public void testBuildAuthority_exception() {
303338
try {
304339
url.buildAuthority();
305340
Assert.fail("no exception was thrown");
306-
} catch (NullPointerException expected) {}
341+
} catch (NullPointerException expected) {
342+
}
307343

308344
// Test without a host.
309345
url = new GenericUrl();
@@ -312,7 +348,8 @@ public void testBuildAuthority_exception() {
312348
try {
313349
url.buildAuthority();
314350
Assert.fail("no exception was thrown");
315-
} catch (NullPointerException expected) {}
351+
} catch (NullPointerException expected) {
352+
}
316353
}
317354

318355
public void testBuildAuthority_simple() {
@@ -377,7 +414,7 @@ public void testBuildRelativeUrl_onlyQuery() {
377414
private static final String FULL_PATH = "/some/path/someone%2Fis%2F@gmail.com/test/?one=1&two=2";
378415

379416
public void testBuildRelativeUrl_full() {
380-
GenericUrl url = new GenericUrl(BASE_URL+FULL_PATH);
417+
GenericUrl url = new GenericUrl(BASE_URL + FULL_PATH);
381418
assertEquals(FULL_PATH, url.buildRelativeUrl());
382419
}
383420

@@ -497,4 +534,34 @@ public void testClone() {
497534
GenericUrl clone = url.clone();
498535
assertEquals("http://www.google.com", clone.build());
499536
}
537+
538+
public void testToUrl_relative() {
539+
// relative redirect
540+
testRedirectUtility("http://www.google.com/test", "http://www.google.com", "/test");
541+
testRedirectUtility("http://www.google.com/test", "http://www.google.com/foo/bar/", "/test");
542+
543+
testRedirectUtility("http://www.google.com/test", "http://www.google.com", "test");
544+
testRedirectUtility("http://www.google.com/test", "http://www.google.com/foo", "test");
545+
testRedirectUtility("http://www.google.com/foo/test", "http://www.google.com/foo/", "test");
546+
testRedirectUtility("http://www.google.com/foo/test", "http://www.google.com/foo/bar", "test");
547+
548+
testRedirectUtility(
549+
"http://www.google.com/foo/test/", "http://www.google.com/foo/bar", "test/");
550+
testRedirectUtility(
551+
"http://www.google.com/foo/test/sub", "http://www.google.com/foo/bar", "test/sub");
552+
553+
// absolute redirect
554+
testRedirectUtility(
555+
"https://example.com/test", "http://www.google.com/foo", "https://example.com/test");
556+
testRedirectUtility(
557+
"https://example.com/test", "http://www.google.com", "https://example.com/test");
558+
testRedirectUtility(
559+
"https://example.com/test", "http://www.google.com/", "https://example.com/test");
560+
}
561+
562+
private void testRedirectUtility(String expectedResult, String url, String relative) {
563+
GenericUrl gu = new GenericUrl(url);
564+
GenericUrl redirectedUrl = new GenericUrl(gu.toURL(relative));
565+
assertEquals(expectedResult, redirectedUrl.toString());
566+
}
500567
}

0 commit comments

Comments
 (0)
0