Skip to content

Commit 324a038

Browse files
[Bugfix] Implement skipsDeserialization() in RestoringTransportResponseHandler (opensearch-project#6154)
Signed-off-by: Finn Carroll <carrofin@amazon.com>
1 parent 3bb8342 commit 324a038

2 files changed

Lines changed: 124 additions & 2 deletions

File tree

src/main/java/org/opensearch/security/transport/SecurityInterceptor.java

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -379,12 +379,14 @@ private ThreadContext getThreadContext() {
379379
// based on
380380
// org.opensearch.transport.TransportService.ContextRestoreResponseHandler<T>
381381
// which is private scoped
382-
private class RestoringTransportResponseHandler<T extends TransportResponse> implements TransportResponseHandler<T> {
382+
// Visible for testing
383+
class RestoringTransportResponseHandler<T extends TransportResponse> implements TransportResponseHandler<T> {
383384

384385
private final Supplier<ThreadContext.StoredContext> contextToRestore;
385386
private final TransportResponseHandler<T> innerHandler;
386387

387-
private RestoringTransportResponseHandler(
388+
// Visible for testing
389+
RestoringTransportResponseHandler(
388390
TransportResponseHandler<T> innerHandler,
389391
Supplier<ThreadContext.StoredContext> contextToRestore
390392
) {
@@ -397,6 +399,11 @@ public T read(StreamInput in) throws IOException {
397399
return innerHandler.read(in);
398400
}
399401

402+
@Override
403+
public boolean skipsDeserialization() {
404+
return innerHandler.skipsDeserialization();
405+
}
406+
400407
@Override
401408
public void handleStreamResponse(StreamTransportResponse<T> response) {
402409
// we do not have to process headers like OPENDISTRO_SECURITY_DLS_QUERY_HEADER which is done in handleResponse(),
Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,115 @@
1+
/*
2+
* SPDX-License-Identifier: Apache-2.0
3+
*
4+
* The OpenSearch Contributors require contributions made to
5+
* this file be licensed under the Apache-2.0 license or a
6+
* compatible open source license.
7+
*/
8+
9+
package org.opensearch.security.transport;
10+
11+
import java.io.IOException;
12+
import java.util.function.Supplier;
13+
14+
import org.junit.Test;
15+
16+
import org.opensearch.common.settings.Settings;
17+
import org.opensearch.common.util.concurrent.ThreadContext;
18+
import org.opensearch.core.common.io.stream.StreamInput;
19+
import org.opensearch.core.transport.TransportResponse;
20+
import org.opensearch.transport.TransportException;
21+
import org.opensearch.transport.TransportResponseHandler;
22+
import org.opensearch.transport.stream.StreamTransportResponse;
23+
24+
import static org.junit.Assert.assertEquals;
25+
import static org.junit.Assert.assertSame;
26+
27+
/**
28+
* Verifies that RestoringTransportResponseHandler delegates all interface methods
29+
* identically to the inner handler.
30+
*/
31+
public class RestoringTransportResponseHandlerTests {
32+
33+
@Test
34+
public void testWrappedHandlerBehavesIdenticallyToInnerHandler() throws IOException {
35+
TestTransportResponseHandler innerHandler = new TestTransportResponseHandler(true, "search_worker");
36+
TransportResponseHandler<TransportResponse> wrappedHandler = getRestorableTransportResponseTransportResponseHandler(innerHandler);
37+
38+
// Verify skipsDeserialization is forwarded (critical for Arrow Flight zero-copy)
39+
assertEquals(
40+
"skipsDeserialization must match inner handler",
41+
innerHandler.skipsDeserialization(),
42+
wrappedHandler.skipsDeserialization()
43+
);
44+
45+
// Verify executor is forwarded
46+
assertEquals("executor must match inner handler", innerHandler.executor(), wrappedHandler.executor());
47+
48+
// Verify read() forwards the StreamInput and returns the same response
49+
byte[] payload = new byte[] { 0x01, 0x42, (byte) 0xFF, 0x00, 0x7E, (byte) 0xAB };
50+
StreamInput streamInput = StreamInput.wrap(payload);
51+
TransportResponse readResult = wrappedHandler.read(streamInput);
52+
53+
assertSame("read() must forward the exact StreamInput to inner handler", streamInput, innerHandler.lastReadInput);
54+
assertSame("read() must return the inner handler's response", innerHandler.readResponse, readResult);
55+
}
56+
57+
private static TransportResponseHandler<TransportResponse> getRestorableTransportResponseTransportResponseHandler(
58+
TestTransportResponseHandler innerHandler
59+
) {
60+
ThreadContext threadContext = new ThreadContext(Settings.EMPTY);
61+
Supplier<ThreadContext.StoredContext> restorableContext = threadContext.newRestorableContext(true);
62+
SecurityInterceptor interceptor = new SecurityInterceptor(
63+
null,
64+
null,
65+
null,
66+
null,
67+
null,
68+
null,
69+
null,
70+
null,
71+
null,
72+
null,
73+
() -> false,
74+
null
75+
);
76+
return interceptor.new RestoringTransportResponseHandler<>(innerHandler, restorableContext);
77+
}
78+
79+
private static class TestTransportResponseHandler implements TransportResponseHandler<TransportResponse> {
80+
private final boolean skipsDeserialization;
81+
private final String executor;
82+
final TransportResponse readResponse = new TransportResponse.Empty();
83+
StreamInput lastReadInput;
84+
85+
TestTransportResponseHandler(boolean skipsDeserialization, String executor) {
86+
this.skipsDeserialization = skipsDeserialization;
87+
this.executor = executor;
88+
}
89+
90+
@Override
91+
public TransportResponse read(StreamInput in) throws IOException {
92+
lastReadInput = in;
93+
return readResponse;
94+
}
95+
96+
@Override
97+
public boolean skipsDeserialization() {
98+
return skipsDeserialization;
99+
}
100+
101+
@Override
102+
public void handleResponse(TransportResponse response) {}
103+
104+
@Override
105+
public void handleStreamResponse(StreamTransportResponse<TransportResponse> response) {}
106+
107+
@Override
108+
public void handleException(TransportException exp) {}
109+
110+
@Override
111+
public String executor() {
112+
return executor;
113+
}
114+
}
115+
}

0 commit comments

Comments
 (0)