google / volley

https://google.github.io/volley
Apache License 2.0
3.37k stars 751 forks source link

Handle http 3xx redirects out of the box #460

Closed iyosifov closed 1 month ago

iyosifov commented 1 month ago

https://github.com/google/volley/blame/master/core/src/main/java/com/android/volley/toolbox/NetworkUtility.java#L198

It appears that http 3xx redirects are not handled, rather they're returned as ServerError to the calling code. Based on the comment, this is intentional, but is there a good reason? Seems the redirect should just be followed instead.

Found this during internal testing of changes to a rest end point and an attempt to make it return a 3xx redirect for compatibility with old clients in the field.

jpd236 commented 1 month ago

This logic actually serves a slightly different purpose - whether to allow the request to be retried as is in the event of a failure. If a 3xx response reaches this point, there's no reason to expect that subsequent requests to the same URI would have a different result.

Volley delegates the responsibility of following 3xx redirects down to the underlying HTTP stack. For example, if an app is using the default HurlStack, calling HttpURLConnection.setFollowRedirects(true) should cause it to follow redirects internally, which means 3xx responses would never actually reach Volley's higher-level logic. This can also be done on a per-request basis without impacting the rest of the app by overriding HurlStack#createConnection and calling setInstanceFollowRedirects on the connection that super(url) returns.

iyosifov commented 1 month ago

Thank you for the information. I filed the bug as such, because I observe the exception originating from the NetworkUtility code. Our app code looks like this:

Volley.newRequestQueue(context).add(
                JsonObjectRequest(
                    Request.Method.POST,
                    .....
                ).setShouldCache(false))

I observe the value of HttpURLConnection.getFollowRedirects() being true before this is executed. Volley then (eventually) calls into HurlStack#createConnection, which sets connection.setInstanceFollowRedirects(HttpURLConnection.getFollowRedirects() == true) but apparently it's still not working.

Am I missing something obvious?

jpd236 commented 1 month ago

I suspect this is just tied to the behavior of HttpURLConnection rather than something specific to Volley.

For example, I believe it won't follow redirects if the protocol changes (e.g. from HTTP to HTTPS or vice versa) or if the redirect URL is malformed.