Commit 636afcd6 authored by Jaeyong Bae's avatar Jaeyong Bae Committed by Commit Bot

[Cronet] Prevent JavaUrlRequest crashes on HTTP 304

This patch means handling exception
when we got redirect header without location value from server.

Bug: 891738
Change-Id: I5fd781cbe94262123a08bcc185bf88f91027878b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1897375
Commit-Queue: Paul Jensen <pauljensen@chromium.org>
Reviewed-by: default avatarPaul Jensen <pauljensen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#721472}
parent 2ecc5c0e
...@@ -515,8 +515,11 @@ final class JavaUrlRequest extends UrlRequestBase { ...@@ -515,8 +515,11 @@ final class JavaUrlRequest extends UrlRequestBase {
Collections.unmodifiableList(headerList), false, selectedTransport, "", 0); Collections.unmodifiableList(headerList), false, selectedTransport, "", 0);
// TODO(clm) actual redirect handling? post -> get and whatnot? // TODO(clm) actual redirect handling? post -> get and whatnot?
if (responseCode >= 300 && responseCode < 400) { if (responseCode >= 300 && responseCode < 400) {
fireRedirectReceived(mUrlResponseInfo.getAllHeaders()); List<String> locationFields = mUrlResponseInfo.getAllHeaders().get("location");
return; if (locationFields != null) {
fireRedirectReceived(locationFields.get(0));
return;
}
} }
fireCloseUploadDataProvider(); fireCloseUploadDataProvider();
if (responseCode >= 400) { if (responseCode >= 400) {
...@@ -550,13 +553,11 @@ final class JavaUrlRequest extends UrlRequestBase { ...@@ -550,13 +553,11 @@ final class JavaUrlRequest extends UrlRequestBase {
} }
} }
private void fireRedirectReceived(final Map<String, List<String>> headerFields) { private void fireRedirectReceived(final String locationField) {
transitionStates(State.STARTED, State.REDIRECT_RECEIVED, new Runnable() { transitionStates(State.STARTED, State.REDIRECT_RECEIVED, new Runnable() {
@Override @Override
public void run() { public void run() {
mPendingRedirectUrl = URI.create(mCurrentUrl) mPendingRedirectUrl = URI.create(mCurrentUrl).resolve(locationField).toString();
.resolve(headerFields.get("location").get(0))
.toString();
mUrlChain.add(mPendingRedirectUrl); mUrlChain.add(mPendingRedirectUrl);
transitionStates( transitionStates(
State.REDIRECT_RECEIVED, State.AWAITING_FOLLOW_REDIRECT, new Runnable() { State.REDIRECT_RECEIVED, State.AWAITING_FOLLOW_REDIRECT, new Runnable() {
......
...@@ -311,6 +311,30 @@ public class CronetUrlRequestTest { ...@@ -311,6 +311,30 @@ public class CronetUrlRequestTest {
testSimpleGet(); testSimpleGet();
} }
/**
* Tests redirect without location header doesn't cause a crash.
*/
@Test
@SmallTest
@Feature({"Cronet"})
public void testRedirectWithNullLocationHeader() throws Exception {
String url = NativeTestServer.getFileURL("/redirect_broken_header.html");
TestUrlRequestCallback callback = new TestUrlRequestCallback();
UrlRequest.Builder builder = mTestFramework.mCronetEngine.newUrlRequestBuilder(
url, callback, callback.getExecutor());
final UrlRequest urlRequest = builder.build();
urlRequest.start();
callback.blockForDone();
assertEquals("<!DOCTYPE html>\n<html>\n<head>\n<title>Redirect</title>\n"
+ "<p>Redirecting...</p>\n</head>\n</html>\n",
callback.mResponseAsString);
assertEquals(ResponseStep.ON_SUCCEEDED, callback.mResponseStep);
assertEquals(302, callback.mResponseInfo.getHttpStatusCode());
assertNull(callback.mError);
assertFalse(callback.mOnErrorCalled);
}
/** /**
* Tests onRedirectReceived after cancel doesn't cause a crash. * Tests onRedirectReceived after cancel doesn't cause a crash.
*/ */
......
<!DOCTYPE html>
<html>
<head>
<title>Redirect</title>
<p>Redirecting...</p>
</head>
</html>
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment