From ecc7f2b30aaa92d969a8a90cff923407e5d169c3 Mon Sep 17 00:00:00 2001 From: jonathanedey Date: Mon, 9 Feb 2026 17:53:05 -0500 Subject: [PATCH 1/2] fix: Reimplement HTTP/2 response entity consumption using ApacheHttp2AsyncEntityConsumer and ApacheHttp2Entity --- .../ApacheHttp2AsyncEntityConsumer.java | 80 +++++++++++++ .../firebase/internal/ApacheHttp2Entity.java | 37 ++++++ .../firebase/internal/ApacheHttp2Request.java | 28 +++-- .../internal/ApacheHttp2Response.java | 45 +++++--- .../internal/ApacheHttp2TransportTest.java | 107 +++++++++++++++++- 5 files changed, 266 insertions(+), 31 deletions(-) create mode 100644 src/main/java/com/google/firebase/internal/ApacheHttp2AsyncEntityConsumer.java create mode 100644 src/main/java/com/google/firebase/internal/ApacheHttp2Entity.java diff --git a/src/main/java/com/google/firebase/internal/ApacheHttp2AsyncEntityConsumer.java b/src/main/java/com/google/firebase/internal/ApacheHttp2AsyncEntityConsumer.java new file mode 100644 index 000000000..b8b0bffd1 --- /dev/null +++ b/src/main/java/com/google/firebase/internal/ApacheHttp2AsyncEntityConsumer.java @@ -0,0 +1,80 @@ +/* + * Copyright 2026 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.firebase.internal; + +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.nio.ByteBuffer; +import java.util.List; +import org.apache.hc.core5.concurrent.FutureCallback; +import org.apache.hc.core5.http.EntityDetails; +import org.apache.hc.core5.http.Header; +import org.apache.hc.core5.http.HttpException; +import org.apache.hc.core5.http.nio.AsyncEntityConsumer; +import org.apache.hc.core5.http.nio.CapacityChannel; + +public class ApacheHttp2AsyncEntityConsumer implements AsyncEntityConsumer { + + private EntityDetails entityDetails; + private FutureCallback resultCallback; + private final ByteArrayOutputStream buffer = new ByteArrayOutputStream(); + + @Override + public void streamStart( + final EntityDetails entityDetails, + final FutureCallback resultCallback) + throws HttpException, IOException { + this.entityDetails = entityDetails; + this.resultCallback = resultCallback; + } + + @Override + public void updateCapacity(CapacityChannel capacityChannel) throws IOException { + capacityChannel.update(Integer.MAX_VALUE); + } + + @Override + public void consume(ByteBuffer src) throws IOException { + byte[] bytes = new byte[src.remaining()]; + src.get(bytes); + buffer.write(bytes); + } + + @Override + public void streamEnd(List trailers) throws HttpException, IOException { + if (resultCallback != null) { + resultCallback.completed(getContent()); + } + } + + @Override + public void failed(Exception cause) { + if (resultCallback != null) { + resultCallback.failed(cause); + } + } + + @Override + public void releaseResources() { + buffer.reset(); + } + + @Override + public ApacheHttp2Entity getContent() { + return new ApacheHttp2Entity(buffer.toByteArray(), entityDetails); + } +} diff --git a/src/main/java/com/google/firebase/internal/ApacheHttp2Entity.java b/src/main/java/com/google/firebase/internal/ApacheHttp2Entity.java new file mode 100644 index 000000000..9769a28a8 --- /dev/null +++ b/src/main/java/com/google/firebase/internal/ApacheHttp2Entity.java @@ -0,0 +1,37 @@ +/* + * Copyright 2026 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.firebase.internal; + +import org.apache.hc.core5.http.EntityDetails; + +public class ApacheHttp2Entity { + private final byte[] content; + private final EntityDetails entityDetails; + + public ApacheHttp2Entity(byte[] content, EntityDetails entityDetails) { + this.content = content; + this.entityDetails = entityDetails; + } + + public byte[] getContent() { + return content; + } + + public EntityDetails getEntityDetails() { + return entityDetails; + } +} diff --git a/src/main/java/com/google/firebase/internal/ApacheHttp2Request.java b/src/main/java/com/google/firebase/internal/ApacheHttp2Request.java index a711dc0ac..ceb213c52 100644 --- a/src/main/java/com/google/firebase/internal/ApacheHttp2Request.java +++ b/src/main/java/com/google/firebase/internal/ApacheHttp2Request.java @@ -32,13 +32,14 @@ import org.apache.hc.client5.http.ConnectTimeoutException; import org.apache.hc.client5.http.HttpHostConnectException; import org.apache.hc.client5.http.async.methods.SimpleHttpRequest; -import org.apache.hc.client5.http.async.methods.SimpleHttpResponse; import org.apache.hc.client5.http.async.methods.SimpleRequestBuilder; -import org.apache.hc.client5.http.async.methods.SimpleResponseConsumer; import org.apache.hc.client5.http.config.RequestConfig; import org.apache.hc.client5.http.impl.async.CloseableHttpAsyncClient; import org.apache.hc.core5.concurrent.FutureCallback; +import org.apache.hc.core5.http.HttpResponse; +import org.apache.hc.core5.http.Message; import org.apache.hc.core5.http.nio.support.BasicRequestProducer; +import org.apache.hc.core5.http.nio.support.BasicResponseConsumer; import org.apache.hc.core5.http2.H2StreamResetException; import org.apache.hc.core5.util.Timeout; @@ -49,6 +50,7 @@ final class ApacheHttp2Request extends LowLevelHttpRequest { private final RequestConfig.Builder requestConfig; private int writeTimeout; private ApacheHttp2AsyncEntityProducer entityProducer; + private ApacheHttp2AsyncEntityConsumer entityConsumer; ApacheHttp2Request( CloseableHttpAsyncClient httpAsyncClient, SimpleRequestBuilder requestBuilder) { @@ -85,17 +87,20 @@ public LowLevelHttpResponse execute() throws IOException { // Build request request = requestBuilder.build(); - // Make Producer + // Make Entity Producer CompletableFuture writeFuture = new CompletableFuture<>(); entityProducer = new ApacheHttp2AsyncEntityProducer(this, writeFuture); + // Make Entity Consumer + entityConsumer = new ApacheHttp2AsyncEntityConsumer(); + // Execute - final Future responseFuture = httpAsyncClient.execute( + final Future> responseFuture = httpAsyncClient.execute( new BasicRequestProducer(request, entityProducer), - SimpleResponseConsumer.create(), - new FutureCallback() { + new BasicResponseConsumer(entityConsumer), + new FutureCallback>() { @Override - public void completed(final SimpleHttpResponse response) { + public void completed(final Message response) { } @Override @@ -120,10 +125,10 @@ public void cancelled() { // Wait for response try { - final SimpleHttpResponse response = responseFuture.get(); + final Message response = responseFuture.get(); return new ApacheHttp2Response(response); } catch (ExecutionException e) { - if (e.getCause() instanceof ConnectTimeoutException + if (e.getCause() instanceof ConnectTimeoutException || e.getCause() instanceof SocketTimeoutException) { throw new IOException("Connection Timeout", e.getCause()); } else if (e.getCause() instanceof HttpHostConnectException) { @@ -144,4 +149,9 @@ public void cancelled() { ApacheHttp2AsyncEntityProducer getEntityProducer() { return entityProducer; } + + @VisibleForTesting + ApacheHttp2AsyncEntityConsumer getEntityConsumer() { + return entityConsumer; + } } diff --git a/src/main/java/com/google/firebase/internal/ApacheHttp2Response.java b/src/main/java/com/google/firebase/internal/ApacheHttp2Response.java index 329ec3970..7d277b972 100644 --- a/src/main/java/com/google/firebase/internal/ApacheHttp2Response.java +++ b/src/main/java/com/google/firebase/internal/ApacheHttp2Response.java @@ -23,17 +23,27 @@ import java.io.IOException; import java.io.InputStream; -import org.apache.hc.client5.http.async.methods.SimpleHttpResponse; -import org.apache.hc.core5.http.ContentType; +import org.apache.hc.core5.http.EntityDetails; import org.apache.hc.core5.http.Header; +import org.apache.hc.core5.http.HttpResponse; +import org.apache.hc.core5.http.Message; +import org.apache.hc.core5.http.message.StatusLine; public class ApacheHttp2Response extends LowLevelHttpResponse { - private final SimpleHttpResponse response; + private final Message message; + private final HttpResponse response; private final Header[] allHeaders; + private final EntityDetails entity; + private final byte[] content; - ApacheHttp2Response(SimpleHttpResponse response) { - this.response = response; - allHeaders = response.getHeaders(); + ApacheHttp2Response(Message message) { + this.message = message; + this.response = message.getHead(); + this.allHeaders = response.getHeaders(); + + ApacheHttp2Entity body = message.getBody(); + this.entity = body != null ? body.getEntityDetails() : null; + this.content = body != null ? body.getContent() : null; } @Override @@ -43,25 +53,25 @@ public int getStatusCode() { @Override public InputStream getContent() throws IOException { - return new ByteArrayInputStream(response.getBodyBytes()); + return content == null ? null : new ByteArrayInputStream(content); } @Override public String getContentEncoding() { - Header contentEncodingHeader = response.getFirstHeader("Content-Encoding"); - return contentEncodingHeader == null ? null : contentEncodingHeader.getValue(); + return entity == null ? null : entity.getContentEncoding(); } @Override public long getContentLength() { - String bodyText = response.getBodyText(); - return bodyText == null ? 0 : bodyText.length(); + if (content != null) { + return content.length; + } + return entity == null ? -1 : entity.getContentLength(); } @Override public String getContentType() { - ContentType contentType = response.getContentType(); - return contentType == null ? null : contentType.toString(); + return entity == null ? null : entity.getContentType(); } @Override @@ -71,11 +81,12 @@ public String getReasonPhrase() { @Override public String getStatusLine() { - return response.toString(); + return new StatusLine(response).toString(); } public String getHeaderValue(String name) { - return response.getLastHeader(name).getValue(); + Header header = response.getLastHeader(name); + return header == null ? null : header.getValue(); } @Override @@ -94,7 +105,7 @@ public String getHeaderName(int index) { } @VisibleForTesting - public SimpleHttpResponse getResponse() { - return response; + public Message getMessage() { + return message; } } diff --git a/src/test/java/com/google/firebase/internal/ApacheHttp2TransportTest.java b/src/test/java/com/google/firebase/internal/ApacheHttp2TransportTest.java index 60b053109..f1ab29e08 100644 --- a/src/test/java/com/google/firebase/internal/ApacheHttp2TransportTest.java +++ b/src/test/java/com/google/firebase/internal/ApacheHttp2TransportTest.java @@ -56,11 +56,13 @@ import org.apache.hc.core5.http.HttpRequestMapper; import org.apache.hc.core5.http.HttpResponse; import org.apache.hc.core5.http.HttpStatus; +import org.apache.hc.core5.http.Message; import org.apache.hc.core5.http.impl.bootstrap.HttpServer; import org.apache.hc.core5.http.impl.io.HttpService; import org.apache.hc.core5.http.io.HttpRequestHandler; import org.apache.hc.core5.http.io.entity.ByteArrayEntity; import org.apache.hc.core5.http.io.support.BasicHttpServerRequestHandler; +import org.apache.hc.core5.http.message.BasicHttpResponse; import org.apache.hc.core5.http.nio.AsyncPushConsumer; import org.apache.hc.core5.http.nio.AsyncRequestProducer; import org.apache.hc.core5.http.nio.AsyncResponseConsumer; @@ -87,7 +89,9 @@ public Future doExecute( final HandlerFactory pushHandlerFactory, final HttpContext context, final FutureCallback callback) { - return (Future) CompletableFuture.completedFuture(new SimpleHttpResponse(200)); + return (Future) CompletableFuture + .completedFuture( + new Message(new BasicHttpResponse(200), null)); } }, requestBuilder); @@ -118,7 +122,9 @@ public Future doExecute( final HandlerFactory pushHandlerFactory, final HttpContext context, final FutureCallback callback) { - return (Future) CompletableFuture.completedFuture(new SimpleHttpResponse(200)); + return (Future) CompletableFuture + .completedFuture( + new Message(new BasicHttpResponse(200), null)); } }, requestBuilder); @@ -148,7 +154,9 @@ public Future doExecute( final HandlerFactory pushHandlerFactory, final HttpContext context, final FutureCallback callback) { - return (Future) CompletableFuture.completedFuture(simpleHttpResponse); + return (Future) CompletableFuture + .completedFuture(new Message(simpleHttpResponse, + new ApacheHttp2Entity(simpleHttpResponse.getBodyBytes(), null))); } }, requestBuilder); LowLevelHttpResponse response = request.execute(); @@ -157,7 +165,7 @@ public Future doExecute( // we confirm that the simple response we prepared in this test is the same as // the content's response assertTrue(response.getContent() instanceof ByteArrayInputStream); - assertEquals(simpleHttpResponse, ((ApacheHttp2Response) response).getResponse()); + assertEquals(simpleHttpResponse, ((ApacheHttp2Response) response).getMessage().getHead()); // No need to cloase ByteArrayInputStream since close() has no effect. } @@ -212,7 +220,9 @@ public Future doExecute( final HandlerFactory pushHandlerFactory, final HttpContext context, final FutureCallback callback) { - return (Future) CompletableFuture.completedFuture(new SimpleHttpResponse(200)); + return (Future) CompletableFuture + .completedFuture(new Message( + new BasicHttpResponse(200), null)); } }; ApacheHttp2Transport transport = new ApacheHttp2Transport(mockClient); @@ -328,6 +338,93 @@ private void execute(ApacheHttp2Request request) throws IOException { request.execute(); } + @Test + public void testGzipResponse() throws IOException { + final String originalContent = "hello world"; + final java.io.ByteArrayOutputStream baos = new java.io.ByteArrayOutputStream(); + try (java.util.zip.GZIPOutputStream gzip = new java.util.zip.GZIPOutputStream(baos)) { + gzip.write(originalContent.getBytes(StandardCharsets.UTF_8)); + } + final byte[] gzippedContent = baos.toByteArray(); + + final HttpRequestHandler handler = new HttpRequestHandler() { + @Override + public void handle( + ClassicHttpRequest request, ClassicHttpResponse response, HttpContext context) + throws HttpException, IOException { + response.setCode(HttpStatus.SC_OK); + response.setHeader(HttpHeaders.CONTENT_ENCODING, "gzip"); + response.setHeader(HttpHeaders.CONTENT_LENGTH, String.valueOf(gzippedContent.length)); + response.setHeader(HttpHeaders.CONTENT_TYPE, "text/plain; charset=UTF-8"); + ByteArrayEntity entity = new ByteArrayEntity(gzippedContent, ContentType.TEXT_PLAIN); + response.setEntity(entity); + } + }; + + try (FakeServer server = new FakeServer(handler)) { + ApacheHttp2Transport transport = new ApacheHttp2Transport(); + GenericUrl testUrl = new GenericUrl("http://localhost/foo"); + testUrl.setPort(server.getPort()); + + // Execute the low-level request directly to accurately assert metadata + // without Google's parsed HttpHeaders map mangling the payload lengths. + ApacheHttp2Request request = transport.buildRequest("GET", testUrl.build()); + LowLevelHttpResponse response = request.execute(); + + assertEquals(200, response.getStatusCode()); + assertEquals("text/plain; charset=UTF-8", response.getContentType()); + + boolean wasAutoDecompressed = response.getContentEncoding() == null; + if (wasAutoDecompressed) { + System.out.println("Auto-decompressed"); + assertEquals(originalContent.length(), response.getContentLength()); + } else { + System.out.println("Not auto-decompressed"); + assertEquals("gzip", response.getContentEncoding()); + assertEquals(gzippedContent.length, response.getContentLength()); + } + + // Verify the low-level stream returns the exact expected payload based on + // decompression state + java.io.InputStream stream = response.getContent(); + byte[] resultBytes = com.google.common.io.ByteStreams.toByteArray(stream); + + if (wasAutoDecompressed) { + assertEquals(originalContent, new String(resultBytes, StandardCharsets.UTF_8)); + } else { + org.junit.Assert.assertArrayEquals(gzippedContent, resultBytes); + } + } + } + + @Test + public void testEmptyResponseWithHeaders() throws IOException { + // Tests that a response with no actual body but headers does not throw NPE + // in ApacheHttp2Response due to entity being null. + final HttpRequestHandler handler = new HttpRequestHandler() { + @Override + public void handle( + ClassicHttpRequest request, ClassicHttpResponse response, HttpContext context) + throws HttpException, IOException { + response.setCode(HttpStatus.SC_NO_CONTENT); + response.setHeader(HttpHeaders.CONTENT_LENGTH, "0"); + // Explicitly omitting the entity to simulate NO_CONTENT bodyless response + } + }; + + try (FakeServer server = new FakeServer(handler)) { + HttpTransport transport = new ApacheHttp2Transport(); + GenericUrl testUrl = new GenericUrl("http://localhost/empty"); + testUrl.setPort(server.getPort()); + com.google.api.client.http.HttpResponse response = transport.createRequestFactory() + .buildGetRequest(testUrl) + .execute(); + + assertEquals(204, response.getStatusCode()); + assertEquals(0L, response.getHeaders().getContentLength().longValue()); + } + } + private static class FakeServer implements AutoCloseable { private final HttpServer server; From 6f7c7e238cf43e373eb4a32b7c34823eab9f624e Mon Sep 17 00:00:00 2001 From: jonathanedey Date: Tue, 10 Feb 2026 11:18:59 -0500 Subject: [PATCH 2/2] fix: Address gemini review --- .../internal/ApacheHttp2AsyncEntityConsumer.java | 11 ++++++++--- .../firebase/internal/ApacheHttp2TransportTest.java | 2 -- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/main/java/com/google/firebase/internal/ApacheHttp2AsyncEntityConsumer.java b/src/main/java/com/google/firebase/internal/ApacheHttp2AsyncEntityConsumer.java index b8b0bffd1..ee8733586 100644 --- a/src/main/java/com/google/firebase/internal/ApacheHttp2AsyncEntityConsumer.java +++ b/src/main/java/com/google/firebase/internal/ApacheHttp2AsyncEntityConsumer.java @@ -49,9 +49,14 @@ public void updateCapacity(CapacityChannel capacityChannel) throws IOException { @Override public void consume(ByteBuffer src) throws IOException { - byte[] bytes = new byte[src.remaining()]; - src.get(bytes); - buffer.write(bytes); + if (src.hasArray()) { + buffer.write(src.array(), src.arrayOffset() + src.position(), src.remaining()); + src.position(src.limit()); + } else { + byte[] bytes = new byte[src.remaining()]; + src.get(bytes); + buffer.write(bytes); + } } @Override diff --git a/src/test/java/com/google/firebase/internal/ApacheHttp2TransportTest.java b/src/test/java/com/google/firebase/internal/ApacheHttp2TransportTest.java index f1ab29e08..ba604d3d1 100644 --- a/src/test/java/com/google/firebase/internal/ApacheHttp2TransportTest.java +++ b/src/test/java/com/google/firebase/internal/ApacheHttp2TransportTest.java @@ -376,10 +376,8 @@ public void handle( boolean wasAutoDecompressed = response.getContentEncoding() == null; if (wasAutoDecompressed) { - System.out.println("Auto-decompressed"); assertEquals(originalContent.length(), response.getContentLength()); } else { - System.out.println("Not auto-decompressed"); assertEquals("gzip", response.getContentEncoding()); assertEquals(gzippedContent.length, response.getContentLength()); }