From f5204b60fb87db53ac9198aea319049661bfee18 Mon Sep 17 00:00:00 2001 From: insaf021 Date: Wed, 1 Jul 2026 08:35:59 +0530 Subject: [PATCH] security: strip cookies on cross-origin redirects and fix response connection leaks --- .../google/api/client/http/HttpRequest.java | 54 ++++++++++-- .../api/client/http/HttpRequestTest.java | 86 +++++++++++++++++++ 2 files changed, 135 insertions(+), 5 deletions(-) diff --git a/google-http-client/src/main/java/com/google/api/client/http/HttpRequest.java b/google-http-client/src/main/java/com/google/api/client/http/HttpRequest.java index 78f15d868..50c30280c 100644 --- a/google-http-client/src/main/java/com/google/api/client/http/HttpRequest.java +++ b/google-http-client/src/main/java/com/google/api/client/http/HttpRequest.java @@ -29,6 +29,7 @@ import io.opencensus.trace.Tracer; import java.io.IOException; import java.io.InputStream; +import java.net.URL; import java.util.Properties; import java.util.concurrent.Callable; import java.util.concurrent.Executor; @@ -1022,10 +1023,17 @@ public HttpResponse execute() throws IOException { response = new HttpResponse(this, lowLevelHttpResponse); responseConstructed = true; } finally { - if (!responseConstructed) { - InputStream lowLevelContent = lowLevelHttpResponse.getContent(); - if (lowLevelContent != null) { - lowLevelContent.close(); + if (!responseConstructed && lowLevelHttpResponse != null) { + try { + InputStream lowLevelContent = lowLevelHttpResponse.getContent(); + if (lowLevelContent != null) { + lowLevelContent.close(); + } + } catch (IOException ignored) { + } + try { + lowLevelHttpResponse.disconnect(); + } catch (IOException ignored) { } } } @@ -1180,7 +1188,25 @@ public boolean handleRedirect(int statusCode, HttpHeaders responseHeaders) { && HttpStatusCodes.isRedirect(statusCode) && redirectLocation != null) { // resolve the redirect location relative to the current location - setUrl(new GenericUrl(url.toURL(redirectLocation), useRawRedirectUrls)); + URL newURL = url.toURL(redirectLocation); + + // Check if redirecting to a different origin + String oldScheme = url.getScheme(); + String oldHost = url.getHost(); + int oldPort = url.getPort(); + + String newScheme = newURL.getProtocol(); + String newHost = newURL.getHost(); + int newPort = newURL.getPort(); + + int oldEffectivePort = getEffectivePort(oldScheme, oldPort); + int newEffectivePort = getEffectivePort(newScheme, newPort); + + boolean sameOrigin = (oldScheme == null ? newScheme == null : oldScheme.equalsIgnoreCase(newScheme)) + && (oldHost == null ? newHost == null : oldHost.equalsIgnoreCase(newHost)) + && (oldEffectivePort == newEffectivePort); + + setUrl(new GenericUrl(newURL, useRawRedirectUrls)); // on 303 change method to GET if (statusCode == HttpStatusCodes.STATUS_CODE_SEE_OTHER) { setRequestMethod(HttpMethods.GET); @@ -1194,11 +1220,29 @@ public boolean handleRedirect(int statusCode, HttpHeaders responseHeaders) { headers.setIfModifiedSince((String) null); headers.setIfUnmodifiedSince((String) null); headers.setIfRange((String) null); + + // remove Cookie header if redirect is cross-origin + if (!sameOrigin) { + headers.setCookie((String) null); + } return true; } return false; } + private static int getEffectivePort(String scheme, int port) { + if (port != -1) { + return port; + } + if ("http".equalsIgnoreCase(scheme)) { + return 80; + } + if ("https".equalsIgnoreCase(scheme)) { + return 443; + } + return -1; + } + /** * Returns the sleeper. * diff --git a/google-http-client/src/test/java/com/google/api/client/http/HttpRequestTest.java b/google-http-client/src/test/java/com/google/api/client/http/HttpRequestTest.java index 085b9f563..af1696630 100644 --- a/google-http-client/src/test/java/com/google/api/client/http/HttpRequestTest.java +++ b/google-http-client/src/test/java/com/google/api/client/http/HttpRequestTest.java @@ -543,6 +543,92 @@ public void subtestHandleRedirect_relativeLocation( assertEquals(newLocation, req.getUrl().toString()); } + @Test + public void testHandleRedirect_crossOriginCookieRemoval() throws IOException { + // 1. Same-origin redirect (http://some.org/a/b -> http://some.org/a/z) + { + HttpTransport transport = new MockHttpTransport(); + HttpRequest req = transport.createRequestFactory().buildGetRequest(new GenericUrl("http://some.org/a/b")); + req.getHeaders().setCookie("foo=bar"); + HttpHeaders responseHeaders = new HttpHeaders().setLocation("http://some.org/a/z"); + req.handleRedirect(HttpStatusCodes.STATUS_CODE_SEE_OTHER, responseHeaders); + assertEquals("foo=bar", req.getHeaders().getCookie()); + assertEquals("http://some.org/a/z", req.getUrl().toString()); + } + + // 2. Cross-origin redirect due to host change (http://some.org/a/b -> http://other.org/c) + { + HttpTransport transport = new MockHttpTransport(); + HttpRequest req = transport.createRequestFactory().buildGetRequest(new GenericUrl("http://some.org/a/b")); + req.getHeaders().setCookie("foo=bar"); + HttpHeaders responseHeaders = new HttpHeaders().setLocation("http://other.org/c"); + req.handleRedirect(HttpStatusCodes.STATUS_CODE_SEE_OTHER, responseHeaders); + assertNull(req.getHeaders().getCookie()); + assertEquals("http://other.org/c", req.getUrl().toString()); + } + + // 3. Cross-origin redirect due to scheme change (https://some.org/a/b -> http://some.org/a/z) + { + HttpTransport transport = new MockHttpTransport(); + HttpRequest req = transport.createRequestFactory().buildGetRequest(new GenericUrl("https://some.org/a/b")); + req.getHeaders().setCookie("foo=bar"); + HttpHeaders responseHeaders = new HttpHeaders().setLocation("http://some.org/a/z"); + req.handleRedirect(HttpStatusCodes.STATUS_CODE_SEE_OTHER, responseHeaders); + assertNull(req.getHeaders().getCookie()); + assertEquals("http://some.org/a/z", req.getUrl().toString()); + } + + // 4. Cross-origin redirect due to port change (http://some.org/a/b -> http://some.org:8080/a/z) + { + HttpTransport transport = new MockHttpTransport(); + HttpRequest req = transport.createRequestFactory().buildGetRequest(new GenericUrl("http://some.org/a/b")); + req.getHeaders().setCookie("foo=bar"); + HttpHeaders responseHeaders = new HttpHeaders().setLocation("http://some.org:8080/a/z"); + req.handleRedirect(HttpStatusCodes.STATUS_CODE_SEE_OTHER, responseHeaders); + assertNull(req.getHeaders().getCookie()); + assertEquals("http://some.org:8080/a/z", req.getUrl().toString()); + } + + // 5. Same-origin redirect with implicit/explicit default ports matching (http://some.org/a/b -> http://some.org:80/a/z) + { + HttpTransport transport = new MockHttpTransport(); + HttpRequest req = transport.createRequestFactory().buildGetRequest(new GenericUrl("http://some.org/a/b")); + req.getHeaders().setCookie("foo=bar"); + HttpHeaders responseHeaders = new HttpHeaders().setLocation("http://some.org:80/a/z"); + req.handleRedirect(HttpStatusCodes.STATUS_CODE_SEE_OTHER, responseHeaders); + assertEquals("foo=bar", req.getHeaders().getCookie()); + assertEquals("http://some.org:80/a/z", req.getUrl().toString()); + } + } + + @Test + public void testExecute_disconnectOnResponseConstructionFailure() throws Exception { + class FailingHttpResponse extends MockLowLevelHttpResponse { + @Override + public String getContentEncoding() { + throw new RuntimeException("Simulated response construction failure"); + } + } + + final FailingHttpResponse failingResponse = new FailingHttpResponse(); + HttpTransport transport = new MockHttpTransport() { + @Override + public LowLevelHttpRequest buildRequest(String method, String url) throws IOException { + return new MockLowLevelHttpRequest().setResponse(failingResponse); + } + }; + + HttpRequest req = transport.createRequestFactory().buildGetRequest(new GenericUrl("http://example.com")); + try { + req.execute(); + fail("Expected RuntimeException"); + } catch (RuntimeException e) { + assertEquals("Simulated response construction failure", e.getMessage()); + } + + assertTrue(failingResponse.isDisconnected()); + } + @Deprecated @Test public void testExecuteErrorWithRetryEnabled() throws Exception {