Open
Conversation
📝 WalkthroughWalkthroughAdds GZIP response compression: new config fields and YAML defaults, a CompressionConfig wrapper, a Global CompressionFilter that gzips responses when client accepts gzip and body ≥ threshold, and unit tests covering parsing, thresholds, and header behavior. Changes
Sequence DiagramsequenceDiagram
participant Client
participant Filter as CompressionFilter
participant Chain as FilterChain
participant Config as ConfigLoader
participant Gzip as GzipProcessor
Client->>Filter: HTTP Request (may include Accept-Encoding: gzip)
Filter->>Config: isCompressionEnabled(), getMinCompressSize()
alt disabled or client doesn't accept gzip
Filter->>Chain: doFilter(req,res)
Chain-->>Filter: Response (uncompressed)
Filter->>Client: Response (uncompressed)
else enabled and client accepts gzip
Filter->>Chain: doFilter(req,res)
Chain-->>Filter: Response Body (bytes)
alt Body size >= minCompressSize
Filter->>Gzip: compress(body)
Gzip-->>Filter: compressed bytes
Filter->>Filter: set Content-Encoding: gzip
Filter->>Filter: set/append Vary: Accept-Encoding
Filter->>Client: Compressed Response
else Body size < minCompressSize
Filter->>Client: Uncompressed Response
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
src/test/java/org/juv25d/filter/CompressionFilterTest.java (2)
60-75: Assert compressed payload integrity, not only method callsCurrent tests can pass even if
setBodyreceives invalid bytes. Capture the body passed tosetBody, decompress it, and assert round-trip equality.✅ Suggested test pattern
+import org.mockito.ArgumentCaptor; +import java.util.zip.GZIPInputStream; +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import static org.junit.jupiter.api.Assertions.assertArrayEquals; @@ filter.doFilter(req, res, chain); - verify(res).setBody(any(byte[].class)); + ArgumentCaptor<byte[]> captor = ArgumentCaptor.forClass(byte[].class); + verify(res).setBody(captor.capture()); + byte[] inflated = gunzip(captor.getValue()); + assertArrayEquals(body, inflated); verify(res).setHeader("Content-Encoding", "gzip"); @@ + private byte[] gunzip(byte[] gz) throws IOException { + try (GZIPInputStream in = new GZIPInputStream(new ByteArrayInputStream(gz)); + ByteArrayOutputStream out = new ByteArrayOutputStream()) { + in.transferTo(out); + return out.toByteArray(); + } + }Also applies to: 93-121
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/org/juv25d/filter/CompressionFilterTest.java` around lines 60 - 75, Test currently only verifies that res.setBody was called without checking payload correctness; update shouldCompress_whenAcceptEncodingIsGzip in CompressionFilterTest to capture the byte[] argument passed to res.setBody (e.g., with an ArgumentCaptor<byte[]> or Mockito.argThat) after calling CompressionFilter.doFilter, then decompress that captured byte[] using GZIPInputStream and assert the decompressed bytes equal the original uncompressed body; do the same for the other tests mentioned (lines 93-121) that verify setBody to ensure round-trip integrity rather than only method calls, referencing CompressionFilter.doFilter and res.setBody in each updated test.
25-122: Add regression tests forgzip;q=and pre-encoded responsesPlease add cases for
Accept-Encoding: gzip;q=1.0and when response already hasContent-Encodingset, to lock in correct behavior and prevent double compression regressions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/org/juv25d/filter/CompressionFilterTest.java` around lines 25 - 122, Add two tests inside CompressionFilterTest: one that constructs CompressionFilter (e.g., new CompressionFilter(true, 100)), stubs req.headers() to include "Accept-Encoding" -> "gzip;q=1.0", stubs res.body() to return a compressible byte[] and asserts doFilter calls res.setBody(...) and sets "Content-Encoding" to "gzip"; and another that stubs req.headers() to accept gzip but stubs res.header("Content-Encoding") (or the response's existing headers) to a non-null value before calling doFilter and asserts that res.setBody(...) is NOT called (no additional compression) and existing Content-Encoding is preserved. Use the existing symbols CompressionFilter, doFilter(req, res, chain), req.headers(), res.body(), res.setBody(...), and res.setHeader(...) to locate and implement these tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/java/org/juv25d/filter/CompressionFilter.java`:
- Around line 61-69: The acceptsGzip method is too strict: it uses a
case-sensitive header lookup and only matches exact "gzip", so values like
"gzip;q=1.0" or different header-casing are missed. Update acceptsGzip to
retrieve the Accept-Encoding header in a case-insensitive way (or normalize
keys) and, when scanning tokens from acceptEncoding.split(","), trim and compare
the token's media-type portion case-insensitively (e.g. split each token on ';'
and check tokenPart.equalsIgnoreCase("gzip") or tokenPart.startsWith("gzip") for
robustness) instead of requiring an exact match; keep the null/empty guard and
anyStream logic around the tokens in the acceptsGzip method.
- Around line 42-45: In CompressionFilter, avoid double-compressing by checking
the response's existing "Content-Encoding" header before calling
compress(body)/res.setBody; if a non-empty Content-Encoding exists, skip
compression and do not set that header. When setting the "Vary" header, merge
with any existing Vary values instead of overwriting: read
res.getHeader("Vary"), append "Accept-Encoding" only if not already present, and
then update the header (use res.setHeader or equivalent). Ensure
res.setHeader("Content-Encoding","gzip") is only called when compression
actually occurs.
In `@src/main/java/org/juv25d/util/ConfigLoader.java`:
- Around line 126-127: The assignment to this.minCompressSize in ConfigLoader
currently parses compressionConfig.getOrDefault("min-compress-size", 1024)
directly and can accept negative values; change this to parse the value safely
(catch NumberFormatException), then enforce a lower bound (e.g. clamp to 0 or a
sensible default like 1024 if the parsed value is negative or invalid) before
assigning to the minCompressSize field so negative config cannot force
unintended compression.
---
Nitpick comments:
In `@src/test/java/org/juv25d/filter/CompressionFilterTest.java`:
- Around line 60-75: Test currently only verifies that res.setBody was called
without checking payload correctness; update
shouldCompress_whenAcceptEncodingIsGzip in CompressionFilterTest to capture the
byte[] argument passed to res.setBody (e.g., with an ArgumentCaptor<byte[]> or
Mockito.argThat) after calling CompressionFilter.doFilter, then decompress that
captured byte[] using GZIPInputStream and assert the decompressed bytes equal
the original uncompressed body; do the same for the other tests mentioned (lines
93-121) that verify setBody to ensure round-trip integrity rather than only
method calls, referencing CompressionFilter.doFilter and res.setBody in each
updated test.
- Around line 25-122: Add two tests inside CompressionFilterTest: one that
constructs CompressionFilter (e.g., new CompressionFilter(true, 100)), stubs
req.headers() to include "Accept-Encoding" -> "gzip;q=1.0", stubs res.body() to
return a compressible byte[] and asserts doFilter calls res.setBody(...) and
sets "Content-Encoding" to "gzip"; and another that stubs req.headers() to
accept gzip but stubs res.header("Content-Encoding") (or the response's existing
headers) to a non-null value before calling doFilter and asserts that
res.setBody(...) is NOT called (no additional compression) and existing
Content-Encoding is preserved. Use the existing symbols CompressionFilter,
doFilter(req, res, chain), req.headers(), res.body(), res.setBody(...), and
res.setHeader(...) to locate and implement these tests.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/main/java/org/juv25d/config/CompressionConfig.javasrc/main/java/org/juv25d/filter/CompressionFilter.javasrc/main/java/org/juv25d/util/ConfigLoader.javasrc/main/resources/application-properties.ymlsrc/test/java/org/juv25d/filter/CompressionFilterTest.java
Johan-jans
previously approved these changes
Feb 27, 2026
Johan-jans
left a comment
Johan-jans
left a comment
There was a problem hiding this comment.
Nice implementation, clean structure and good test coverage
OskarLundqvist33
previously approved these changes
Feb 28, 2026
OskarLundqvist33
left a comment
OskarLundqvist33
left a comment
There was a problem hiding this comment.
Great work, looks really clean and a great implementation!
One small nitpick, in src/main/java/org/juv25d/filter/CompressionFilter.java on row 44-45 there could be a small potential issue. This doesn't check weather Content-Encoding was already present before the compression, and if some other filter or handler already compressed (or otherwise encoded) the response, this filter would double compress it, which could be bad. Could probably just add a check to see if Content-Encoding header already exists to avoid double compression risk. Small risk but could be a nice safety. Otherwise awesome work!
DennSel
dismissed stale reviews from OskarLundqvist33 and Johan-jans
via
749a170
Author
Good catch, added a skip before compression: |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/main/java/org/juv25d/filter/CompressionFilter.java (1)
43-56:⚠️ Potential issue | 🟡 MinorHarden
Content-Encoding/Varyhandling to avoid false skips and duplicateVaryvalues.At Line 43, a blank
Content-Encodingstill blocks compression. At Line 53,Accept-Encodingis appended even if already present, producing duplicateVarytokens.🔧 Suggested fix
- if (res.getHeader("Content-Encoding") != null) { + String existingEncoding = res.getHeader("Content-Encoding"); + if (existingEncoding != null && !existingEncoding.isBlank()) { return; } @@ - String existingVary = res.getHeader("Vary"); - if (existingVary != null && !existingVary.isEmpty()) { - res.setHeader("Vary", existingVary + ", Accept-Encoding"); - } else { - res.setHeader("Vary", "Accept-Encoding"); - } + String existingVary = res.getHeader("Vary"); + if (existingVary == null || existingVary.isBlank()) { + res.setHeader("Vary", "Accept-Encoding"); + } else if (Arrays.stream(existingVary.split(",")) + .map(String::trim) + .noneMatch(v -> v.equalsIgnoreCase("Accept-Encoding"))) { + res.setHeader("Vary", existingVary + ", Accept-Encoding"); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/juv25d/filter/CompressionFilter.java` around lines 43 - 56, The current logic in CompressionFilter incorrectly skips compression when Content-Encoding is present but blank and blindly appends "Accept-Encoding" to Vary causing duplicates; change the Content-Encoding check to read the header into a variable and only return if the header is non-null and contains non-whitespace characters (e.g., header != null && !header.trim().isEmpty()); for the Vary handling, parse the existing Vary value from res.getHeader("Vary") into comma-separated tokens, trim and compare case-insensitively for "Accept-Encoding", and only append ", Accept-Encoding" (using res.setHeader) when that token is not already present, otherwise leave the header unchanged (use the existing compress(body), res.setBody and res.setHeader calls in CompressionFilter).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/java/org/juv25d/filter/CompressionFilter.java`:
- Around line 82-85: The Accept-Encoding parsing in CompressionFilter currently
treats entries like "gzip;q=0" as acceptable; update the logic that processes
acceptEncoding (the stream mapping that splits on ";" and trims) to parse any
"q=" parameter for each encoding and treat q=0 as unacceptable—i.e., for each
entry split into token and parameters, parse the q value (default 1.0 if
missing) and only consider the token equalIgnoreCase("gzip") when its q value is
> 0; adjust the stream to filter out q==0 entries before anyMatch.
---
Duplicate comments:
In `@src/main/java/org/juv25d/filter/CompressionFilter.java`:
- Around line 43-56: The current logic in CompressionFilter incorrectly skips
compression when Content-Encoding is present but blank and blindly appends
"Accept-Encoding" to Vary causing duplicates; change the Content-Encoding check
to read the header into a variable and only return if the header is non-null and
contains non-whitespace characters (e.g., header != null &&
!header.trim().isEmpty()); for the Vary handling, parse the existing Vary value
from res.getHeader("Vary") into comma-separated tokens, trim and compare
case-insensitively for "Accept-Encoding", and only append ", Accept-Encoding"
(using res.setHeader) when that token is not already present, otherwise leave
the header unchanged (use the existing compress(body), res.setBody and
res.setHeader calls in CompressionFilter).
Parse quality values in Accept-Encoding header to respect client preferences. Skip compression when gzip;q=0 is specified by client. Add payload integrity verification using round-trip decompression in tests. Fixes: - Respect Accept-Encoding quality values (gzip;q=0 should not compress) - Verify compressed data can be decompressed correctly - Prevent double compression when Content-Encoding already exists
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/main/java/org/juv25d/filter/CompressionFilter.java (2)
106-113: Consider using try-with-resources for consistency.While
ByteArrayOutputStreamdoesn't hold external resources, using try-with-resources ensures theGZIPOutputStreamis closed even if an exception occurs duringwrite().♻️ Suggested refactor
- private byte [] compress(byte [] data) throws IOException { - ByteArrayOutputStream byteStream = new ByteArrayOutputStream(); - GZIPOutputStream gzipstream = new GZIPOutputStream(byteStream); - - gzipstream.write(data); - gzipstream.close(); - return byteStream.toByteArray(); + private byte[] compress(byte[] data) throws IOException { + ByteArrayOutputStream byteStream = new ByteArrayOutputStream(); + try (GZIPOutputStream gzipStream = new GZIPOutputStream(byteStream)) { + gzipStream.write(data); + } + return byteStream.toByteArray(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/juv25d/filter/CompressionFilter.java` around lines 106 - 113, The compress(byte [] data) method currently constructs a ByteArrayOutputStream and GZIPOutputStream but closes the GZIPOutputStream only on the happy path; change it to use try-with-resources to ensure GZIPOutputStream (and optionally ByteArrayOutputStream) are closed on exceptions. Locate the compress method and wrap creation of GZIPOutputStream in a try-with-resources block (using GZIPOutputStream gzip = new GZIPOutputStream(byteStream)) so gzip.write(data) is executed inside the try and the method returns byteStream.toByteArray() after the try completes.
51-56: Vary header may contain duplicateAccept-Encodingvalues.The current logic appends
, Accept-Encodingwhen a Vary header exists, but doesn't check if it already containsAccept-Encoding. This could result inVary: Accept-Encoding, Accept-Encodingif another filter set the Vary header with that value.♻️ Suggested fix to prevent duplicates
String existingVary = res.getHeader("Vary"); if (existingVary != null && !existingVary.isEmpty()) { - res.setHeader("Vary", existingVary + ", Accept-Encoding"); + boolean alreadyHasAcceptEncoding = Arrays.stream(existingVary.split(",")) + .map(String::trim) + .anyMatch(v -> v.equalsIgnoreCase("Accept-Encoding")); + if (!alreadyHasAcceptEncoding) { + res.setHeader("Vary", existingVary + ", Accept-Encoding"); + } } else { res.setHeader("Vary", "Accept-Encoding"); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/juv25d/filter/CompressionFilter.java` around lines 51 - 56, The Vary header handling in CompressionFilter currently appends ", Accept-Encoding" without checking for existing tokens; update the logic in the method where res.getHeader("Vary") is read (in CompressionFilter) to first parse existingVary into comma-separated tokens, trim and compare tokens case-insensitively to see if "Accept-Encoding" is already present, and only append it when absent; then call res.setHeader("Vary", ... ) with either the original value or the original value plus ", Accept-Encoding" (ensuring proper trimming and a single separator) to avoid duplicate Accept-Encoding entries.src/test/java/org/juv25d/filter/CompressionFilterTest.java (1)
167-173: LGTM!The
gunziphelper is correctly implemented for test verification.Consider adding tests for:
- Double-compression prevention (when
Content-Encodingalready exists)- Vary header merging (when
Varyheader already exists)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/org/juv25d/filter/CompressionFilterTest.java` around lines 167 - 173, Add two unit tests in CompressionFilterTest to cover the scenarios mentioned: 1) testDoubleCompressionPrevention should exercise the filter with a response that already has a Content-Encoding header set and assert that the filter does not apply gzip compression (use the existing gunzip helper to validate no double-compression and verify Content-Encoding remains unchanged). 2) testVaryHeaderMerging should exercise the filter when the response already contains a Vary header and assert that the filter merges its own Vary value with the existing one rather than overwriting it (verify final Vary contains both values). Use existing test helpers and the gunzip method to inspect response payload where applicable and reuse request/response setup patterns from other tests in this class.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/java/org/juv25d/filter/CompressionFilter.java`:
- Around line 79-87: Run the code formatter to fix Spotless violations in
CompressionFilter.java: run `mvn spotless:apply` and commit the resulting
changes; specifically reformat the method that checks Accept-Encoding (the logic
using acceptEncoding,
Arrays.stream(...).map(String::trim).filter(this::isGzipWithQualityAboveZero).anyMatch(...))
so it conforms to project style (spacing, line breaks, imports) and re-run
Spotless to ensure no formatting errors remain before pushing.
- Around line 72-87: In acceptsGzip, guard against a null header name by
changing the header key check to ensure e.getKey() != null before calling
equalsIgnoreCase, and simplify the encoding stream by removing the redundant
map/filter/anyMatch sequence: after splitting and trimming the encodings,
directly use anyMatch(this::isGzipWithQualityAboveZero) instead of filtering
then checking equalsIgnoreCase("gzip") because isGzipWithQualityAboveZero
already enforces gzip and quality>0; refer to the acceptsGzip method and
isGzipWithQualityAboveZero when making these changes.
---
Nitpick comments:
In `@src/main/java/org/juv25d/filter/CompressionFilter.java`:
- Around line 106-113: The compress(byte [] data) method currently constructs a
ByteArrayOutputStream and GZIPOutputStream but closes the GZIPOutputStream only
on the happy path; change it to use try-with-resources to ensure
GZIPOutputStream (and optionally ByteArrayOutputStream) are closed on
exceptions. Locate the compress method and wrap creation of GZIPOutputStream in
a try-with-resources block (using GZIPOutputStream gzip = new
GZIPOutputStream(byteStream)) so gzip.write(data) is executed inside the try and
the method returns byteStream.toByteArray() after the try completes.
- Around line 51-56: The Vary header handling in CompressionFilter currently
appends ", Accept-Encoding" without checking for existing tokens; update the
logic in the method where res.getHeader("Vary") is read (in CompressionFilter)
to first parse existingVary into comma-separated tokens, trim and compare tokens
case-insensitively to see if "Accept-Encoding" is already present, and only
append it when absent; then call res.setHeader("Vary", ... ) with either the
original value or the original value plus ", Accept-Encoding" (ensuring proper
trimming and a single separator) to avoid duplicate Accept-Encoding entries.
In `@src/test/java/org/juv25d/filter/CompressionFilterTest.java`:
- Around line 167-173: Add two unit tests in CompressionFilterTest to cover the
scenarios mentioned: 1) testDoubleCompressionPrevention should exercise the
filter with a response that already has a Content-Encoding header set and assert
that the filter does not apply gzip compression (use the existing gunzip helper
to validate no double-compression and verify Content-Encoding remains
unchanged). 2) testVaryHeaderMerging should exercise the filter when the
response already contains a Vary header and assert that the filter merges its
own Vary value with the existing one rather than overwriting it (verify final
Vary contains both values). Use existing test helpers and the gunzip method to
inspect response payload where applicable and reuse request/response setup
patterns from other tests in this class.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/org/juv25d/filter/CompressionFilter.javasrc/test/java/org/juv25d/filter/CompressionFilterTest.java
Comment on lines
+72
to
+87
| private boolean acceptsGzip(HttpRequest req) { | ||
| String acceptEncoding = req.headers().entrySet().stream() | ||
| .filter(e -> e.getKey().equalsIgnoreCase("Accept-Encoding")) | ||
| .map(Map.Entry::getValue) | ||
| .findFirst() | ||
| .orElse(null); | ||
|
|
||
| if (acceptEncoding == null || acceptEncoding.isEmpty()) { | ||
| return false; | ||
| } | ||
|
|
||
| return Arrays.stream(acceptEncoding.split(",")) | ||
| .map(String::trim) | ||
| .filter(this::isGzipWithQualityAboveZero) | ||
| .anyMatch(e -> e.split(";")[0].trim().equalsIgnoreCase("gzip")); | ||
| } |
There was a problem hiding this comment.
Potential NPE and redundant filtering logic.
Two issues in acceptsGzip:
-
Line 74:
e.getKey().equalsIgnoreCase(...)may throw NPE if a map entry has a null key. Add a null guard. -
Lines 83-86: The logic is redundant.
isGzipWithQualityAboveZeroalready returnstrueonly when the encoding isgzipAND quality > 0. The subsequentanyMatchcheck forequalsIgnoreCase("gzip")is always true for items that passed the filter.
🐛 Proposed fix
private boolean acceptsGzip(HttpRequest req) {
String acceptEncoding = req.headers().entrySet().stream()
- .filter(e -> e.getKey().equalsIgnoreCase("Accept-Encoding"))
+ .filter(e -> e.getKey() != null && e.getKey().equalsIgnoreCase("Accept-Encoding"))
.map(Map.Entry::getValue)
.findFirst()
.orElse(null);
if (acceptEncoding == null || acceptEncoding.isEmpty()) {
return false;
}
return Arrays.stream(acceptEncoding.split(","))
.map(String::trim)
- .filter(this::isGzipWithQualityAboveZero)
- .anyMatch(e -> e.split(";")[0].trim().equalsIgnoreCase("gzip"));
+ .anyMatch(this::isGzipWithQualityAboveZero);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/org/juv25d/filter/CompressionFilter.java` around lines 72 - 87,
In acceptsGzip, guard against a null header name by changing the header key
check to ensure e.getKey() != null before calling equalsIgnoreCase, and simplify
the encoding stream by removing the redundant map/filter/anyMatch sequence:
after splitting and trimming the encodings, directly use
anyMatch(this::isGzipWithQualityAboveZero) instead of filtering then checking
equalsIgnoreCase("gzip") because isGzipWithQualityAboveZero already enforces
gzip and quality>0; refer to the acceptsGzip method and
isGzipWithQualityAboveZero when making these changes.
Check for non-blank Content-Encoding to avoid skipping compression when header exists but is empty. Prevent duplicate Vary header values by checking if Accept-Encoding already exists. Use try-with-resources for GZIPOutputStream to ensure proper resource cleanup. Fixes: - Skip compression only if Content-Encoding has actual value - Avoid duplicate Accept-Encoding in Vary header - Ensure GZIPOutputStream is closed even on write failure
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/main/java/org/juv25d/filter/CompressionFilter.java (1)
75-90:⚠️ Potential issue | 🟡 MinorPotential NPE and redundant filtering logic.
Two issues in
acceptsGzip:
Line 77:
e.getKey().equalsIgnoreCase(...)will throw NPE if a map entry has a null key. Add a null guard.Lines 88-89: The logic is redundant.
isGzipWithQualityAboveZeroalready returnstrueonly when the encoding isgzipAND quality > 0. The subsequentanyMatchcheck forequalsIgnoreCase("gzip")is unnecessary.🐛 Proposed fix
private boolean acceptsGzip(HttpRequest req) { String acceptEncoding = req.headers().entrySet().stream() - .filter(e -> e.getKey().equalsIgnoreCase("Accept-Encoding")) + .filter(e -> e.getKey() != null && e.getKey().equalsIgnoreCase("Accept-Encoding")) .map(Map.Entry::getValue) .findFirst() .orElse(null); if (acceptEncoding == null || acceptEncoding.isEmpty()) { return false; } return Arrays.stream(acceptEncoding.split(",")) .map(String::trim) - .filter(this::isGzipWithQualityAboveZero) - .anyMatch(e -> e.split(";")[0].trim().equalsIgnoreCase("gzip")); + .anyMatch(this::isGzipWithQualityAboveZero); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/juv25d/filter/CompressionFilter.java` around lines 75 - 90, In acceptsGzip(HttpRequest req) guard against null map keys and simplify the stream: when filtering headers use a null-safe check (e.getKey() != null && e.getKey().equalsIgnoreCase("Accept-Encoding")) or otherwise handle null keys before equalsIgnoreCase, and replace the final two-stage filter+anyMatch with a single anyMatch(this::isGzipWithQualityAboveZero) since isGzipWithQualityAboveZero already checks for "gzip" and quality>0; keep references to acceptsGzip and isGzipWithQualityAboveZero to locate the changes.
🧹 Nitpick comments (3)
src/main/java/org/juv25d/filter/CompressionFilter.java (3)
18-18: Minor formatting: missing space before{.Add a space between
Filterand{to follow standard Java formatting conventions.-public class CompressionFilter implements Filter{ +public class CompressionFilter implements Filter {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/juv25d/filter/CompressionFilter.java` at line 18, The class declaration "public class CompressionFilter implements Filter{" is missing a space before the opening brace; update the declaration in CompressionFilter (the class named CompressionFilter that implements the Filter interface) to "public class CompressionFilter implements Filter {" to follow standard Java formatting.
109-109: Minor formatting: inconsistent array type spacing.The array type should use
byte[]without spaces for consistency with Java conventions.-private byte [] compress(byte [] data) throws IOException { +private byte[] compress(byte[] data) throws IOException {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/juv25d/filter/CompressionFilter.java` at line 109, In CompressionFilter.change the method signature for compress to use standard Java array spacing: replace the inconsistent "byte [] compress(byte [] data)" with "byte[] compress(byte[] data)" (update both the return type and parameter types in the compress method declaration and any matching occurrences to maintain consistency).
97-106: Silent exception handling may cause unintended behavior.When
q=is present but malformed (e.g.,gzip;q=invalid), theNumberFormatExceptionis caught and the method falls through toreturn true(line 106), treating it as acceptable. Consider returningfalsefor malformed quality values instead, or explicitly document this lenient behavior.try { double quality = Double.parseDouble(q.substring(2)); return quality > 0; - } catch (NumberFormatException ignored) {} + } catch (NumberFormatException e) { + return false; // Malformed quality value, reject + } } } return true;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/juv25d/filter/CompressionFilter.java` around lines 97 - 106, The code in CompressionFilter that parses the "q=" quality (the block handling parts, q, and parsing into double quality) currently swallows NumberFormatException and falls through to return true, treating malformed values as acceptable; change the catch to return false (or explicitly return false when Double.parseDouble fails) so malformed "q=" values like "gzip;q=invalid" are treated as unacceptable, or alternatively log the parse error then return false from the method that contains this parsing logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/main/java/org/juv25d/filter/CompressionFilter.java`:
- Around line 75-90: In acceptsGzip(HttpRequest req) guard against null map keys
and simplify the stream: when filtering headers use a null-safe check
(e.getKey() != null && e.getKey().equalsIgnoreCase("Accept-Encoding")) or
otherwise handle null keys before equalsIgnoreCase, and replace the final
two-stage filter+anyMatch with a single
anyMatch(this::isGzipWithQualityAboveZero) since isGzipWithQualityAboveZero
already checks for "gzip" and quality>0; keep references to acceptsGzip and
isGzipWithQualityAboveZero to locate the changes.
---
Nitpick comments:
In `@src/main/java/org/juv25d/filter/CompressionFilter.java`:
- Line 18: The class declaration "public class CompressionFilter implements
Filter{" is missing a space before the opening brace; update the declaration in
CompressionFilter (the class named CompressionFilter that implements the Filter
interface) to "public class CompressionFilter implements Filter {" to follow
standard Java formatting.
- Line 109: In CompressionFilter.change the method signature for compress to use
standard Java array spacing: replace the inconsistent "byte [] compress(byte []
data)" with "byte[] compress(byte[] data)" (update both the return type and
parameter types in the compress method declaration and any matching occurrences
to maintain consistency).
- Around line 97-106: The code in CompressionFilter that parses the "q=" quality
(the block handling parts, q, and parsing into double quality) currently
swallows NumberFormatException and falls through to return true, treating
malformed values as acceptable; change the catch to return false (or explicitly
return false when Double.parseDouble fails) so malformed "q=" values like
"gzip;q=invalid" are treated as unacceptable, or alternatively log the parse
error then return false from the method that contains this parsing logic.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/main/java/org/juv25d/filter/CompressionFilter.java (1)
75-90:⚠️ Potential issue | 🟡 MinorHarden
acceptsGzipnull-safety and simplify redundant matching.Line 77 can throw if a header key is null, and Line 88-89 is redundant because
isGzipWithQualityAboveZeroalready enforces gzip withq>0.🔧 Suggested patch
private boolean acceptsGzip(HttpRequest req) { String acceptEncoding = req.headers().entrySet().stream() - .filter(e -> e.getKey().equalsIgnoreCase("Accept-Encoding")) + .filter(e -> e.getKey() != null && e.getKey().equalsIgnoreCase("Accept-Encoding")) .map(Map.Entry::getValue) .findFirst() .orElse(null); if (acceptEncoding == null || acceptEncoding.isEmpty()) { return false; } return Arrays.stream(acceptEncoding.split(",")) .map(String::trim) - .filter(this::isGzipWithQualityAboveZero) - .anyMatch(e -> e.split(";")[0].trim().equalsIgnoreCase("gzip")); + .anyMatch(this::isGzipWithQualityAboveZero); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/juv25d/filter/CompressionFilter.java` around lines 75 - 90, The acceptsGzip method can NPE when a header key is null and performs a redundant split/match because isGzipWithQualityAboveZero already ensures the token is gzip with q>0; update acceptsGzip to safely compare header names (use "Accept-Encoding".equalsIgnoreCase(key) or guard key != null) when filtering the headers stream to avoid NPE, and then, after splitting the header value by commas and trimming, simply call isGzipWithQualityAboveZero and anyMatch on that result (remove the extra e.split(";")[0] equality check). Keep references to acceptsGzip and isGzipWithQualityAboveZero when applying these changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/main/java/org/juv25d/filter/CompressionFilter.java`:
- Around line 75-90: The acceptsGzip method can NPE when a header key is null
and performs a redundant split/match because isGzipWithQualityAboveZero already
ensures the token is gzip with q>0; update acceptsGzip to safely compare header
names (use "Accept-Encoding".equalsIgnoreCase(key) or guard key != null) when
filtering the headers stream to avoid NPE, and then, after splitting the header
value by commas and trimming, simply call isGzipWithQualityAboveZero and
anyMatch on that result (remove the extra e.split(";")[0] equality check). Keep
references to acceptsGzip and isGzipWithQualityAboveZero when applying these
changes.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/main/java/org/juv25d/util/ConfigLoader.java (1)
13-14: Extract compression defaults into named constants.
1024and100are repeated magic numbers; constants would make this easier to maintain and less error-prone.♻️ Proposed refactor
public class ConfigLoader { `@Nullable` private static ConfigLoader instance; + private static final int DEFAULT_MIN_COMPRESS_SIZE = 1024; + private static final int MIN_ALLOWED_COMPRESS_SIZE = 100; + private int port; private int minCompressSize; @@ - this.minCompressSize = 1024; + this.minCompressSize = DEFAULT_MIN_COMPRESS_SIZE; @@ int parsedMinCompressSize = - Integer.parseInt(String.valueOf(compressionConfig.getOrDefault("min-compress-size", 1024))); - this.minCompressSize = Math.max(100, parsedMinCompressSize); + Integer.parseInt( + String.valueOf(compressionConfig.getOrDefault("min-compress-size", DEFAULT_MIN_COMPRESS_SIZE)) + ); + this.minCompressSize = Math.max(MIN_ALLOWED_COMPRESS_SIZE, parsedMinCompressSize); }Also applies to: 61-62, 133-135
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/juv25d/util/ConfigLoader.java` around lines 13 - 14, Extract the magic numbers 1024 and 100 into named constants in ConfigLoader (e.g., private static final int DEFAULT_MIN_COMPRESS_SIZE = 1024; private static final int DEFAULT_MAX_COMPRESS_PERCENT = 100) and use these constants to initialize/compare the existing fields (minCompressSize and whatever field or parameter uses the 100 percent value) and replace all literal occurrences within ConfigLoader (including the default field initialization, any constructors, and validation logic currently using 1024 or 100) so the class consistently references the new constants (update references in methods that parse or validate compression settings such as setMinCompressSize / setMaxCompressPercent or similar).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/main/java/org/juv25d/util/ConfigLoader.java`:
- Around line 13-14: Extract the magic numbers 1024 and 100 into named constants
in ConfigLoader (e.g., private static final int DEFAULT_MIN_COMPRESS_SIZE =
1024; private static final int DEFAULT_MAX_COMPRESS_PERCENT = 100) and use these
constants to initialize/compare the existing fields (minCompressSize and
whatever field or parameter uses the 100 percent value) and replace all literal
occurrences within ConfigLoader (including the default field initialization, any
constructors, and validation logic currently using 1024 or 100) so the class
consistently references the new constants (update references in methods that
parse or validate compression settings such as setMinCompressSize /
setMaxCompressPercent or similar).
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/org/juv25d/filter/CompressionFilter.javasrc/main/java/org/juv25d/util/ConfigLoader.javasrc/main/resources/application-properties.yml
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/org/juv25d/filter/CompressionFilter.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #144
Implemented a CompressionFilter that compresses HTTP response bodies using GZIP encoding.
Changes:
Results:
Summary by CodeRabbit
New Features
Tests