From 8df297beac3079f7f919cf3fb3ae5ef9288ed188 Mon Sep 17 00:00:00 2001 From: Greg Schohn Date: Wed, 13 Sep 2023 16:33:31 -0400 Subject: [PATCH] Two bugfixes to facilitate "--remove-auth-headers" (MIGRATIONS-1309). First, wire up the remove method for the wrapper (list adapting) map of the case insensitive map implementation so that remove() is actually called on the underlying map, rather than calling an UnsupportedOperationException. The second issue was that the original request headers and the newly transformed headers were deemed to be identical, even though an item was missing within the transformed map. We only iterated through the transformed map and we didn't make sure that we were going to hit all of the elements. Now we check that the lengths are the same too. Signed-off-by: Greg Schohn --- ...tKeyAdaptingCaseInsensitiveHeadersMap.java | 6 ++++ ...dHttpRequestPreliminaryConvertHandler.java | 3 +- .../HttpJsonTransformingConsumerTest.java | 33 ++++++++++++++++++- .../requests/raw/get_withAuthHeader.txt | 5 +++ 4 files changed, 45 insertions(+), 2 deletions(-) create mode 100644 TrafficCapture/trafficReplayer/src/test/resources/requests/raw/get_withAuthHeader.txt diff --git a/TrafficCapture/trafficReplayer/src/main/java/org/opensearch/migrations/replay/datahandlers/http/ListKeyAdaptingCaseInsensitiveHeadersMap.java b/TrafficCapture/trafficReplayer/src/main/java/org/opensearch/migrations/replay/datahandlers/http/ListKeyAdaptingCaseInsensitiveHeadersMap.java index ce679a426..d8fc83bf3 100644 --- a/TrafficCapture/trafficReplayer/src/main/java/org/opensearch/migrations/replay/datahandlers/http/ListKeyAdaptingCaseInsensitiveHeadersMap.java +++ b/TrafficCapture/trafficReplayer/src/main/java/org/opensearch/migrations/replay/datahandlers/http/ListKeyAdaptingCaseInsensitiveHeadersMap.java @@ -50,6 +50,12 @@ public List put(String key, Object value) { return strictHeadersMap.put(key, strList); } + @Override + public List remove(Object key) { + return strictHeadersMap.remove(key); + } + + /** * This is just casting the underlying object's entrySet. An old git commit will show this unrolled, * but this should be significantly more efficient. diff --git a/TrafficCapture/trafficReplayer/src/main/java/org/opensearch/migrations/replay/datahandlers/http/NettyDecodedHttpRequestPreliminaryConvertHandler.java b/TrafficCapture/trafficReplayer/src/main/java/org/opensearch/migrations/replay/datahandlers/http/NettyDecodedHttpRequestPreliminaryConvertHandler.java index f54c3fcc0..13232e276 100644 --- a/TrafficCapture/trafficReplayer/src/main/java/org/opensearch/migrations/replay/datahandlers/http/NettyDecodedHttpRequestPreliminaryConvertHandler.java +++ b/TrafficCapture/trafficReplayer/src/main/java/org/opensearch/migrations/replay/datahandlers/http/NettyDecodedHttpRequestPreliminaryConvertHandler.java @@ -150,7 +150,8 @@ private void handlePayloadNeutralTransformationOrThrow(ChannelHandlerContext ctx private boolean headerFieldsAreIdentical(HttpRequest request, HttpJsonMessageWithFaultingPayload httpJsonMessage) { if (!request.uri().equals(httpJsonMessage.path()) || - !request.method().toString().equals(httpJsonMessage.method())) { + !request.method().toString().equals(httpJsonMessage.method()) || + request.headers().size() != httpJsonMessage.headers().strictHeadersMap.size()) { return false; } for (var headerName : httpJsonMessage.headers().keySet()) { diff --git a/TrafficCapture/trafficReplayer/src/test/java/org/opensearch/migrations/replay/datahandlers/http/HttpJsonTransformingConsumerTest.java b/TrafficCapture/trafficReplayer/src/test/java/org/opensearch/migrations/replay/datahandlers/http/HttpJsonTransformingConsumerTest.java index 9e080470e..5cb65aebd 100644 --- a/TrafficCapture/trafficReplayer/src/test/java/org/opensearch/migrations/replay/datahandlers/http/HttpJsonTransformingConsumerTest.java +++ b/TrafficCapture/trafficReplayer/src/test/java/org/opensearch/migrations/replay/datahandlers/http/HttpJsonTransformingConsumerTest.java @@ -9,6 +9,7 @@ import org.opensearch.migrations.transform.JsonCompositeTransformer; import org.opensearch.migrations.transform.JsonJoltTransformer; import org.opensearch.migrations.transform.IJsonTransformer; +import org.opensearch.migrations.transform.RemovingAuthTransformerFactory; import java.nio.charset.StandardCharsets; import java.time.Duration; @@ -48,7 +49,7 @@ public void testPassThroughSinglePacketWithoutBodyTransformationPost() throws Ex null, testPacketCapture, "TEST", new UniqueRequestKey("testConnectionId", 0)); byte[] testBytes; try (var sampleStream = HttpJsonTransformingConsumer.class.getResourceAsStream( - "/requests/raw/post_formUrlEncoded_withFixedLength.txt")) { + "/requests/raw/get_withAuthHeader.txt")) { testBytes = sampleStream.readAllBytes(); testBytes = new String(testBytes, StandardCharsets.UTF_8) .replace("foo.example", "test.domain") @@ -62,6 +63,34 @@ public void testPassThroughSinglePacketWithoutBodyTransformationPost() throws Ex Assertions.assertEquals(HttpRequestTransformationStatus.SKIPPED, returnedResponse.transformationStatus); } + @Test + public void testRemoveAuthHeadersWorks() throws Exception { + final var dummyAggregatedResponse = new AggregatedRawResponse(17, null, null, null); + var testPacketCapture = new TestCapturePacketToHttpHandler(Duration.ofMillis(100), dummyAggregatedResponse); + var transformingHandler = + new HttpJsonTransformingConsumer( + JsonJoltTransformer.newBuilder() + .addHostSwitchOperation("test.domain") + .build(), + RemovingAuthTransformerFactory.instance, + testPacketCapture, "TEST", new UniqueRequestKey("testConnectionId", 0)); + byte[] testBytes; + try (var sampleStream = HttpJsonTransformingConsumer.class.getResourceAsStream( + "/requests/raw/get_withAuthHeader.txt")) { + testBytes = sampleStream.readAllBytes(); + testBytes = new String(testBytes, StandardCharsets.UTF_8) + .replace("foo.example", "test.domain") + .replace("auTHorization: Basic YWRtaW46YWRtaW4=\n", "") + .getBytes(StandardCharsets.UTF_8); + } + transformingHandler.consumeBytes(testBytes); + var returnedResponse = transformingHandler.finalizeRequest().get(); + Assertions.assertEquals(new String(testBytes, StandardCharsets.UTF_8), + testPacketCapture.getCapturedAsString()); + Assertions.assertArrayEquals(testBytes, testPacketCapture.getBytesCaptured()); + Assertions.assertEquals(HttpRequestTransformationStatus.SKIPPED, returnedResponse.transformationStatus); + } + @Test public void testPartialBodyThrowsAndIsRedriven() throws Exception { final var dummyAggregatedResponse = new AggregatedRawResponse(17, null, null, null); @@ -100,4 +129,6 @@ private void walkMaps(Object o) { Assertions.assertInstanceOf(NettyJsonBodyAccumulateHandler.IncompleteJsonBodyException.class, returnedResponse.error); } + + } \ No newline at end of file diff --git a/TrafficCapture/trafficReplayer/src/test/resources/requests/raw/get_withAuthHeader.txt b/TrafficCapture/trafficReplayer/src/test/resources/requests/raw/get_withAuthHeader.txt new file mode 100644 index 000000000..c8808385c --- /dev/null +++ b/TrafficCapture/trafficReplayer/src/test/resources/requests/raw/get_withAuthHeader.txt @@ -0,0 +1,5 @@ +GET /test HTTP/1.1 +Host: foo.example +auTHorization: Basic YWRtaW46YWRtaW4= +Content-Type: application/json +