Skip to content

Commit

Permalink
Enable requestBody to read buffer as duplicate. Also update OkHttpCli…
Browse files Browse the repository at this point in the history
…ent to not follow redirects by default. (#27960)

OkHttpClient by default has redirectfollow set to true, which makes it difficult for us to invoke our own redirect policies. Updated that. I expect this not to be a breaking change since any SDK would have had to add redirect policy for Netty client which would mean OkHttp would continue to work for them. There is a small chance that an SDK may only support OkHttpClient and so may get affected however the right fix would be to update the redirect policy there. Let me know if anyone disagrees.
  • Loading branch information
pallavit authored Mar 31, 2022
1 parent 283246f commit 0789536
Show file tree
Hide file tree
Showing 3 changed files with 70 additions and 0 deletions.
10 changes: 10 additions & 0 deletions sdk/core/azure-core-http-okhttp/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,19 @@
## 1.8.0-beta.1 (Unreleased)

### Features Added
- Added `followRedirects` property on the `OkHttpClientBuilder`.

### Breaking Changes

- Okhttp-backed `HttpClient` client will no longer follow redirects automatically. ([#27960](https://github.com/Azure/azure-sdk-for-java/pull/27960)).
<br>To get the older behavior please create an instance of `HttpClient` as follows

```java
HttpClient client = new OkHttpAsyncHttpClientBuilder()
.followRedirects(true)
.build();
```

### Bugs Fixed

### Other Changes
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ public class OkHttpAsyncHttpClientBuilder {
private Dispatcher dispatcher;
private ProxyOptions proxyOptions;
private Configuration configuration;
private boolean followRedirects;

/**
* Creates OkHttpAsyncHttpClientBuilder.
Expand Down Expand Up @@ -212,6 +213,20 @@ public OkHttpAsyncHttpClientBuilder configuration(Configuration configuration) {
return this;
}

/**
* <p>Sets the followRedirect flag on the underlying OkHttp-backed {@link com.azure.core.http.HttpClient}.</p>
*
* <p>If this is set to 'true' redirects will be followed automatically, and
* if your HTTP pipeline is configured with a redirect policy it will not be called.</p>
*
* @param followRedirects The followRedirects value to use.
* @return The updated OkHttpAsyncHttpClientBuilder object.
*/
public OkHttpAsyncHttpClientBuilder followRedirects(boolean followRedirects) {
this.followRedirects = followRedirects;
return this;
}

/**
* Creates a new OkHttp-backed {@link com.azure.core.http.HttpClient} instance on every call, using the
* configuration set in the builder at the time of the build method call.
Expand Down Expand Up @@ -267,6 +282,9 @@ public HttpClient build() {
}
}

// Set the followRedirects property.
httpClientBuilder.followRedirects(this.followRedirects);

return new OkHttpAsyncHttpClient(httpClientBuilder.build());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ public class OkHttpAsyncHttpClientBuilderTests {
private static final String COOKIE_VALIDATOR_PATH = "/cookieValidator";
private static final String DEFAULT_PATH = "/default";
private static final String DISPATCHER_PATH = "/dispatcher";
private static final String REDIRECT_PATH = "/redirect";
private static final String LOCATION_PATH = "/location";

private static final String JAVA_SYSTEM_PROXY_PREREQUISITE = "java.net.useSystemProxies";
private static final String JAVA_NON_PROXY_HOSTS = "http.nonProxyHosts";
Expand All @@ -63,6 +65,8 @@ public class OkHttpAsyncHttpClientBuilderTests {
private static String cookieValidatorUrl;
private static String defaultUrl;
private static String dispatcherUrl;
private static String locationUrl;
private static String redirectUrl;

@BeforeAll
public static void setupWireMock() {
Expand All @@ -84,6 +88,12 @@ public static void setupWireMock() {
cookieValidatorUrl = "http://localhost:" + server.port() + COOKIE_VALIDATOR_PATH;
defaultUrl = "http://localhost:" + server.port() + DEFAULT_PATH;
dispatcherUrl = "http://localhost:" + server.port() + DISPATCHER_PATH;
redirectUrl = "http://localhost:" + server.port() + REDIRECT_PATH;
locationUrl = "http://localhost:" + server.port() + LOCATION_PATH;

// Mocked endpoint to test the redirect behavior.
server.stubFor(WireMock.get(REDIRECT_PATH).willReturn(WireMock.aResponse().withStatus(307).withHeader("Location", locationUrl)));
server.stubFor(WireMock.get(LOCATION_PATH).willReturn(WireMock.aResponse().withStatus(200)));
}

@AfterAll
Expand Down Expand Up @@ -195,6 +205,38 @@ public void buildWithConnectionTimeout() {
.verifyComplete();
}


@Test
public void buildWithFollowRedirectSetToTrue() {
HttpClient okClient = new OkHttpAsyncHttpClientBuilder()
.followRedirects(true)
.build();

StepVerifier.create(okClient.send(new HttpRequest(HttpMethod.GET, redirectUrl)))
.assertNext(response -> assertEquals(200, response.getStatusCode()))
.verifyComplete();
}

@Test
public void buildWithFollowRedirectSetToFalse() {
HttpClient okClient = new OkHttpAsyncHttpClientBuilder()
.followRedirects(false)
.build();

StepVerifier.create(okClient.send(new HttpRequest(HttpMethod.GET, redirectUrl)))
.assertNext(response -> assertEquals(307, response.getStatusCode()))
.verifyComplete();
}

@Test
public void buildWithFollowRedirectDefault() {
HttpClient okClient = new OkHttpAsyncHttpClientBuilder().build();

StepVerifier.create(okClient.send(new HttpRequest(HttpMethod.GET, redirectUrl)))
.assertNext(response -> assertEquals(307, response.getStatusCode()))
.verifyComplete();
}

/**
* Tests building a client with a given {@code connectionTimeout}.
*/
Expand Down

0 comments on commit 0789536

Please sign in to comment.