Skip to content
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

Use new instance of Decompressor on channel initialization #3583

Merged
merged 9 commits into from
Oct 25, 2023
Merged
Original file line number Diff line number Diff line change
@@ -1,3 +1,13 @@
/*
cwperks marked this conversation as resolved.
Show resolved Hide resolved
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*
* The OpenSearch Contributors require contributions made to
* this file be licensed under the Apache-2.0 license or a
* compatible open source license.
*
*/

package org.opensearch.security;

import static org.opensearch.action.support.WriteRequest.RefreshPolicy.IMMEDIATE;
Expand Down
159 changes: 159 additions & 0 deletions src/integrationTest/java/org/opensearch/security/rest/GzipTests.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,159 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*
* The OpenSearch Contributors require contributions made to
* this file be licensed under the Apache-2.0 license or a
* compatible open source license.
*
*/

package org.opensearch.security.rest;

import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope;
import org.apache.hc.client5.http.classic.methods.HttpPost;
import org.apache.hc.core5.http.ContentType;
import org.apache.hc.core5.http.Header;
import org.apache.hc.core5.http.HttpStatus;
import org.apache.hc.core5.http.io.entity.ByteArrayEntity;
import org.apache.hc.core5.http.message.BasicHeader;
import org.junit.ClassRule;
import org.junit.Test;
import org.junit.runner.RunWith;

import static org.hamcrest.CoreMatchers.containsString;
import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.CoreMatchers.not;
import static org.hamcrest.MatcherAssert.assertThat;
import org.opensearch.test.framework.TestSecurityConfig;
import org.opensearch.test.framework.cluster.ClusterManager;
import org.opensearch.test.framework.cluster.LocalCluster;
import org.opensearch.test.framework.cluster.TestRestClient;

import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.util.List;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.ForkJoinPool;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
import java.util.stream.Collectors;
import java.util.stream.IntStream;
import java.util.zip.GZIPOutputStream;

import static org.junit.Assert.fail;
import static org.opensearch.test.framework.TestSecurityConfig.AuthcDomain.AUTHC_HTTPBASIC_INTERNAL;
import static org.opensearch.test.framework.TestSecurityConfig.Role.ALL_ACCESS;
import static org.opensearch.test.framework.cluster.TestRestClientConfiguration.getBasicAuthHeader;

@RunWith(com.carrotsearch.randomizedtesting.RandomizedRunner.class)
@ThreadLeakScope(ThreadLeakScope.Scope.NONE)
public class GzipTests {
peternied marked this conversation as resolved.
Show resolved Hide resolved
private static final TestSecurityConfig.User ADMIN_USER = new TestSecurityConfig.User("admin").roles(ALL_ACCESS);

@ClassRule
public static LocalCluster cluster = new LocalCluster.Builder().clusterManager(ClusterManager.THREE_CLUSTER_MANAGERS)
.authc(AUTHC_HTTPBASIC_INTERNAL)
.users(ADMIN_USER)
.anonymousAuth(false)
.build();

@Test
public void testAuthenticatedGzippedRequests() {
final String requestPath = "/*/_search";
final int parallelism = 10;
final int totalNumberOfRequests = 100;

final byte[] compressedRequestBody = createCompressedRequestBody();
try (final TestRestClient client = cluster.getRestClient(ADMIN_USER, new BasicHeader("Content-Encoding", "gzip"))) {

final ForkJoinPool forkJoinPool = new ForkJoinPool(parallelism);

final List<CompletableFuture<Void>> waitingOn = IntStream.rangeClosed(1, totalNumberOfRequests)
.boxed()
.map(i -> CompletableFuture.runAsync(() -> {
final HttpPost post = new HttpPost(client.getHttpServerUri() + requestPath);
post.setEntity(new ByteArrayEntity(compressedRequestBody, ContentType.APPLICATION_JSON));
TestRestClient.HttpResponse response = client.executeRequest(post);
assertThat(response.getStatusCode(), equalTo(HttpStatus.SC_OK));
assertThat(response.getBody(), not(containsString("json_parse_exception")));
peternied marked this conversation as resolved.
Show resolved Hide resolved
}, forkJoinPool))
.collect(Collectors.toList());

final CompletableFuture<Void> allOfThem = CompletableFuture.allOf(waitingOn.toArray(new CompletableFuture[0]));

allOfThem.get(30, TimeUnit.SECONDS);
} catch (ExecutionException e) {
Throwable rootCause = e.getCause();
if (rootCause instanceof AssertionError) {
fail("Received exception: " + e.getMessage());
}
// ignore
} catch (InterruptedException e) {
// ignore
} catch (TimeoutException e) {
// ignore
}
}

@Test
public void testMixOfAuthenticatedAndUnauthenticatedGzippedRequests() {
final String requestPath = "/*/_search";
final int parallelism = 10;
final int totalNumberOfRequests = 100;

final byte[] compressedRequestBody = createCompressedRequestBody();
try (TestRestClient client = cluster.getRestClient(new BasicHeader("Content-Encoding", "gzip"))) {

final ForkJoinPool forkJoinPool = new ForkJoinPool(parallelism);

Header basicAuthHeader = getBasicAuthHeader(ADMIN_USER.getName(), ADMIN_USER.getPassword());

final List<CompletableFuture<Void>> waitingOn = IntStream.rangeClosed(1, totalNumberOfRequests)
.boxed()
.map(i -> CompletableFuture.runAsync(() -> {
final HttpPost post = new HttpPost(client.getHttpServerUri() + requestPath);
post.setEntity(new ByteArrayEntity(compressedRequestBody, ContentType.APPLICATION_JSON));
TestRestClient.HttpResponse response = i % 2 == 0
? client.executeRequest(post)
: client.executeRequest(post, basicAuthHeader);
assertThat(response.getStatusCode(), equalTo(i % 2 == 0 ? HttpStatus.SC_UNAUTHORIZED : HttpStatus.SC_OK));
assertThat(response.getBody(), not(containsString("json_parse_exception")));
}, forkJoinPool))
.collect(Collectors.toList());
peternied marked this conversation as resolved.
Show resolved Hide resolved

final CompletableFuture<Void> allOfThem = CompletableFuture.allOf(waitingOn.toArray(new CompletableFuture[0]));

allOfThem.get(30, TimeUnit.SECONDS);
} catch (ExecutionException e) {
Throwable rootCause = e.getCause();
if (rootCause instanceof AssertionError) {
fail("Received exception: " + e.getMessage());
}
// ignore
} catch (InterruptedException e) {
// ignore
} catch (TimeoutException e) {
// ignore
}
}

private byte[] createCompressedRequestBody() {
cwperks marked this conversation as resolved.
Show resolved Hide resolved
final String rawBody = "{ \"query\": { \"match\": { \"foo\": \"bar\" }}}";

try (
final ByteArrayOutputStream byteArrayOutputStream = new ByteArrayOutputStream();
final GZIPOutputStream gzipOutputStream = new GZIPOutputStream(byteArrayOutputStream)
) {
gzipOutputStream.write(rawBody.getBytes(StandardCharsets.UTF_8));
gzipOutputStream.finish();

final byte[] compressedRequestBody = byteArrayOutputStream.toByteArray();
return compressedRequestBody;
} catch (final IOException ioe) {
throw new RuntimeException(ioe);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@
public class SecurityNonSslHttpServerTransport extends Netty4HttpServerTransport {

private final ChannelInboundHandlerAdapter headerVerifier;
private final ChannelInboundHandlerAdapter conditionalDecompressor;

public SecurityNonSslHttpServerTransport(
final Settings settings,
Expand All @@ -73,7 +72,6 @@ public SecurityNonSslHttpServerTransport(
tracer
);
headerVerifier = new Netty4HttpRequestHeaderVerifier(restFilter, threadPool, settings);
conditionalDecompressor = new Netty4ConditionalDecompressor();
}

@Override
Expand All @@ -100,6 +98,6 @@ protected ChannelInboundHandlerAdapter createHeaderVerifier() {

@Override
protected ChannelInboundHandlerAdapter createDecompressor() {
return conditionalDecompressor;
return new Netty4ConditionalDecompressor();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@

package org.opensearch.security.ssl.http.netty;

import io.netty.channel.ChannelHandler.Sharable;
import io.netty.channel.embedded.EmbeddedChannel;
import io.netty.handler.codec.http.HttpContentDecompressor;

Expand All @@ -17,7 +16,6 @@

import org.opensearch.security.filter.NettyAttribute;

@Sharable
peternied marked this conversation as resolved.
Show resolved Hide resolved
public class Netty4ConditionalDecompressor extends HttpContentDecompressor {

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ public class SecuritySSLNettyHttpServerTransport extends Netty4HttpServerTranspo
private final SecurityKeyStore sks;
private final SslExceptionHandler errorHandler;
private final ChannelInboundHandlerAdapter headerVerifier;
private final ChannelInboundHandlerAdapter conditionalDecompressor;

public SecuritySSLNettyHttpServerTransport(
final Settings settings,
Expand Down Expand Up @@ -79,7 +78,6 @@ public SecuritySSLNettyHttpServerTransport(
this.sks = sks;
this.errorHandler = errorHandler;
headerVerifier = new Netty4HttpRequestHeaderVerifier(restFilter, threadPool, settings);
conditionalDecompressor = new Netty4ConditionalDecompressor();
}

@Override
Expand Down Expand Up @@ -164,6 +162,6 @@ protected ChannelInboundHandlerAdapter createHeaderVerifier() {

@Override
protected ChannelInboundHandlerAdapter createDecompressor() {
return conditionalDecompressor;
return new Netty4ConditionalDecompressor();
}
}