Skip to content

Commit 019054f

Browse files
mr3mr3
authored
fix: replace getQueryString with getParameter to avoid reading request inputstream (#424)
1 parent 408a0a0 commit 019054f

File tree

9 files changed

+101
-16
lines changed

9 files changed

+101
-16
lines changed

‎arex-agent-bootstrap/src/main/java/io/arex/agent/bootstrap/util/StringUtil.java‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,39 @@ public static String substring(String str, int start) {
143143
return start > str.length() ? EMPTY : str.substring(start);
144144
}
145145

146+
public static String substring(final String str, int start, int end) {
147+
if (str == null) {
148+
return null;
149+
}
150+
151+
// handle negatives
152+
if (end < 0) {
153+
end = str.length() + end; // remember end is negative
154+
}
155+
if (start < 0) {
156+
start = str.length() + start; // remember start is negative
157+
}
158+
159+
// check length next
160+
if (end > str.length()) {
161+
end = str.length();
162+
}
163+
164+
// if start is greater than end, return ""
165+
if (start > end) {
166+
return EMPTY;
167+
}
168+
169+
if (start < 0) {
170+
start = 0;
171+
}
172+
if (end < 0) {
173+
end = 0;
174+
}
175+
176+
return str.substring(start, end);
177+
}
178+
146179
public static String[] split(final String source, final char separator) {
147180
return split(source, separator, false);
148181
}

‎arex-agent-bootstrap/src/test/java/io/arex/agent/bootstrap/util/StringUtilTest.java‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,9 @@
1010
import org.junit.jupiter.api.Test;
1111
import org.junit.jupiter.params.ParameterizedTest;
1212
import org.junit.jupiter.params.provider.CsvSource;
13+
import org.junit.jupiter.params.provider.EmptySource;
14+
import org.junit.jupiter.params.provider.NullSource;
15+
import org.junit.jupiter.params.provider.ValueSource;
1316

1417
class StringUtilTest {
1518
@Test
@@ -119,6 +122,19 @@ void substring() {
119122
assertEquals("abc", actualResult);
120123
}
121124

125+
@Test
126+
void substring_start_end() {
127+
assertNull(StringUtil.substring(null, 0, 0));
128+
assertEquals("", StringUtil.substring("", 0 , 0));
129+
assertEquals("ab", StringUtil.substring("abc", 0, 2));
130+
assertEquals("", StringUtil.substring("abc", 2, 0));
131+
assertEquals("c", StringUtil.substring("abc", 2, 4));
132+
assertEquals("", StringUtil.substring("abc", 4, 6));
133+
assertEquals("", StringUtil.substring("abc", 2, 2));
134+
assertEquals("b", StringUtil.substring("abc", -2, -1));
135+
assertEquals("ab", StringUtil.substring("abc", -4, 2));
136+
}
137+
122138
@Test
123139
void split() {
124140
String[] actualResult = StringUtil.split(null, ',');

‎arex-instrumentation/servlet/arex-httpservlet/src/main/java/io/arex/inst/httpservlet/ServletAdviceHelper.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -261,7 +261,7 @@ private static <TRequest> boolean shouldSkip(ServletAdapter<TRequest, ?> adapter
261261

262262
private static <TRequest, TResponse> String getRedirectRecordId(ServletAdapter<TRequest, TResponse> adapter,
263263
TRequest httpServletRequest) {
264-
String redirectRecordId = adapter.getParameter(httpServletRequest, ArexConstants.RECORD_ID);
264+
String redirectRecordId = adapter.getParameterFromQueryString(httpServletRequest, ArexConstants.RECORD_ID);
265265
if (StringUtil.isEmpty(redirectRecordId)) {
266266
return null;
267267
}
@@ -273,7 +273,7 @@ private static <TRequest, TResponse> String getRedirectRecordId(ServletAdapter<T
273273
}
274274

275275
ArexContext context = ContextManager.getContext(redirectRecordId);
276-
if (context.isRedirectRequest(referer)) {
276+
if (context != null && context.isRedirectRequest(referer)) {
277277
return redirectRecordId;
278278
}
279279

‎arex-instrumentation/servlet/arex-httpservlet/src/main/java/io/arex/inst/httpservlet/adapter/ServletAdapter.java‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package io.arex.inst.httpservlet.adapter;
22

3+
import io.arex.agent.bootstrap.util.StringUtil;
34
import org.springframework.web.context.request.NativeWebRequest;
45

56
import javax.annotation.Nullable;
@@ -68,5 +69,22 @@ void addListener(ServletAdapter<HttpServletRequest, HttpServletResponse> adapter
6869

6970
boolean markProcessed(HttpServletRequest httpServletRequest, String mark);
7071

71-
String getParameter(HttpServletRequest httpServletRequest, String name);
72+
String getQueryString(HttpServletRequest httpServletRequest);
73+
74+
default String getParameterFromQueryString(HttpServletRequest httpServletRequest, String name) {
75+
String queryString = getQueryString(httpServletRequest);
76+
if (StringUtil.isEmpty(queryString)) {
77+
return null;
78+
}
79+
80+
int lastIndex = queryString.lastIndexOf(name);
81+
if (lastIndex < 0) {
82+
return null;
83+
}
84+
85+
int start = lastIndex + name.length() + 1;
86+
int end = queryString.indexOf("&", start);
87+
end = end < 0 ? queryString.length() : end;
88+
return StringUtil.substring(queryString, start, end);
89+
}
7290
}

‎arex-instrumentation/servlet/arex-httpservlet/src/main/java/io/arex/inst/httpservlet/adapter/impl/ServletAdapterImplV3.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,7 @@ public boolean markProcessed(HttpServletRequest httpServletRequest, String mark)
226226
}
227227

228228
@Override
229-
public String getParameter(HttpServletRequest servletRequest, String name) {
230-
return servletRequest.getParameter(name);
229+
public String getQueryString(HttpServletRequest servletRequest) {
230+
return servletRequest.getQueryString();
231231
}
232232
}

‎arex-instrumentation/servlet/arex-httpservlet/src/main/java/io/arex/inst/httpservlet/adapter/impl/ServletAdapterImplV5.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,7 @@ public boolean markProcessed(HttpServletRequest httpServletRequest, String mark)
226226
}
227227

228228
@Override
229-
public String getParameter(HttpServletRequest httpServletRequest, String name) {
230-
return httpServletRequest.getParameter(name);
229+
public String getQueryString(HttpServletRequest httpServletRequest) {
230+
return httpServletRequest.getQueryString();
231231
}
232232
}

‎arex-instrumentation/servlet/arex-httpservlet/src/test/java/io/arex/inst/httpservlet/ServletAdviceHelperTest.java‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package io.arex.inst.httpservlet;
22

33
import io.arex.agent.bootstrap.internal.Pair;
4+
import io.arex.agent.bootstrap.util.StringUtil;
45
import io.arex.inst.runtime.config.Config;
56
import io.arex.inst.runtime.context.ArexContext;
67
import io.arex.inst.runtime.context.ContextManager;
@@ -20,6 +21,7 @@
2021
import org.junit.jupiter.params.ParameterizedTest;
2122
import org.junit.jupiter.params.provider.Arguments;
2223
import org.junit.jupiter.params.provider.MethodSource;
24+
import org.junit.jupiter.params.provider.ValueSource;
2325
import org.mockito.MockedConstruction;
2426
import org.mockito.Mockito;
2527
import org.mockito.junit.jupiter.MockitoExtension;
@@ -100,14 +102,14 @@ static Stream<Arguments> onServiceEnterCase() {
100102
Mockito.when(adapter.getRequestHeader(any(), eq(ArexConstants.RECORD_ID))).thenReturn("mock");
101103
};
102104
Runnable getRedirectRecordId1 = () -> {
103-
Mockito.when(adapter.getParameter(any(), eq(ArexConstants.RECORD_ID))).thenReturn(null);
105+
Mockito.when(adapter.getParameterFromQueryString(any(), eq(ArexConstants.RECORD_ID))).thenReturn(null);
104106
};
105107
Runnable getRedirectRecordId2 = () -> {
106-
Mockito.when(adapter.getParameter(any(), eq(ArexConstants.RECORD_ID))).thenReturn("mock-redirectRecordId");
108+
Mockito.when(adapter.getParameterFromQueryString(any(), eq(ArexConstants.RECORD_ID))).thenReturn("mock-redirectRecordId");
107109
Mockito.when(adapter.getRequestHeader(any(), eq("referer"))).thenReturn(null);
108110
};
109111
Runnable getRedirectRecordId3 = () -> {
110-
Mockito.when(adapter.getParameter(any(), eq(ArexConstants.RECORD_ID))).thenReturn("mock-redirectRecordId");
112+
Mockito.when(adapter.getParameterFromQueryString(any(), eq(ArexConstants.RECORD_ID))).thenReturn("mock-redirectRecordId");
111113
Mockito.when(adapter.getRequestHeader(any(), eq("referer"))).thenReturn("mock-referer");
112114
ArexContext context = ArexContext.of("mock-record-id");
113115
context.setAttachment(ArexConstants.REDIRECT_REFERER, "mock-referer");

‎arex-instrumentation/servlet/arex-httpservlet/src/test/java/io/arex/inst/httpservlet/adapter/impl/ServletAdapterImplV3Test.java‎

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import static org.mockito.ArgumentMatchers.eq;
66
import static org.mockito.Mockito.when;
77

8+
import io.arex.agent.bootstrap.util.StringUtil;
89
import io.arex.inst.httpservlet.wrapper.CachedBodyRequestWrapperV3;
910
import io.arex.inst.httpservlet.wrapper.CachedBodyResponseWrapperV3;
1011
import java.io.ByteArrayInputStream;
@@ -20,6 +21,8 @@
2021
import org.junit.jupiter.api.AfterEach;
2122
import org.junit.jupiter.api.BeforeEach;
2223
import org.junit.jupiter.api.Test;
24+
import org.junit.jupiter.params.ParameterizedTest;
25+
import org.junit.jupiter.params.provider.ValueSource;
2326
import org.mockito.Mockito;
2427
import org.springframework.web.context.request.NativeWebRequest;
2528

@@ -266,8 +269,21 @@ void markProcessed() {
266269
}
267270

268271
@Test
269-
void getParameter() {
270-
when(mockRequest.getParameter(any())).thenReturn("mock-parameter");
271-
assertEquals("mock-parameter", instance.getParameter(mockRequest, "arex-parameter"));
272+
void getQueryString() {
273+
when(mockRequest.getQueryString()).thenReturn("k1=v1&k2=v2");
274+
assertEquals("k1=v1&k2=v2", instance.getQueryString(mockRequest));
275+
}
276+
277+
@ParameterizedTest
278+
@ValueSource(strings = {
279+
"k1=v1&arex-record-id=mock-redirectRecordId&k2=v2",
280+
"k1=v1&arex-record-id=mock-redirectRecordId",
281+
"arex-record-id=mock-redirectRecordId&k2=v2",
282+
"arex-record-id=mock-redirectRecordId"
283+
})
284+
void getParameterFromQueryString(String queryString) {
285+
when(mockRequest.getQueryString()).thenReturn(queryString);
286+
String redirectRecordId = instance.getParameterFromQueryString(mockRequest, "arex-record-id");
287+
assertEquals("mock-redirectRecordId", redirectRecordId);
272288
}
273289
}

‎arex-instrumentation/servlet/arex-httpservlet/src/test/java/io/arex/inst/httpservlet/adapter/impl/ServletAdapterImplV5Test.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -268,8 +268,8 @@ void markProcessed() {
268268
}
269269

270270
@Test
271-
void getParameter() {
272-
when(mockRequest.getParameter(any())).thenReturn("mock-parameter");
273-
assertEquals("mock-parameter", instance.getParameter(mockRequest, "arex-parameter"));
271+
void getQueryString() {
272+
when(mockRequest.getQueryString()).thenReturn("k1=v1&k2=v2");
273+
assertEquals("k1=v1&k2=v2", instance.getQueryString(mockRequest));
274274
}
275275
}

0 commit comments

Comments
 (0)