[Cronet] Handle redirects in CronetHttpURLConnection

components/cronet/android/java/src/org/chromium/net/urlconnection/CronetHttpURLConnection.java

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 package org.chromium.net.urlconnection;
 
 import org.chromium.net.ExtendedResponseInfo;
 import org.chromium.net.ResponseInfo;
 import org.chromium.net.UrlRequest;
 import org.chromium.net.UrlRequestContext;
 import org.chromium.net.UrlRequestException;
 import org.chromium.net.UrlRequestListener;
 
 import java.io.IOException;
 import java.io.InputStream;
 import java.net.HttpURLConnection;
+import java.net.MalformedURLException;
 import java.net.URL;
 import java.nio.ByteBuffer;
 
 /**
  * An implementation of HttpURLConnection that uses Cronet to send requests and
  * receive response. This class inherits a {@code connected} field from the
  * superclass. That field indicates whether a connection has ever been
  * attempted.
  */
 public class CronetHttpURLConnection extends HttpURLConnection {
@@ skipping 73 matching lines @@
         return mResponseInfo.getHttpStatusCode();
     }
 
     /**
      * Returns an InputStream for reading data from the resource pointed by this
      * URLConnection.
      */
     @Override
     public InputStream getInputStream() throws IOException {
         connect();
+        if (mResponseInfo == null) {
+            throw new IOException();
+        }
+        if (!instanceFollowRedirects
+                && mResponseInfo.getHttpStatusCode() == 302) {

[mmenke, 2014/12/10 21:44:52]

301, 303, 307, and 308 may also be redirects (308s may not always be redirects,
though, as some Google services use them in weird, non-HTTP compliant manners,
and the response bodies of those can actually be read).

Best to just not explicitly check the response code.  Should instead check
whether onRedirect was called while instanceFollowRedirects was false.

[xunjieli, 2014/12/12 20:58:02]

Done. Smart idea! thanks

+            throw new IOException(
+                    "Cannot read response body of a 302 response");
+        }
         return mInputStream;
     }
 
     /**
      * Adds the given property to the request header.
      */
     @Override
     public final void addRequestProperty(String key, String value) {
         // Note that Cronet right now does not allow setting multiple headers
         // of the same key, see crbug.com/432719 for more details.
@@ skipping 49 matching lines @@
             mResponseInfo = info;
             mResponseByteBuffer = ByteBuffer.allocate(byteBuffer.capacity());
             mResponseByteBuffer.put(byteBuffer);
             mResponseByteBuffer.flip();
             mMessageLoop.postQuitTask();
         }
 
         @Override
         public void onRedirect(UrlRequest request, ResponseInfo info,
                 String newLocationUrl) {
-            // TODO(xunjieli): Handle redirect and test it.
+            if (instanceFollowRedirects) {
+                try {
+                    url = new URL(newLocationUrl);
+                } catch (MalformedURLException e) {

[xunjieli, 2014/12/10 20:27:17]

Not sure if we need to handle malformed url, I guess the request will fail
anyway if the new url is malformed. Thoughts?

[mef, 2014/12/11 16:30:56]

I agree.

[xunjieli, 2014/12/12 20:58:02]

Acknowledged. I will leave it unhandled, unless Matt has any objection.

+                    // Ignored.
+                }
+            } else {
+                mResponseInfo = info;
+                mRequest.cancel();
+                setResponseDataCompleted();

[mef, 2014/12/11 16:30:56]

Per Matt's suggestion maybe set a flag indicating that request was canceled due
to redirect?

[xunjieli, 2014/12/12 20:58:01]

Done.

+            }
         }
 
         @Override
         public void onSucceeded(UrlRequest request, ExtendedResponseInfo info) {
             mResponseInfo = info.getResponseInfo();
             setResponseDataCompleted();
         }
 
         @Override
         public void onFailed(UrlRequest request, ResponseInfo info,
                 UrlRequestException exception) {
             // TODO(xunjieli): Handle failure.
             setResponseDataCompleted();
         }
 
         /**
          * Notifies {@link #mInputStream} that transferring of response data has
          * completed.
          */
         private void setResponseDataCompleted() {
             if (mInputStream != null) {
                 mInputStream.setResponseDataCompleted();
             }
             mMessageLoop.postQuitTask();
         }
     }
 }