-
Notifications
You must be signed in to change notification settings - Fork 0
Feature/144 gzip compression filter #149
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
DennSel
wants to merge
26
commits into
main
Choose a base branch
from
feature/144-gzip-compression-filter
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
26 commits
Select commit
Hold shift + click to select a range
5012693
Add CompressionConfig
DennSel e171a44
add fields, @Global annotation, constructors for default config and t…
DennSel e024652
add acceptsGzip helper with header parsing
DennSel d294b3b
add compress helper using GZIPOutputStream
DennSel 4eab8ee
implement doFilter with early exit and GZIP compression logic
DennSel a23e7fe
add test for disabled filter
DennSel f52f884
add test for missing Accept-Encoding header
DennSel 1e96a2b
add test for non-gzip Accept-Encoding header
DennSel 0fd94d8
add test for compression of large response body
DennSel 84bd376
add test for small body below compression threshold
DennSel d8da87a
add test for case-insensitive gzip header matching
DennSel 5414e7c
add test to verify compression at exact threshold boundary
DennSel 3685a15
add compression fields to ConfigLoader
DennSel 6c1a919
parse compression settings from application-properties.yml
DennSel 71a5dd7
add getters for compression settings in ConfigLoader
DennSel ba0eff8
add CompressionConfig now reading from ConfigLoader
DennSel 12beaf4
prevent double-compression and preserve existing Vary header
DennSel f0a0597
handle case-insensitive header key and gzip quality values
DennSel 89a758a
no negative min-compress-size values accepted
DennSel 4b764fe
enforce minimum compress size of 100 bytes
DennSel 79463fb
Merge branch 'main' into feature/144-gzip-compression-filter
DennSel 749a170
Add a check before compression to prevent double encoding
DennSel 5d6df15
Fix Accept-Encoding quality value parsing and improve compression tests
DennSel d2ae1ab
Spotless
DennSel e188d47
Fix edge cases in Content-Encoding and Vary header handling
DennSel 1b862fa
Merge branch 'main' into feature/144-gzip-compression-filter
DennSel File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| package org.juv25d.config; | ||
|
|
||
| import org.juv25d.util.ConfigLoader; | ||
|
|
||
| public class CompressionConfig { | ||
| private final boolean enabled; | ||
| private final int minCompressSize; | ||
|
|
||
| public CompressionConfig() { | ||
| ConfigLoader config = ConfigLoader.getInstance(); | ||
| this.enabled = config.isCompressionEnabled(); | ||
| this.minCompressSize = config.getMinCompressSize(); | ||
| } | ||
|
|
||
| public boolean isEnabled() { | ||
| return enabled; | ||
| } | ||
|
|
||
| public int getMinCompressSize() { | ||
| return minCompressSize; | ||
| } | ||
| } |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,116 @@ | ||
| package org.juv25d.filter; | ||
|
|
||
| import org.juv25d.config.CompressionConfig; | ||
| import org.juv25d.filter.annotation.Global; | ||
| import org.juv25d.http.HttpRequest; | ||
| import org.juv25d.http.HttpResponse; | ||
| import org.juv25d.logging.ServerLogging; | ||
|
|
||
| import java.io.ByteArrayOutputStream; | ||
| import java.io.IOException; | ||
| import java.util.Arrays; | ||
| import java.util.Map; | ||
| import java.util.logging.Logger; | ||
| import java.util.zip.GZIPOutputStream; | ||
|
|
||
|
|
||
| @Global(order = 10) | ||
| public class CompressionFilter implements Filter{ | ||
| private static final Logger LOGGER = ServerLogging.getLogger(); | ||
|
|
||
| private final boolean enabled; | ||
| private final int minCompressSize; | ||
|
|
||
| @Override | ||
| public void doFilter(HttpRequest req, HttpResponse res, FilterChain chain) throws IOException { | ||
| if (!enabled) { | ||
| chain.doFilter(req, res); | ||
| return; | ||
| } | ||
|
|
||
| if (!acceptsGzip(req)) { | ||
| chain.doFilter(req, res); | ||
| return; | ||
| } | ||
|
|
||
| chain.doFilter(req, res); | ||
|
|
||
| byte[] body = res.body(); | ||
| if (body.length < minCompressSize) { | ||
| return; | ||
| } | ||
|
|
||
| String existingEncoding = res.getHeader("Content-Encoding"); | ||
| if (existingEncoding != null && !existingEncoding.isBlank()) { | ||
| return; | ||
| } | ||
|
|
||
| byte[] compressed = compress(body); | ||
| res.setBody(compressed); | ||
| res.setHeader("Content-Encoding", "gzip"); | ||
|
|
||
| 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"); | ||
| } | ||
|
|
||
| LOGGER.info("Compressed " + body.length + " bytes to " + compressed.length + " bytes"); | ||
| } | ||
|
|
||
| public CompressionFilter() { | ||
| CompressionConfig config = new CompressionConfig(); | ||
| this.enabled = config.isEnabled(); | ||
| this.minCompressSize = config.getMinCompressSize(); | ||
| } | ||
|
|
||
| public CompressionFilter(boolean enabled, int minCompressSize) { | ||
| this.enabled = enabled; | ||
| this.minCompressSize = minCompressSize; | ||
| } | ||
|
|
||
| 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")); | ||
| } | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
| private boolean isGzipWithQualityAboveZero(String encoding) { | ||
| String[] parts = encoding.split(";"); | ||
| String name = parts[0].trim(); | ||
| if (!name.equalsIgnoreCase("gzip")) return false; | ||
|
|
||
| if (parts.length > 1) { | ||
| String q = parts[1].trim(); | ||
| if (q.startsWith("q=")) { | ||
| try { | ||
| double quality = Double.parseDouble(q.substring(2)); | ||
| return quality > 0; | ||
| } catch (NumberFormatException ignored) {} | ||
| } | ||
| } | ||
| return true; | ||
| } | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
| private byte [] compress(byte [] data) throws IOException { | ||
| ByteArrayOutputStream byteStream = new ByteArrayOutputStream(); | ||
| try (GZIPOutputStream gzipstream = new GZIPOutputStream(byteStream)) { | ||
| gzipstream.write(data); | ||
| } | ||
| return byteStream.toByteArray(); | ||
| } | ||
| } | ||
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
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
174 changes: 174 additions & 0 deletions
174
src/test/java/org/juv25d/filter/CompressionFilterTest.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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,174 @@ | ||
| package org.juv25d.filter; | ||
|
|
||
| import org.junit.jupiter.api.Test; | ||
| import org.junit.jupiter.api.extension.ExtendWith; | ||
| import org.juv25d.http.HttpRequest; | ||
| import org.juv25d.http.HttpResponse; | ||
| import org.mockito.ArgumentCaptor; | ||
| import org.mockito.Mock; | ||
| import org.mockito.junit.jupiter.MockitoExtension; | ||
|
|
||
| import java.io.ByteArrayInputStream; | ||
| import java.io.ByteArrayOutputStream; | ||
| import java.io.IOException; | ||
| import java.util.Map; | ||
| import java.util.zip.GZIPInputStream; | ||
|
|
||
| import static org.junit.jupiter.api.Assertions.assertArrayEquals; | ||
| import static org.mockito.Mockito.*; | ||
|
|
||
| @ExtendWith(MockitoExtension.class) | ||
| public class CompressionFilterTest { | ||
|
|
||
| @Mock | ||
| private HttpRequest req; | ||
| @Mock | ||
| private HttpResponse res; | ||
| @Mock | ||
| private FilterChain chain; | ||
|
|
||
| @Test | ||
| void shouldNotCompress_whenDisabled() throws IOException { | ||
| CompressionFilter filter = new CompressionFilter(false, 1024); | ||
|
|
||
| filter.doFilter(req, res, chain); | ||
|
|
||
| verify(chain).doFilter(req, res); | ||
| verifyNoMoreInteractions(res); | ||
| } | ||
|
|
||
| @Test | ||
| void shouldNotCompress_whenNoAcceptEncoding() throws IOException { | ||
| CompressionFilter filter = new CompressionFilter(true, 1024); | ||
| when(req.headers()).thenReturn(Map.of()); | ||
|
|
||
| filter.doFilter(req, res, chain); | ||
|
|
||
| verify(chain).doFilter(req, res); | ||
| verify(res, never()).setBody(any()); | ||
| } | ||
|
|
||
| @Test | ||
| void shouldNotCompress_whenAcceptEncodingIsNotGzip() throws IOException { | ||
| CompressionFilter filter = new CompressionFilter(true, 1024); | ||
| when(req.headers()).thenReturn(Map.of( | ||
| "Accept-Encoding", "deflate, br" | ||
| )); | ||
|
|
||
| filter.doFilter(req, res, chain); | ||
|
|
||
| verify(chain).doFilter(req, res); | ||
| verify(res, never()).setBody(any()); | ||
| } | ||
|
|
||
| @Test | ||
| void shouldCompress_whenAcceptEncodingIsGzip() throws IOException { | ||
| CompressionFilter filter = new CompressionFilter(true, 100); | ||
| when(req.headers()).thenReturn(Map.of( | ||
| "Accept-Encoding", "gzip, deflate" | ||
| )); | ||
|
|
||
| byte[] body = "Hello, world!".repeat(100).getBytes(); | ||
| when(res.body()).thenReturn(body); | ||
|
|
||
| filter.doFilter(req, res, chain); | ||
|
|
||
| ArgumentCaptor<byte[]> captor = ArgumentCaptor.forClass(byte[].class); | ||
| verify(res).setBody(captor.capture()); | ||
| byte[] decompressed = gunzip(captor.getValue()); | ||
| assertArrayEquals(body, decompressed); | ||
| verify(res).setHeader("Content-Encoding", "gzip"); | ||
| verify(res).setHeader("Vary", "Accept-Encoding"); | ||
| } | ||
|
|
||
| @Test | ||
| void shouldNotCompress_whenBodyIsSmallerThanThreshold() throws IOException { | ||
| CompressionFilter filter = new CompressionFilter(true, 1024); | ||
| when(req.headers()).thenReturn(Map.of( | ||
| "Accept-Encoding", "gzip" | ||
| )); | ||
|
|
||
| when(res.body()).thenReturn("small".getBytes()); | ||
|
|
||
| filter.doFilter(req, res, chain); | ||
|
|
||
| verify(chain).doFilter(req, res); | ||
| verify(res, never()).setBody(any()); | ||
| } | ||
|
|
||
| @Test | ||
| void shouldCompress_whenAcceptEncodingIsUpperCase() throws IOException { | ||
| CompressionFilter filter = new CompressionFilter(true, 100); | ||
| when(req.headers()).thenReturn(Map.of( | ||
| "Accept-Encoding", "GZIP" | ||
| )); | ||
|
|
||
| byte[] body = "Hello, world!".repeat(100).getBytes(); | ||
| when(res.body()).thenReturn(body); | ||
|
|
||
| filter.doFilter(req, res, chain); | ||
|
|
||
| ArgumentCaptor<byte[]> captor = ArgumentCaptor.forClass(byte[].class); | ||
| verify(res).setBody(captor.capture()); | ||
| byte[] decompressed = gunzip(captor.getValue()); | ||
| assertArrayEquals(body, decompressed); | ||
| verify(res).setHeader("Content-Encoding", "gzip"); | ||
| } | ||
|
|
||
| @Test | ||
| void shouldCompress_whenBodyIsExactlyThreshold() throws IOException { | ||
| CompressionFilter filter = new CompressionFilter(true, 5); | ||
| when(req.headers()).thenReturn(Map.of( | ||
| "Accept-Encoding", "gzip" | ||
| )); | ||
|
|
||
| when(res.body()).thenReturn("Hello".getBytes()); | ||
|
|
||
| filter.doFilter(req, res, chain); | ||
|
|
||
| ArgumentCaptor<byte[]> captor = ArgumentCaptor.forClass(byte[].class); | ||
| verify(res).setBody(captor.capture()); | ||
| byte[] decompressed = gunzip(captor.getValue()); | ||
| assertArrayEquals("Hello".getBytes(), decompressed); | ||
| verify(res).setHeader("Content-Encoding", "gzip"); | ||
| } | ||
|
|
||
| @Test | ||
| void shouldNotCompress_whenGzipWithQualityZero() throws IOException { | ||
| CompressionFilter filter = new CompressionFilter(true, 100); | ||
| when(req.headers()).thenReturn(Map.of( | ||
| "Accept-Encoding", "gzip;q=0" | ||
| )); | ||
|
|
||
| filter.doFilter(req, res, chain); | ||
|
|
||
| verify(chain).doFilter(req, res); | ||
| verify(res, never()).setBody(any()); | ||
| } | ||
|
|
||
| @Test | ||
| void shouldCompress_whenGzipWithQualityAboveZero() throws IOException { | ||
| CompressionFilter filter = new CompressionFilter(true, 100); | ||
| when(req.headers()).thenReturn(Map.of( | ||
| "Accept-Encoding", "gzip;q=0.5" | ||
| )); | ||
| byte[] body = "Hello, world!".repeat(100).getBytes(); | ||
| when(res.body()).thenReturn(body); | ||
|
|
||
| filter.doFilter(req, res, chain); | ||
|
|
||
| ArgumentCaptor<byte[]> captor = ArgumentCaptor.forClass(byte[].class); | ||
| verify(res).setBody(captor.capture()); | ||
| byte[] decompressed = gunzip(captor.getValue()); | ||
| assertArrayEquals(body, decompressed); | ||
| 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(); | ||
| } | ||
| } | ||
| } |
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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