From 909383fea6ba30ec7a8a03437bf4e70fec474323 Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Mon, 8 Feb 2021 15:56:01 -0500 Subject: [PATCH 1/3] - fixes a bug where chunk encoding would make the redirect handler fail --- .../microsoft/graph/httpcore/HttpClients.java | 1 + .../graph/httpcore/RedirectHandler.java | 55 ++++++++++--------- 2 files changed, 29 insertions(+), 27 deletions(-) diff --git a/src/main/java/com/microsoft/graph/httpcore/HttpClients.java b/src/main/java/com/microsoft/graph/httpcore/HttpClients.java index e6bea9a0..93e13c92 100644 --- a/src/main/java/com/microsoft/graph/httpcore/HttpClients.java +++ b/src/main/java/com/microsoft/graph/httpcore/HttpClients.java @@ -22,6 +22,7 @@ public static Builder custom() { return new OkHttpClient.Builder() .addInterceptor(new TelemetryHandler()) .followRedirects(false) + .followSslRedirects(false) .protocols(Arrays.asList(Protocol.HTTP_1_1)); //https://stackoverflow.com/questions/62031298/sockettimeout-on-java-11-but-not-on-java-8 } diff --git a/src/main/java/com/microsoft/graph/httpcore/RedirectHandler.java b/src/main/java/com/microsoft/graph/httpcore/RedirectHandler.java index b04d774c..b9fc8a62 100644 --- a/src/main/java/com/microsoft/graph/httpcore/RedirectHandler.java +++ b/src/main/java/com/microsoft/graph/httpcore/RedirectHandler.java @@ -19,18 +19,18 @@ import okhttp3.Response; public class RedirectHandler implements Interceptor{ - + public final MiddlewareType MIDDLEWARE_TYPE = MiddlewareType.REDIRECT; - + private RedirectOptions mRedirectOptions; - + /* * Initialize using default redirect options, default IShouldRedirect and max redirect value */ public RedirectHandler() { this(null); } - + /* * @param redirectOptions pass instance of redirect options to be used */ @@ -40,16 +40,16 @@ public RedirectHandler(RedirectOptions redirectOptions) { this.mRedirectOptions = new RedirectOptions(); } } - + boolean isRedirected(Request request, Response response, int redirectCount, RedirectOptions redirectOptions) throws IOException { // Check max count of redirects reached if(redirectCount > redirectOptions.maxRedirects()) return false; - + // Location header empty then don't redirect final String locationHeader = response.header("location"); if(locationHeader == null) return false; - + // If any of 301,302,303,307,308 then redirect final int statusCode = response.code(); if(statusCode == HTTP_PERM_REDIRECT || //308 @@ -58,16 +58,16 @@ boolean isRedirected(Request request, Response response, int redirectCount, Redi statusCode == HTTP_SEE_OTHER || //303 statusCode == HTTP_MOVED_TEMP) //302 return true; - + return false; } - + Request getRedirect( final Request request, - final Response userResponse) throws ProtocolException { + final Response userResponse) throws ProtocolException { String location = userResponse.header("Location"); if (location == null || location.length() == 0) return null; - + // For relative URL in location header, the new url to redirect is relative to original request if(location.startsWith("/")) { if(request.url().toString().endsWith("/")) { @@ -75,14 +75,14 @@ Request getRedirect( } location = request.url() + location; } - + HttpUrl requestUrl = userResponse.request().url(); - + HttpUrl locationUrl = userResponse.request().url().resolve(location); - + // Don't follow redirects to unsupported protocols. if (locationUrl == null) return null; - + // Most redirects don't include a request body. Request.Builder requestBuilder = userResponse.request().newBuilder(); @@ -94,7 +94,7 @@ Request getRedirect( if (!sameScheme || !sameHost) { requestBuilder.removeHeader("Authorization"); } - + // Response status code 303 See Other then POST changes to GET if(userResponse.code() == HTTP_SEE_OTHER) { requestBuilder.method("GET", null); @@ -107,29 +107,30 @@ Request getRedirect( @Override public Response intercept(Chain chain) throws IOException { Request request = chain.request(); - + if(request.tag(TelemetryOptions.class) == null) request = request.newBuilder().tag(TelemetryOptions.class, new TelemetryOptions()).build(); request.tag(TelemetryOptions.class).setFeatureUsage(TelemetryOptions.REDIRECT_HANDLER_ENABLED_FLAG); - + Response response = null; int requestsCount = 1; - + // Use should retry pass along with this request RedirectOptions redirectOptions = request.tag(RedirectOptions.class); redirectOptions = redirectOptions != null ? redirectOptions : this.mRedirectOptions; - + while(true) { response = chain.proceed(request); - boolean shouldRedirect = isRedirected(request, response, requestsCount, redirectOptions) + final boolean shouldRedirect = isRedirected(request, response, requestsCount, redirectOptions) && redirectOptions.shouldRedirect().shouldRedirect(response); if(!shouldRedirect) break; - - Request followup = getRedirect(request, response); - if(followup == null) break; - request = followup; - - requestsCount++; + + final Request followup = getRedirect(request, response); + if(followup != null) { + response.close(); + request = followup; + requestsCount++; + } } return response; } From b4793a515ae3be99497c347930e81a5df6727043 Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Mon, 8 Feb 2021 16:00:55 -0500 Subject: [PATCH 2/3] - fixes a bug where the retry handler could fail on chunked encoding responses --- .../graph/httpcore/RetryHandler.java | 62 ++++++++++--------- 1 file changed, 32 insertions(+), 30 deletions(-) diff --git a/src/main/java/com/microsoft/graph/httpcore/RetryHandler.java b/src/main/java/com/microsoft/graph/httpcore/RetryHandler.java index 9478060e..6ba5900c 100644 --- a/src/main/java/com/microsoft/graph/httpcore/RetryHandler.java +++ b/src/main/java/com/microsoft/graph/httpcore/RetryHandler.java @@ -12,11 +12,11 @@ import okhttp3.Response; public class RetryHandler implements Interceptor{ - + public final MiddlewareType MIDDLEWARE_TYPE = MiddlewareType.RETRY; private RetryOptions mRetryOption; - + /* * constant string being used */ @@ -26,13 +26,13 @@ public class RetryHandler implements Interceptor{ private final String TRANSFER_ENCODING_CHUNKED = "chunked"; private final String APPLICATION_OCTET_STREAM = "application/octet-stream"; private final String CONTENT_TYPE = "Content-Type"; - + public static final int MSClientErrorCodeTooManyRequests = 429; public static final int MSClientErrorCodeServiceUnavailable = 503; public static final int MSClientErrorCodeGatewayTimeout = 504; - + private final long DELAY_MILLISECONDS = 1000; - + /* * @retryOption Create Retry handler using retry option */ @@ -48,28 +48,28 @@ public RetryHandler(RetryOptions retryOption) { public RetryHandler() { this(null); } - + boolean retryRequest(Response response, int executionCount, Request request, RetryOptions retryOptions) { - + // Should retry option // Use should retry common for all requests IShouldRetry shouldRetryCallback = null; if(retryOptions != null) { shouldRetryCallback = retryOptions.shouldRetry(); } - + boolean shouldRetry = false; // Status codes 429 503 504 int statusCode = response.code(); - // Only requests with payloads that are buffered/rewindable are supported. - // Payloads with forward only streams will be have the responses returned + // Only requests with payloads that are buffered/rewindable are supported. + // Payloads with forward only streams will be have the responses returned // without any retry attempt. - shouldRetry = - (executionCount <= retryOptions.maxRetries()) + shouldRetry = + (executionCount <= retryOptions.maxRetries()) && checkStatus(statusCode) && isBuffered(response, request) - && shouldRetryCallback != null + && shouldRetryCallback != null && shouldRetryCallback.shouldRetry(retryOptions.delay(), executionCount, request, response); - + if(shouldRetry) { long retryInterval = getRetryAfter(response, retryOptions.delay(), executionCount); try { @@ -80,7 +80,7 @@ && checkStatus(statusCode) && isBuffered(response, request) } return shouldRetry; } - + /** * Get retry after in milliseconds * @param response Response @@ -100,28 +100,28 @@ long getRetryAfter(Response response, long delay, int executionCount) { } return (long)Math.min(retryDelay, RetryOptions.MAX_DELAY * DELAY_MILLISECONDS); } - + boolean checkStatus(int statusCode) { - return (statusCode == MSClientErrorCodeTooManyRequests || statusCode == MSClientErrorCodeServiceUnavailable + return (statusCode == MSClientErrorCodeTooManyRequests || statusCode == MSClientErrorCodeServiceUnavailable || statusCode == MSClientErrorCodeGatewayTimeout); } - + boolean isBuffered(Response response, Request request) { String methodName = request.method(); - if(methodName.equalsIgnoreCase("GET") || methodName.equalsIgnoreCase("DELETE") || methodName.equalsIgnoreCase("HEAD") || methodName.equalsIgnoreCase("OPTIONS")) + if(methodName.equalsIgnoreCase("GET") || methodName.equalsIgnoreCase("DELETE") || methodName.equalsIgnoreCase("HEAD") || methodName.equalsIgnoreCase("OPTIONS")) return true; - + boolean isHTTPMethodPutPatchOrPost = methodName.equalsIgnoreCase("POST") || methodName.equalsIgnoreCase("PUT") || methodName.equalsIgnoreCase("PATCH"); - + if(isHTTPMethodPutPatchOrPost) { boolean isStream = response.header(CONTENT_TYPE)!=null && response.header(CONTENT_TYPE).equalsIgnoreCase(APPLICATION_OCTET_STREAM); if(!isStream) { String transferEncoding = response.header(TRANSFER_ENCODING); - boolean isTransferEncodingChunked = (transferEncoding != null) && + boolean isTransferEncodingChunked = (transferEncoding != null) && transferEncoding.equalsIgnoreCase(TRANSFER_ENCODING_CHUNKED); - + if(request.body() != null && isTransferEncodingChunked) return true; } @@ -132,24 +132,26 @@ boolean isBuffered(Response response, Request request) { @Override public Response intercept(Chain chain) throws IOException { Request request = chain.request(); - + if(request.tag(TelemetryOptions.class) == null) request = request.newBuilder().tag(TelemetryOptions.class, new TelemetryOptions()).build(); request.tag(TelemetryOptions.class).setFeatureUsage(TelemetryOptions.RETRY_HANDLER_ENABLED_FLAG); - + Response response = chain.proceed(request); - + // Use should retry pass along with this request RetryOptions retryOption = request.tag(RetryOptions.class); retryOption = retryOption != null ? retryOption : mRetryOption; - + int executionCount = 1; while(retryRequest(response, executionCount, request, retryOption)) { request = request.newBuilder().addHeader(RETRY_ATTEMPT_HEADER, String.valueOf(executionCount)).build(); executionCount++; - if(response != null && response.body() != null) { - response.body().close(); - } + if(response != null) { + if(response.body() != null) + response.body().close(); + response.close(); + } response = chain.proceed(request); } return response; From 6d87a3e74e3e1444ff0b3dd0e91cf2a6c20dc090 Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Tue, 9 Feb 2021 09:10:40 -0500 Subject: [PATCH 3/3] - patch version bump --- gradle.properties | 2 +- readme.md | 4 ++-- .../java/com/microsoft/graph/httpcore/TelemetryHandler.java | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/gradle.properties b/gradle.properties index 8defcc2e..0c6e07e5 100644 --- a/gradle.properties +++ b/gradle.properties @@ -25,7 +25,7 @@ mavenGroupId = com.microsoft.graph mavenArtifactId = microsoft-graph-core mavenMajorVersion = 1 mavenMinorVersion = 0 -mavenPatchVersion = 7 +mavenPatchVersion = 8 mavenArtifactSuffix = nightliesUrl = http://dl.bintray.com/MicrosoftGraph/Maven diff --git a/readme.md b/readme.md index 44aa829e..a3a305ad 100644 --- a/readme.md +++ b/readme.md @@ -20,7 +20,7 @@ repositories { dependencies { // Include the sdk as a dependency - implementation 'com.microsoft.graph:microsoft-graph-core:1.0.7' + implementation 'com.microsoft.graph:microsoft-graph-core:1.0.8' } ``` @@ -32,7 +32,7 @@ Add the dependency in `dependencies` in pom.xml com.microsoft.graph microsoft-graph-core - 1.0.7 + 1.0.8 ``` diff --git a/src/main/java/com/microsoft/graph/httpcore/TelemetryHandler.java b/src/main/java/com/microsoft/graph/httpcore/TelemetryHandler.java index 8e6d2899..3af01bf6 100644 --- a/src/main/java/com/microsoft/graph/httpcore/TelemetryHandler.java +++ b/src/main/java/com/microsoft/graph/httpcore/TelemetryHandler.java @@ -12,7 +12,7 @@ public class TelemetryHandler implements Interceptor{ public static final String SDK_VERSION = "SdkVersion"; - public static final String VERSION = "v1.0.7"; + public static final String VERSION = "v1.0.8"; public static final String GRAPH_VERSION_PREFIX = "graph-java-core"; public static final String JAVA_VERSION_PREFIX = "java"; public static final String ANDROID_VERSION_PREFIX = "android";