From 7acfdcaee13c9ace59ef258a7c78631e8b7148c0 Mon Sep 17 00:00:00 2001 From: cen1 Date: Mon, 30 Nov 2020 11:47:15 +0100 Subject: [PATCH] Adds a new jetty specific client property to set the max response buffer size. If property is not set, the old code path is used and exception is thrown for large responses (bigger than 2MiB). This addresses the immediate paint point and is least invasive. Exception is still thrown if response size is too big even for the custom size. Since this behaviour differs from async path it might be a good idea to just replace it completely in some major release. --- .../connector/JettyClientProperties.java | 10 + .../jetty/connector/JettyConnector.java | 31 +++- .../jetty/connector/SyncResponseSizeTest.java | 171 ++++++++++++++++++ 3 files changed, 208 insertions(+), 4 deletions(-) create mode 100644 connectors/jetty-connector/src/test/java/org/glassfish/jersey/jetty/connector/SyncResponseSizeTest.java diff --git a/connectors/jetty-connector/src/main/java/org/glassfish/jersey/jetty/connector/JettyClientProperties.java b/connectors/jetty-connector/src/main/java/org/glassfish/jersey/jetty/connector/JettyClientProperties.java index e38fbb500ae..2e3f9aafe18 100644 --- a/connectors/jetty-connector/src/main/java/org/glassfish/jersey/jetty/connector/JettyClientProperties.java +++ b/connectors/jetty-connector/src/main/java/org/glassfish/jersey/jetty/connector/JettyClientProperties.java @@ -75,6 +75,16 @@ private JettyClientProperties() { public static final String ENABLE_SSL_HOSTNAME_VERIFICATION = "jersey.config.jetty.client.enableSslHostnameVerification"; + /** + * Overrides the default Jetty synchronous listener response max buffer size. + * In practise, this allows you to read larger responses. + * Size in bytes. + *

+ * If the property is absent, the value is such as specified by Jetty (currently 2MiB). + */ + public static final String SYNC_LISTENER_RESPONSE_MAX_SIZE = + "jersey.config.jetty.client.syncListenerResponseMaxSize"; + /** * Get the value of the specified property. * diff --git a/connectors/jetty-connector/src/main/java/org/glassfish/jersey/jetty/connector/JettyConnector.java b/connectors/jetty-connector/src/main/java/org/glassfish/jersey/jetty/connector/JettyConnector.java index 48a539422cb..a0e8e50e604 100644 --- a/connectors/jetty-connector/src/main/java/org/glassfish/jersey/jetty/connector/JettyConnector.java +++ b/connectors/jetty-connector/src/main/java/org/glassfish/jersey/jetty/connector/JettyConnector.java @@ -31,8 +31,10 @@ import java.util.Optional; import java.util.concurrent.CancellationException; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutionException; import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; import java.util.logging.Level; @@ -45,6 +47,10 @@ import javax.net.ssl.SSLContext; +import org.eclipse.jetty.client.util.BasicAuthentication; +import org.eclipse.jetty.client.util.BytesContentProvider; +import org.eclipse.jetty.client.util.FutureResponseListener; +import org.eclipse.jetty.client.util.OutputStreamContentProvider; import org.glassfish.jersey.client.ClientProperties; import org.glassfish.jersey.client.ClientRequest; import org.glassfish.jersey.client.ClientResponse; @@ -65,9 +71,6 @@ import org.eclipse.jetty.client.api.Request; import org.eclipse.jetty.client.api.Response; import org.eclipse.jetty.client.api.Result; -import org.eclipse.jetty.client.util.BasicAuthentication; -import org.eclipse.jetty.client.util.BytesContentProvider; -import org.eclipse.jetty.client.util.OutputStreamContentProvider; import org.eclipse.jetty.http.HttpField; import org.eclipse.jetty.http.HttpFields; import org.eclipse.jetty.http.HttpHeader; @@ -130,6 +133,7 @@ class JettyConnector implements Connector { private final HttpClient client; private final CookieStore cookieStore; private final Configuration configuration; + private final Optional syncListenerResponseMaxSize; /** * Create the new Jetty client connector. @@ -199,6 +203,16 @@ class JettyConnector implements Connector { client.setCookieStore(new HttpCookieStore.Empty()); } + final Object slResponseMaxSize = configuration.getProperties() + .get(JettyClientProperties.SYNC_LISTENER_RESPONSE_MAX_SIZE); + if (slResponseMaxSize != null && slResponseMaxSize instanceof Integer + && (Integer) slResponseMaxSize > 0) { + this.syncListenerResponseMaxSize = Optional.of((Integer) slResponseMaxSize); + } + else { + this.syncListenerResponseMaxSize = Optional.empty(); + } + try { client.start(); } catch (final Exception e) { @@ -248,7 +262,16 @@ public ClientResponse apply(final ClientRequest jerseyRequest) throws Processing } try { - final ContentResponse jettyResponse = jettyRequest.send(); + final ContentResponse jettyResponse; + if (!syncListenerResponseMaxSize.isPresent()) { + jettyResponse = jettyRequest.send(); + } + else { + final FutureResponseListener listener + = new FutureResponseListener(jettyRequest, syncListenerResponseMaxSize.get()); + jettyRequest.send(listener); + jettyResponse = listener.get(); + } HeaderUtils.checkHeaderChanges(clientHeadersSnapshot, jerseyRequest.getHeaders(), JettyConnector.this.getClass().getName(), jerseyRequest.getConfiguration()); diff --git a/connectors/jetty-connector/src/test/java/org/glassfish/jersey/jetty/connector/SyncResponseSizeTest.java b/connectors/jetty-connector/src/test/java/org/glassfish/jersey/jetty/connector/SyncResponseSizeTest.java new file mode 100644 index 00000000000..b233a4ccb05 --- /dev/null +++ b/connectors/jetty-connector/src/test/java/org/glassfish/jersey/jetty/connector/SyncResponseSizeTest.java @@ -0,0 +1,171 @@ +/* + * Copyright (c) 2013, 2020 Oracle and/or its affiliates. All rights reserved. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License v. 2.0, which is available at + * http://www.eclipse.org/legal/epl-2.0. + * + * This Source Code may also be made available under the following Secondary + * Licenses when the conditions for such availability set forth in the + * Eclipse Public License v. 2.0 are satisfied: GNU General Public License, + * version 2 with the GNU Classpath Exception, which is available at + * https://www.gnu.org/software/classpath/license.html. + * + * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0 + */ + +package org.glassfish.jersey.jetty.connector; + +import org.glassfish.jersey.client.ClientConfig; +import org.glassfish.jersey.client.ClientProperties; +import org.glassfish.jersey.logging.LoggingFeature; +import org.glassfish.jersey.server.ResourceConfig; +import org.glassfish.jersey.test.JerseyTest; +import org.junit.Test; + +import javax.ws.rs.GET; +import javax.ws.rs.Path; +import javax.ws.rs.ProcessingException; +import javax.ws.rs.client.Client; +import javax.ws.rs.client.ClientBuilder; +import javax.ws.rs.client.WebTarget; +import javax.ws.rs.core.Application; +import javax.ws.rs.core.Response; +import java.net.URI; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.TimeoutException; +import java.util.logging.Logger; + +import static org.hamcrest.CoreMatchers.instanceOf; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + +/** + * Default synchronous jetty client implementation has a hard response size limit of 2MiB. + * When response is too big, a processing exception is thrown. + * The original code path was left to preserve this behaviour but could be removed + * and reworked in the future with a custom listener like async path. + * + * This tests the previous behavior with large payloads (>2MiB), the new size override (4MiB) + * and very big payloads (>4MiB). + * + * @author cen1 (cen.is.imba at gmail.com) + */ +public class SyncResponseSizeTest extends JerseyTest { + + private static final Logger LOGGER = Logger.getLogger(SyncResponseSizeTest.class.getName()); + + private static final int maxBufferSize = 4 * 1024 * 1024; //4 MiB + + @Path("/test") + public static class TimeoutResource { + + private static final byte[] data = new byte[maxBufferSize]; + + static { + Byte b = "a".getBytes()[0]; + for (int i = 0; i < maxBufferSize; i++) data[i] = b.byteValue(); + } + + @GET + @Path("/small") + public String getSmall() { + return "GET"; + } + + @GET + @Path("/big") + public String getBig() { + return new String(data); + } + + @GET + @Path("/verybig") + public String getVeryBig() { + return new String(data) + "a"; + } + } + + @Override + protected Application configure() { + ResourceConfig config = new ResourceConfig(TimeoutResource.class); + config.register(new LoggingFeature(LOGGER, LoggingFeature.Verbosity.PAYLOAD_ANY)); + return config; + } + + @Override + protected void configureClient(ClientConfig config) { + config.connectorProvider(new JettyConnectorProvider()); + } + + @Test + public void testDefaultSmall() { + Response r = target("test/small").request().get(); + assertEquals(200, r.getStatus()); + assertEquals("GET", r.readEntity(String.class)); + } + + @Test + public void testDefaultTooBig() { + final URI u = target().getUri(); + ClientConfig config = new ClientConfig().property(ClientProperties.READ_TIMEOUT, 1_000); + config.connectorProvider(new JettyConnectorProvider()); + + Client c = ClientBuilder.newClient(config); + WebTarget t = c.target(u); + try { + t.path("test/big").request().get(); + fail("Exception expected."); + } catch (ProcessingException e) { + // Buffering capacity ... exceeded. + assertTrue(ExecutionException.class.isInstance(e.getCause())); + assertTrue(IllegalArgumentException.class.isInstance(e.getCause().getCause())); + } finally { + c.close(); + } + } + + @Test + public void testCustomBig() { + final URI u = target().getUri(); + ClientConfig config = new ClientConfig().property(ClientProperties.READ_TIMEOUT, 1_000); + config.connectorProvider(new JettyConnectorProvider()); + config.property(JettyClientProperties.SYNC_LISTENER_RESPONSE_MAX_SIZE, maxBufferSize); + + Client c = ClientBuilder.newClient(config); + WebTarget t = c.target(u); + try { + Response r = t.path("test/big").request().get(); + String p = r.readEntity(String.class); + assertEquals(p.length(), maxBufferSize); + } catch (ProcessingException e) { + assertThat("Unexpected processing exception cause", + e.getCause(), instanceOf(TimeoutException.class)); + } finally { + c.close(); + } + } + + @Test + public void testCustomTooBig() { + final URI u = target().getUri(); + ClientConfig config = new ClientConfig().property(ClientProperties.READ_TIMEOUT, 1_000); + config.connectorProvider(new JettyConnectorProvider()); + config.property(JettyClientProperties.SYNC_LISTENER_RESPONSE_MAX_SIZE, maxBufferSize); + + Client c = ClientBuilder.newClient(config); + WebTarget t = c.target(u); + try { + t.path("test/verybig").request().get(); + fail("Exception expected."); + } catch (ProcessingException e) { + // Buffering capacity ... exceeded. + assertTrue(ExecutionException.class.isInstance(e.getCause())); + assertTrue(IllegalArgumentException.class.isInstance(e.getCause().getCause())); + } finally { + c.close(); + } + } +}