Skip to content

DEVEXP-562 Can now prevent Accept-Encoding=gzip header being used #1593

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,10 @@ public class ConfigureOkHttp {
* header should not result in losing any other values besides "gzip" for the header. You are free to
* customize this as you wish though; this is primarily intended as an example for how to customize OkHttp
* when using the MarkLogic Java Client.
*/
*
* As of Java Client 6.3.0, this can now be accomplished via the {@code DatabaseClientFactory} class and
* {@code RemoveAcceptEncodingConfigurator}.
*/
public static void removeAcceptEncodingGzipHeader() {
DatabaseClientFactory.addConfigurator(new OkHttpClientConfigurator() {
@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,6 @@ public DatabaseClientBuilder withCloudAuth(String apiKey, String basePath) {
}

/**
*
* @param apiKey
* @param basePath
* @param tokenDuration length in minutes until the generated access token expires
Expand All @@ -174,7 +173,6 @@ public DatabaseClientBuilder withCertificateAuth(String file, String password) {
}

/**
*
* @param sslContext
* @param trustManager
* @return
Expand Down Expand Up @@ -250,6 +248,17 @@ public DatabaseClientBuilder withSSLHostnameVerifier(DatabaseClientFactory.SSLHo
props.put(PREFIX + "sslHostnameVerifier", sslHostnameVerifier);
return this;
}

/**
* Prevents the underlying OkHttp library from sending an "Accept-Encoding-gzip" request header on each request.
*
* @return
* @since 6.3.0
*/
public DatabaseClientBuilder withGzippedResponsesDisabled() {
props.put(PREFIX + "disableGzippedResponses", true);
return this;
}
}


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@
*/
public class DatabaseClientFactory {

static private ClientConfigurator<?> clientConfigurator;
static private List<ClientConfigurator<?>> clientConfigurators = new ArrayList<>();
static private HandleFactoryRegistry handleRegistry =
HandleFactoryRegistryImpl.newDefault();

Expand Down Expand Up @@ -1273,6 +1273,9 @@ public String getCertificatePassword() {
* <li>marklogic.client.basePath = must be a String</li>
* <li>marklogic.client.database = must be a String</li>
* <li>marklogic.client.connectionType = must be a String or instance of {@code ConnectionType}</li>
* <li>marklogic.client.disableGzippedResponses = can be a String or Boolean; if "true" or true, the client
* will not send an "Accept-Encoding" request header with a value of "gzip" on each request; supported
* since 6.3.0.</li>
* <li>marklogic.client.securityContext = an instance of {@code SecurityContext}; if set, then all other
* authentication properties pertaining to the construction of a {@code SecurityContext} will be ignored,
* including the properties pertaining to SSL; this is effectively an escape hatch for providing a
Expand All @@ -1285,6 +1288,8 @@ public String getCertificatePassword() {
* <li>marklogic.client.certificate.file = must be a String; optional for certificate authentication</li>
* <li>marklogic.client.certificate.password = must be a String; optional for certificate authentication</li>
* <li>marklogic.client.cloud.apiKey = must be a String; required for cloud authentication</li>
* <li>marklogic.client.cloud.tokenDuration = must be a number; optional for configuring the duration in
* minutes for which an access token lasts; supported since 6.3.0.</li>
* <li>marklogic.client.kerberos.principal = must be a String; required for Kerberos authentication</li>
* <li>marklogic.client.saml.token = must be a String; required for SAML authentication</li>
* <li>marklogic.client.sslContext = must be an instance of {@code javax.net.ssl.SSLContext}</li>
Expand Down Expand Up @@ -1408,17 +1413,19 @@ static public DatabaseClient newClient(String host, int port, String basePath, S
}
services.connect(host, port, basePath, database, securityContext);

if (clientConfigurator != null) {
if (clientConfigurator instanceof OkHttpClientConfigurator) {
OkHttpClient okHttpClient = (OkHttpClient) services.getClientImplementation();
OkHttpClient.Builder clientBuilder = okHttpClient.newBuilder();
((OkHttpClientConfigurator) clientConfigurator).configure(clientBuilder);
((OkHttpServices) services).setClientImplementation(clientBuilder.build());
} else if (clientConfigurator instanceof HttpClientConfigurator) {
// do nothing as we no longer use HttpClient so there's nothing this can configure
} else {
throw new IllegalArgumentException("A ClientConfigurator must implement OkHttpClientConfigurator");
}
if (clientConfigurators != null) {
clientConfigurators.forEach(configurator -> {
if (configurator instanceof OkHttpClientConfigurator) {
OkHttpClient okHttpClient = (OkHttpClient) services.getClientImplementation();
OkHttpClient.Builder clientBuilder = okHttpClient.newBuilder();
((OkHttpClientConfigurator) configurator).configure(clientBuilder);
((OkHttpServices) services).setClientImplementation(clientBuilder.build());
} else if (configurator instanceof HttpClientConfigurator) {
// do nothing as we no longer use HttpClient so there's nothing this can configure
} else {
throw new IllegalArgumentException("A ClientConfigurator must implement OkHttpClientConfigurator");
}
});
}

DatabaseClientImpl client = new DatabaseClientImpl(
Expand Down Expand Up @@ -1584,6 +1591,10 @@ static public void registerDefaultHandles() {
/**
* Adds a listener that provides custom configuration when a communication library
* is created.
*
* As of 6.3.0, this method can now be called multiple times. When a {@code DatabaseClient} is constructed,
* configurators will be invoked in the order they were passed in.
*
* @see com.marklogic.client.extra.okhttpclient.OkHttpClientConfigurator
* @param configurator the listener for configuring the communication library
*/
Expand All @@ -1594,7 +1605,16 @@ static public void addConfigurator(ClientConfigurator<?> configurator) {
);
}

clientConfigurator = configurator;
clientConfigurators.add(configurator);
}

/**
* Removes any instances of {@code ClientConfigurator} that were passed in via {@code addConfigurator}.
*
* @since 6.3.0
*/
static public void removeConfigurators() {
clientConfigurators.clear();
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
package com.marklogic.client.extra.okhttpclient;

import okhttp3.OkHttpClient;
import okhttp3.Request;

/**
* Can be used with {@code DatabaseClientFactory.addConfigurator} to remove the "Accept-Encoding=gzip" request header
* that the underlying OkHttp library adds by default. This is useful in a scenario where many small HTTP responses
* are expected to be returned by MarkLogic, and thus the costs of gzipping the responses may outweigh the benefits.
*
* @since 6.3.0
*/
public class RemoveAcceptEncodingConfigurator implements OkHttpClientConfigurator {

@Override
public void configure(OkHttpClient.Builder builder) {
builder.addNetworkInterceptor(chain -> {
Request newRequest = chain.request().newBuilder().removeHeader("Accept-Encoding").build();
return chain.proceed(newRequest);
});
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import com.marklogic.client.DatabaseClient;
import com.marklogic.client.DatabaseClientBuilder;
import com.marklogic.client.DatabaseClientFactory;
import com.marklogic.client.extra.okhttpclient.RemoveAcceptEncodingConfigurator;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand Down Expand Up @@ -91,6 +92,17 @@ public class DatabaseClientPropertySource {
throw new IllegalArgumentException("Connection type must either be a String or an instance of ConnectionType");
}
});
connectionPropertyHandlers.put(PREFIX + "disableGzippedResponses", (bean, value) -> {
boolean disableGzippedResponses = false;
if (value instanceof Boolean && Boolean.TRUE.equals(value)) {
disableGzippedResponses = true;
} else if (value instanceof String) {
disableGzippedResponses = Boolean.parseBoolean((String)value);
}
if (disableGzippedResponses) {
DatabaseClientFactory.addConfigurator(new RemoveAcceptEncodingConfigurator());
}
});
}

public DatabaseClientPropertySource(Function<String, Object> propertySource) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,21 @@ void cloudWithNonNumericDuration() {
assertEquals("Cloud token duration must be numeric", ex.getMessage());
}

@Test
void disableGzippedResponses() {
final String prop = PREFIX + "disableGzippedResponses";

props.put(PREFIX + prop, "true");
// Won't throw an error, but we can't verify the results because the list of configurators in
// DatabaseClientFactory is private.
buildBean();

// Verifying this doesn't throw an error either; the impl should be using Boolean.parseBoolean which only cares
// if the value equals 'true'.
props.put(prop, "123");
buildBean();
}

private DatabaseClientFactory.Bean buildBean() {
DatabaseClientPropertySource source = new DatabaseClientPropertySource(propertyName -> props.get(propertyName));
return source.newClientBean();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,7 @@ public void testConfigurator() {
assertEquals(testConnectTimeoutMillis, okClient.connectTimeoutMillis());
} finally {
client.release();
DatabaseClientFactory.removeConfigurators();
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
package com.marklogic.client.test.extra;

import com.marklogic.client.DatabaseClientFactory;
import com.marklogic.client.extra.okhttpclient.OkHttpClientConfigurator;
import com.marklogic.client.test.Common;
import com.marklogic.client.test.junit5.RequiresML11;
import okhttp3.Interceptor;
import okhttp3.Response;
import org.jetbrains.annotations.NotNull;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;

import java.io.IOException;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNull;

@ExtendWith(RequiresML11.class)
public class DisableGzippedResponsesTest {

@AfterEach
void afterEach() {
DatabaseClientFactory.removeConfigurators();
}

@Test
void test() {
// Add a DatabaseClientFactory configurator that uses an OkHttp interceptor to capture the value of the
// Content-Encoding response header. That is the easiest way to verify whether the Accept-Encoding:gzip request
// header is being sent or not.
TestInterceptor testInterceptor = new TestInterceptor();
DatabaseClientFactory.addConfigurator((OkHttpClientConfigurator) builder -> builder.addNetworkInterceptor(testInterceptor));


final String testUri = "/optic/test/musician1.json";

Common.newClient().newJSONDocumentManager().read(testUri);
assertEquals("gzip", testInterceptor.contentEncoding, "MarkLogic 11 now supports the Accept-Encoding:gzip " +
"header. The OkHttp library used by the Java Client includes this request header by default. So we " +
"expect for responses from the REST API to be gzipped, which is indicated by the Content-Encoding " +
"response header having a value of 'gzip'.");

Common.newClientBuilder().withGzippedResponsesDisabled().build()
.newJSONDocumentManager().read(testUri);
assertNull(testInterceptor.contentEncoding, "When a DatabaseClient is constructed with gzipped responses " +
"disabled, the Accept-Encoding header added automatically by OkHttp should be removed before the request " +
"is sent to MarkLogic. This prevents MarkLogic from gzipping the response, which is indicated by the " +
"HTTP response not having a 'Content-Encoding' header.");
}

/**
* Used to capture the value of the Content-Encoding response header.
*/
private static class TestInterceptor implements Interceptor {

String contentEncoding;

@NotNull
@Override
public Response intercept(@NotNull Chain chain) throws IOException {
Response response = chain.proceed(chain.request());
contentEncoding = response.header("Content-Encoding");
return response;
}
}
}