Skip to content

Commit 15d4953

Browse files
committed
fix: Resolve the SonarCloud findings on the gzip code
- RequestBodyReader: do the clamp subtraction in long so the int arithmetic cannot be read as a narrowing hazard (S2184). The value is unchanged. - GzipIT: assert with hasSizeLessThan on the array rather than on its length field. - RequestBodyReaderTest: drop a throws IOException the body cannot throw, since the gzip call sits inside the assertion lambda. - RequestPreparationFilterTest: static-import the Mockito DSL, matching the convention the rest of the suite already follows.
1 parent 5807c59 commit 15d4953

4 files changed

Lines changed: 20 additions & 17 deletions

File tree

src/main/java/com/retailsvc/http/internal/RequestBodyReader.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ public RequestBodyReader(long maxDecompressedBytes) {
3636
"maxDecompressedBytes must be positive, got " + maxDecompressedBytes);
3737
}
3838
this.maxDecompressedBytes = maxDecompressedBytes;
39-
this.readLimit = (int) Math.min(maxDecompressedBytes, Integer.MAX_VALUE - 1) + 1;
39+
this.readLimit = (int) Math.min(maxDecompressedBytes, Integer.MAX_VALUE - 1L) + 1;
4040
}
4141

4242
/**

src/test/java/com/retailsvc/http/GzipIT.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ void largeResponseIsGzippedWhenClientAcceptsGzip() throws Exception {
133133
assertThat(response.headers().firstValue("Vary"))
134134
.hasValueSatisfying(vary -> assertThat(vary).contains(ACCEPT_ENCODING));
135135
assertThat(new String(gunzip(response.body()), UTF_8)).isEqualTo(payload);
136-
assertThat(response.body().length).isLessThan(payload.length());
136+
assertThat(response.body()).hasSizeLessThan(payload.length());
137137
}
138138
}
139139

src/test/java/com/retailsvc/http/internal/RequestBodyReaderTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ void unsupportedCodingThrows415() {
7777
}
7878

7979
@Test
80-
void oversizedInflatedBodyThrows413() throws IOException {
80+
void oversizedInflatedBodyThrows413() {
8181
byte[] bomb = new byte[(int) CAP * 4];
8282

8383
assertThatThrownBy(() -> reader.read(exchange(gzip(bomb), "gzip")))

src/test/java/com/retailsvc/http/internal/RequestPreparationFilterTest.java

Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,11 @@
44
import static java.net.HttpURLConnection.HTTP_UNSUPPORTED_TYPE;
55
import static org.assertj.core.api.Assertions.assertThat;
66
import static org.assertj.core.api.Assertions.assertThatThrownBy;
7+
import static org.mockito.ArgumentMatchers.any;
8+
import static org.mockito.Mockito.doAnswer;
79
import static org.mockito.Mockito.mock;
10+
import static org.mockito.Mockito.verify;
11+
import static org.mockito.Mockito.when;
812

913
import com.retailsvc.http.BadRequestException;
1014
import com.retailsvc.http.ExceptionHandler;
@@ -41,7 +45,6 @@
4145
import java.util.concurrent.atomic.AtomicReference;
4246
import java.util.zip.GZIPOutputStream;
4347
import org.junit.jupiter.api.Test;
44-
import org.mockito.Mockito;
4548

4649
class RequestPreparationFilterTest {
4750

@@ -51,10 +54,10 @@ private HttpExchange exchange(String method, String path, byte[] body) {
5154

5255
private HttpExchange exchange(String method, String path, byte[] body, Headers headers) {
5356
HttpExchange ex = mock(HttpExchange.class);
54-
Mockito.when(ex.getRequestMethod()).thenReturn(method);
55-
Mockito.when(ex.getRequestURI()).thenReturn(URI.create(path));
56-
Mockito.when(ex.getRequestHeaders()).thenReturn(headers);
57-
Mockito.when(ex.getRequestBody()).thenReturn(new ByteArrayInputStream(body));
57+
when(ex.getRequestMethod()).thenReturn(method);
58+
when(ex.getRequestURI()).thenReturn(URI.create(path));
59+
when(ex.getRequestHeaders()).thenReturn(headers);
60+
when(ex.getRequestBody()).thenReturn(new ByteArrayInputStream(body));
5861
return ex;
5962
}
6063

@@ -140,21 +143,21 @@ void successPathBindsRequestContextDuringChain() throws Exception {
140143
AtomicReference<Map<String, String>> seenPathParams = new AtomicReference<>();
141144

142145
Filter.Chain chain = mock(Filter.Chain.class);
143-
Mockito.doAnswer(
146+
doAnswer(
144147
inv -> {
145148
Request req = DispatchHandler.CURRENT.get();
146149
seenOpId.set(req.operationId());
147150
seenPathParams.set(req.pathParams());
148151
return null;
149152
})
150153
.when(chain)
151-
.doFilter(Mockito.any());
154+
.doFilter(any());
152155

153156
f.doFilter(ex, chain);
154157

155158
assertThat(seenOpId.get()).isEqualTo("get-user");
156159
assertThat(seenPathParams.get()).containsEntry("id", "42");
157-
Mockito.verify(chain).doFilter(ex);
160+
verify(chain).doFilter(ex);
158161
}
159162

160163
@Test
@@ -241,7 +244,7 @@ void integerQueryParamIsCoercedFromStringBeforeValidation() throws Exception {
241244
HttpExchange ex = exchange("GET", "/x?n=42", new byte[0]);
242245
Filter.Chain chain = mock(Filter.Chain.class);
243246
f.doFilter(ex, chain);
244-
Mockito.verify(chain).doFilter(ex);
247+
verify(chain).doFilter(ex);
245248
}
246249

247250
@Test
@@ -288,7 +291,7 @@ void numberQueryParamIsCoercedFromStringBeforeValidation() throws Exception {
288291
HttpExchange ex = exchange("GET", "/x?n=1.5", new byte[0]);
289292
Filter.Chain chain = mock(Filter.Chain.class);
290293
f.doFilter(ex, chain);
291-
Mockito.verify(chain).doFilter(ex);
294+
verify(chain).doFilter(ex);
292295
}
293296

294297
@Test
@@ -337,8 +340,8 @@ void booleanQueryParamCoercesTrueAndFalse() throws Exception {
337340
HttpExchange falseEx = exchange("GET", "/x?b=false", new byte[0]);
338341
f.doFilter(trueEx, trueChain);
339342
f.doFilter(falseEx, falseChain);
340-
Mockito.verify(trueChain).doFilter(trueEx);
341-
Mockito.verify(falseChain).doFilter(falseEx);
343+
verify(trueChain).doFilter(trueEx);
344+
verify(falseChain).doFilter(falseEx);
342345
}
343346

344347
@Test
@@ -383,15 +386,15 @@ void gzipRequestBodyIsInflatedBeforeValidation() throws Exception {
383386
AtomicReference<byte[]> seenBody = new AtomicReference<>();
384387
AtomicReference<String> seenEncoding = new AtomicReference<>("still here");
385388
Filter.Chain chain = mock(Filter.Chain.class);
386-
Mockito.doAnswer(
389+
doAnswer(
387390
inv -> {
388391
Request req = DispatchHandler.CURRENT.get();
389392
seenBody.set(req.bytes());
390393
seenEncoding.set(req.header("Content-Encoding").orElse(null));
391394
return null;
392395
})
393396
.when(chain)
394-
.doFilter(Mockito.any());
397+
.doFilter(any());
395398

396399
f.doFilter(ex, chain);
397400

0 commit comments

Comments
 (0)