Repository navigation
feat(rfc9530): implement RFC 9530 Digest Fields for Jetty and Undertow - #163
Conversation
Adds response digest generation (Repr-Digest + Content-Digest) at the server layer so post-compression hashes can be computed correctly, plus request-side DigestValidationMiddleware in enkan-web. Architecture: - Jetty: DigestFilter (inside ServletContextHandler, pre-compression → Repr-Digest) + ContentDigestHandler (Wrapper outside CompressionHandler, post-compression → Content-Digest) - Undertow: DigestConduit registered in appHandler (pre-compression → Repr-Digest) + DigestOuterHandler wrapping EncodingHandler (post- compression → Content-Digest) - enkan-web: DigestFieldsUtils (compute/negotiate helpers) + DigestValidationMiddleware (request body validation, 400 on mismatch) - Want-Content-Digest / Want-Repr-Digest negotiation via SF Dictionary - Supported algorithms: sha-256, sha-512 - Streaming bodies (InputStream/File/StreamingBody) are skipped Closes #132 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- DigestConduit.terminateWrites(): break on n<=0 in flush loop to avoid busy-spin on partial writes - DigestConduit.writeFinal(): add comment explaining why terminateWrites() is not called here (framework calls it separately; calling it from writeFinal would cause double header-set and double byte-forward) - DigestFilter.PARAM_COMPRESSED: expand Javadoc to document coupling constraint with JettyAdapter's compress? option - DigestFilter.BufferingResponseWrapper.flushBuffer(): expand comment to explain why commit is suppressed (headers must remain mutable until after chain.doFilter returns) - JettyComponent.enableDigestFields(): replace fully-qualified DigestFieldsUtils reference with proper import statement - Remove duplicate web.DIGEST_ALGORITHM_UNSUPPORTED keys from enkan-component-jetty and enkan-component-undertow properties files; the key is already defined in enkan-web which both modules depend on - UndertowAdapterDigestTest.startWith(): remove unused port local variable Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- negotiateAlgorithm: exclude priority=0 per RFC 9530 §4 ("a value of 0
means the client does not want that algorithm"); add test for zero-priority
exclusion and mixed zero/non-zero case
- DigestConduit: replace String headerName field with HttpString header to
eliminate the terminateWrites() switch; update DigestOuterHandler and
UndertowAdapter callers accordingly; add REPR_DIGEST/CONTENT_DIGEST
constants to UndertowAdapter and DigestOuterHandler
- UndertowAdapter: remove redundant final String algo copies (reprAlgo /
contentAlgo are already effectively final); replace fully-qualified
DigestConduit references with import
- DigestFilter.BufferingResponseWrapper: enforce getOutputStream/getWriter
mutual exclusion per Servlet spec; write PrintWriter directly to buffer
to avoid the getOutputStream indirection
- DigestValidationMiddleware.verifyDigest: fix Javadoc ("all supported
algorithms match" rather than "at least one")
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Implements RFC 9530 Digest Fields support across the Enkan web stack by adding digest computation utilities, request-side digest validation middleware, and server-adapter integrations for Jetty and Undertow (including negotiation via Want-* headers and compression-aware digesting).
Changes:
- Added
DigestFieldsUtils(compute/serialize digests, negotiate algorithm) andDigestValidationMiddlewarefor request header validation. - Integrated response-side
Repr-Digest/Content-Digestgeneration into Jetty (filter + handler) and Undertow (conduit + outer handler), with algorithm negotiation. - Added unit and adapter integration tests covering core digest behavior and adapter header behavior.
Reviewed changes
Copilot reviewed 16 out of 18 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| enkan-web/src/main/java/enkan/web/util/DigestFieldsUtils.java | Digest header computation + Want-* negotiation utilities. |
| enkan-web/src/main/java/enkan/web/middleware/DigestValidationMiddleware.java | Request middleware validating Content-Digest / Repr-Digest by buffering/restoring body. |
| enkan-web/src/main/resources/META-INF/misconfiguration.properties | Adds web.DIGEST_ALGORITHM_UNSUPPORTED message keys. |
| enkan-web/src/main/resources/META-INF/misconfiguration_ja.properties | Japanese messages for unsupported digest algorithm. |
| enkan-web/src/test/java/enkan/web/util/DigestFieldsUtilsTest.java | Unit tests for digest formatting, known hashes, negotiation, JCA mapping. |
| enkan-web/src/test/java/enkan/web/middleware/DigestValidationMiddlewareTest.java | Unit tests for request validation outcomes and body restoration. |
| enkan-component-undertow/src/main/java/enkan/component/undertow/UndertowComponent.java | Adds enableDigestFields(...) configuration API and option wiring. |
| enkan-component-undertow/src/main/java/enkan/adapter/UndertowAdapter.java | Adds Undertow digest wrapper registration and compression-aware handler wrapping. |
| enkan-component-undertow/src/main/java/enkan/adapter/digest/DigestOuterHandler.java | Registers Content-Digest conduit outside encoding handler for post-compression bytes. |
| enkan-component-undertow/src/main/java/enkan/adapter/digest/DigestConduit.java | Buffers response bytes, computes digest, sets header, forwards buffered bytes. |
| enkan-component-undertow/src/test/java/enkan/adapter/UndertowAdapterDigestTest.java | Undertow integration tests for digest headers and negotiation. |
| enkan-component-jetty/src/main/java/enkan/component/jetty/JettyComponent.java | Adds enableDigestFields(...) configuration API and option wiring. |
| enkan-component-jetty/src/main/java/enkan/adapter/JettyAdapter.java | Installs digest filter and (when compressing) wraps compression handler with content-digest handler. |
| enkan-component-jetty/src/main/java/enkan/adapter/digest/DigestFilter.java | Servlet filter buffering pre-compression bytes to set Repr-Digest (+ Content-Digest when no compression). |
| enkan-component-jetty/src/main/java/enkan/adapter/digest/ContentDigestHandler.java | Jetty handler buffering post-compression bytes to set Content-Digest. |
| enkan-component-jetty/src/test/java/enkan/adapter/JettyAdapterDigestTest.java | Jetty integration tests for digest headers and negotiation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| public static String negotiateAlgorithm(String wantHeaderValue, String defaultAlgorithm) { | ||
| if (wantHeaderValue == null) return defaultAlgorithm; | ||
| SfDictionary dict = StructuredFields.parseDictionary(wantHeaderValue); | ||
| return dict.members().entrySet().stream() | ||
| .filter(e -> SUPPORTED_ALGORITHMS.contains(e.getKey())) | ||
| .filter(e -> e.getValue() instanceof SfItem item | ||
| && item.value() instanceof SfValue.SfInteger i | ||
| && i.value() > 0) // RFC 9530 §4: priority 0 means "not wanted" | ||
| .max(Comparator.comparingLong( | ||
| e -> ((SfValue.SfInteger) ((SfItem) e.getValue()).value()).value())) | ||
| .map(Map.Entry::getKey) | ||
| .orElse(null); | ||
| } |
There was a problem hiding this comment.
negotiateAlgorithm calls StructuredFields.parseDictionary(...), which throws SfParseException on malformed/invalid input. Since Want-* headers are client-controlled, a bad header will currently bubble up and likely produce a 500. Consider catching SfParseException here and falling back to defaultAlgorithm (or null to omit) to make negotiation robust against malformed Want-Content-Digest / Want-Repr-Digest values.
| @Override | ||
| public void terminateWrites() throws IOException { | ||
| byte[] allBytes = buffer.toByteArray(); | ||
| String digestValue = DigestFieldsUtils.computeDigestHeader(allBytes, algorithm); | ||
| exchange.getResponseHeaders().put(header, digestValue); | ||
|
|
||
| // Write all buffered bytes to the underlying conduit, then terminate. | ||
| // super.write() may not consume all bytes in one call (partial writes); | ||
| // break on <= 0 to avoid a busy-spin if the conduit stalls unexpectedly. | ||
| if (allBytes.length > 0) { | ||
| ByteBuffer buf = ByteBuffer.wrap(allBytes); | ||
| while (buf.hasRemaining()) { | ||
| int n = super.write(buf); | ||
| if (n <= 0) break; | ||
| } | ||
| } | ||
| super.terminateWrites(); | ||
| } |
There was a problem hiding this comment.
terminateWrites() breaks out of the write loop when super.write(buf) returns <= 0, then immediately calls super.terminateWrites(). If the underlying conduit performs partial/non-blocking writes and returns 0, this can terminate the exchange with unwritten bytes (truncated responses). This needs a non-truncating strategy: either ensure all buffered bytes are flushed (blocking/awaitWritable where appropriate) or keep remaining bytes and complete the write via subsequent write/flush/resumeWrites callbacks before terminating.
| // Register ReprDigest conduit inside appHandler (outermost/app-side) | ||
| // so it observes pre-compression bytes. | ||
| // When compression is disabled, also compute Content-Digest here. | ||
| if (digestAlgorithm != null) { | ||
| String wantRepr = exchange.getRequestHeaders().getFirst("Want-Repr-Digest"); | ||
| String reprAlgo = DigestFieldsUtils.negotiateAlgorithm(wantRepr, digestAlgorithm); | ||
| if (reprAlgo != null) { | ||
| exchange.addResponseWrapper((factory, ex) -> | ||
| new DigestConduit(factory.create(), ex, reprAlgo, REPR_DIGEST)); | ||
| } | ||
| if (!compress) { | ||
| String wantContent = exchange.getRequestHeaders().getFirst("Want-Content-Digest"); | ||
| String contentAlgo = DigestFieldsUtils.negotiateAlgorithm(wantContent, digestAlgorithm); | ||
| if (contentAlgo != null) { | ||
| exchange.addResponseWrapper((factory, ex) -> |
There was a problem hiding this comment.
When compress is false, both Repr-Digest and Content-Digest are implemented by stacking two DigestConduits. That means the entire response body is buffered twice and hashed twice, increasing memory and CPU for every response with digests enabled. Consider using a single buffering step that can set both headers (potentially with different negotiated algorithms) from the same captured byte array.
| // Register ReprDigest conduit inside appHandler (outermost/app-side) | |
| // so it observes pre-compression bytes. | |
| // When compression is disabled, also compute Content-Digest here. | |
| if (digestAlgorithm != null) { | |
| String wantRepr = exchange.getRequestHeaders().getFirst("Want-Repr-Digest"); | |
| String reprAlgo = DigestFieldsUtils.negotiateAlgorithm(wantRepr, digestAlgorithm); | |
| if (reprAlgo != null) { | |
| exchange.addResponseWrapper((factory, ex) -> | |
| new DigestConduit(factory.create(), ex, reprAlgo, REPR_DIGEST)); | |
| } | |
| if (!compress) { | |
| String wantContent = exchange.getRequestHeaders().getFirst("Want-Content-Digest"); | |
| String contentAlgo = DigestFieldsUtils.negotiateAlgorithm(wantContent, digestAlgorithm); | |
| if (contentAlgo != null) { | |
| exchange.addResponseWrapper((factory, ex) -> | |
| class CombinedDigestConduit extends org.xnio.conduits.AbstractStreamSinkConduit<org.xnio.conduits.StreamSinkConduit> { | |
| private final HttpServerExchange exchange; | |
| private final String reprAlgorithm; | |
| private final String contentAlgorithm; | |
| private final java.io.ByteArrayOutputStream buffer = new java.io.ByteArrayOutputStream(); | |
| private boolean writesTerminated; | |
| private boolean responseWritten; | |
| CombinedDigestConduit(org.xnio.conduits.StreamSinkConduit next, | |
| HttpServerExchange exchange, | |
| String reprAlgorithm, | |
| String contentAlgorithm) { | |
| super(next); | |
| this.exchange = exchange; | |
| this.reprAlgorithm = reprAlgorithm; | |
| this.contentAlgorithm = contentAlgorithm; | |
| } | |
| private String toMessageDigestName(String algorithm) { | |
| return algorithm == null ? null : algorithm.replace("-", "").toUpperCase(java.util.Locale.ROOT); | |
| } | |
| private String formatDigestHeader(String algorithm, byte[] body) throws IOException { | |
| try { | |
| MessageDigest messageDigest = MessageDigest.getInstance(toMessageDigestName(algorithm)); | |
| byte[] digest = messageDigest.digest(body); | |
| return algorithm + "=:" + java.util.Base64.getEncoder().encodeToString(digest) + ":"; | |
| } catch (NoSuchAlgorithmException e) { | |
| throw new IOException("Unsupported digest algorithm: " + algorithm, e); | |
| } | |
| } | |
| private void append(ByteBuffer src, int length) { | |
| if (length <= 0) { | |
| return; | |
| } | |
| ByteBuffer duplicate = src.duplicate(); | |
| int originalLimit = duplicate.limit(); | |
| duplicate.limit(duplicate.position() + length); | |
| byte[] bytes = new byte[length]; | |
| duplicate.get(bytes); | |
| duplicate.limit(originalLimit); | |
| buffer.write(bytes, 0, bytes.length); | |
| } | |
| private void writeDigestsAndBodyIfReady() throws IOException { | |
| if (!writesTerminated || responseWritten) { | |
| return; | |
| } | |
| byte[] body = buffer.toByteArray(); | |
| HeaderMap headers = exchange.getResponseHeaders(); | |
| if (reprAlgorithm != null) { | |
| headers.put(REPR_DIGEST, formatDigestHeader(reprAlgorithm, body)); | |
| } | |
| if (contentAlgorithm != null) { | |
| headers.put(CONTENT_DIGEST, formatDigestHeader(contentAlgorithm, body)); | |
| } | |
| ByteBuffer output = ByteBuffer.wrap(body); | |
| while (output.hasRemaining()) { | |
| next.write(output); | |
| } | |
| next.terminateWrites(); | |
| responseWritten = true; | |
| } | |
| @Override | |
| public int write(ByteBuffer src) throws IOException { | |
| int remaining = src.remaining(); | |
| append(src, remaining); | |
| src.position(src.position() + remaining); | |
| return remaining; | |
| } | |
| @Override | |
| public long write(ByteBuffer[] srcs, int offs, int len) throws IOException { | |
| long total = 0; | |
| for (int i = offs; i < offs + len; i++) { | |
| int remaining = srcs[i].remaining(); | |
| append(srcs[i], remaining); | |
| srcs[i].position(srcs[i].position() + remaining); | |
| total += remaining; | |
| } | |
| return total; | |
| } | 8000|
| @Override | |
| public int writeFinal(ByteBuffer src) throws IOException { | |
| int written = write(src); | |
| writesTerminated = true; | |
| return written; | |
| } | |
| @Override | |
| public long writeFinal(ByteBuffer[] srcs, int offs, int len) throws IOException { | |
| long written = write(srcs, offs, len); | |
| writesTerminated = true; | |
| return written; | |
| } | |
| @Override | |
| public long transferFrom(FileChannel src, long position, long count) throws IOException { | |
| long total = 0; | |
| long currentPosition = position; | |
| ByteBuffer chunk = ByteBuffer.allocate((int) Math.min(8192L, count)); | |
| while (total < count) { | |
| chunk.clear(); | |
| chunk.limit((int) Math.min(chunk.capacity(), count - total)); | |
| int read = src.read(chunk, currentPosition); | |
| if (read <= 0) { | |
| break; | |
| } | |
| chunk.flip(); | |
| append(chunk, read); | |
| total += read; | |
| currentPosition += read; | |
| } | |
| return total; | |
| } | |
| @Override | |
| public long transferFrom(org.xnio.channels.StreamSourceChannel source, long count, ByteBuffer throughBuffer) throws IOException { | |
| long total = 0; | |
| while (total < count) { | |
| throughBuffer.clear(); | |
| throughBuffer.limit((int) Math.min(throughBuffer.capacity(), count - total)); | |
| int read = source.read(throughBuffer); | |
| if (read <= 0) { | |
| break; | |
| } | |
| throughBuffer.flip(); | |
| append(throughBuffer, read); | |
| total += read; | |
| } | |
| return total; | |
| } | |
| @Override | |
| public void terminateWrites() throws IOException { | |
| writesTerminated = true; | |
| } | |
| @Override | |
| public void truncateWrites() throws IOException { | |
| buffer.reset(); | |
| responseWritten = true; | |
| next.truncateWrites(); | |
| } | |
| @Override | |
| public boolean flush() throws IOException { | |
| writeDigestsAndBodyIfReady(); | |
| return responseWritten ? next.flush() : true; | |
| } | |
| } | |
| // Register ReprDigest conduit inside appHandler (outermost/app-side) | |
| // so it observes pre-compression bytes. | |
| // When compression is disabled, compute both digests from the same buffered body. | |
| if (digestAlgorithm != null) { | |
| String wantRepr = exchange.getRequestHeaders().getFirst("Want-Repr-Digest"); | |
| String reprAlgo = DigestFieldsUtils.negotiateAlgorithm(wantRepr, digestAlgorithm); | |
| String contentAlgo = null; | |
| if (!compress) { | |
| String wantContent = exchange.getRequestHeaders().getFirst("Want-Content-Digest"); | |
| contentAlgo = DigestFieldsUtils.negotiateAlgorithm(wantContent, digestAlgorithm); | |
| } | |
| if (!compress && reprAlgo != null && contentAlgo != null) { | |
| String finalReprAlgo = reprAlgo; | |
| String finalContentAlgo = contentAlgo; | |
| exchange.addResponseWrapper((factory, ex) -> | |
| new CombinedDigestConduit(factory.create(), ex, finalReprAlgo, finalContentAlgo)); | |
| } else { | |
| if (reprAlgo != null) { | |
| exchange.addResponseWrapper((factory, ex) -> | |
| new DigestConduit(factory.create(), ex, reprAlgo, REPR_DIGEST)); | |
| } | |
| if (!compress && contentAlgo != null) { | |
| exchange.addResponseWrapper((factory, ex) -> |
| URI uri = URI.create("http://127.0.0.1:" + port + path); | ||
| HttpURLConnection conn = (HttpURLConnection) uri.toURL().openConnection(); | ||
| conn.setConnectTimeout(3000); | ||
| conn.setReadTimeout(3000); | ||
| conn.setRequestProperty("Accept-Encoding", "identity"); |
There was a problem hiding this comment.
The connect(...) helper unconditionally sets Accept-Encoding: identity, which prevents gzip from being selected. That means the "with compression" tests won’t actually exercise the post-compression Content-Digest path. To validate the two-layer design, let the compression tests request Accept-Encoding: gzip and then compute Content-Digest over the raw response bytes that are read (no decompression needed).
| URI uri = URI.create("http://127.0.0.1:" + port + path); | |
| HttpURLConnection conn = (HttpURLConnection) uri.toURL().openConnection(); | |
| conn.setConnectTimeout(3000); | |
| conn.setReadTimeout(3000); | |
| conn.setRequestProperty("Accept-Encoding", "identity"); | |
| return connect(port, path, "identity"); | |
| } | |
| private HttpURLConnection connect(int port, String path, String acceptEncoding) throws IOException { | |
| URI uri = URI.create("http://127.0.0.1:" + port + path); | |
| HttpURLConnection conn = (HttpURLConnection) uri.toURL().openConnection(); | |
| conn.setConnectTimeout(3000); | |
| conn.setReadTimeout(3000); | |
| conn.setRequestProperty("Accept-Encoding", acceptEncoding); |
| URI uri = URI.create("http://127.0.0.1:" + port + path); | ||
| HttpURLConnection conn = (HttpURLConnection) uri.toURL().openConnection(); | ||
| conn.setConnectTimeout(3000); | ||
| conn.setReadTimeout(3000); | ||
| // Disable accept-encoding so the response is not compressed (we don't have gzip decoder in tests) | ||
| conn.setRequestProperty("Accept-Encoding", "identity"); |
There was a problem hiding this comment.
The connect(...) helper unconditionally sets Accept-Encoding: identity, so even when the server is started with compress?=true, the response is typically not compressed. This makes the compression-path digest logic hard to validate in integration tests. Consider allowing the compression tests to send Accept-Encoding: gzip and assert Content-Digest over the raw on-the-wire bytes (no gzip decoding required).
| URI uri = URI.create("http://127.0.0.1:" + port + path); | |
| HttpURLConnection conn = (HttpURLConnection) uri.toURL().openConnection(); | |
| conn.setConnectTimeout(3000); | |
| conn.setReadTimeout(3000); | |
| // Disable accept-encoding so the response is not compressed (we don't have gzip decoder in tests) | |
| conn.setRequestProperty("Accept-Encoding", "identity"); | |
| return connect(port, path, "identity"); | |
| } | |
| private HttpURLConnection connect(int port, String path, String acceptEncoding) throws IOException { | |
| URI uri = URI.create("http://127.0.0.1:" + port + path); | |
| HttpURLConnection conn = (HttpURLConnection) uri.toURL().openConnection(); | |
| conn.setConnectTimeout(3000); | |
| conn.setReadTimeout(3000); | |
| if (acceptEncoding != null) { | |
| conn.setRequestProperty("Accept-Encoding", acceptEncoding); | |
| } |
| * <p>Algorithm negotiation via {@code Want-Repr-Digest} is supported. | ||
| * Streaming responses ({@code Transfer-Encoding: chunked} that bypass buffering) are | ||
| * not supported and will have no digest header set. | ||
| * | ||
| * @author kawasima | ||
| */ | ||
| public class DigestFilter implements Filter { | ||
|
|
||
| /** Init-param name for the default digest algorithm. */ | ||
| public static final String PARAM_ALGORITHM = "digestAlgorithm"; | ||
| /** | ||
| * Init-param name indicating whether a {@code CompressionHandler} is active in the chain. | ||
| * | ||
| * <p>When {@code true}, this filter only computes {@code Repr-Digest} (pre-compression bytes). | ||
| * {@code Content-Digest} (post-compression bytes) is handled by {@code ContentDigestHandler} | ||
| * which is placed outside the {@code CompressionHandler}. The value must be kept in sync | ||
| * with the {@code compress?} option passed to {@code JettyAdapter}. |
There was a problem hiding this comment.
The Javadoc says streaming/chunked responses that bypass buffering are not supported and will have no digest header set, but the implementation always buffers all writes and suppresses flushBuffer() (and throws on setWriteListener). Either update the documentation to reflect the actual behavior (response is fully buffered; flush/non-blocking unsupported), or add a detection path that skips digest computation and delegates to the underlying response for streaming/non-blocking cases.
| public JettyComponent enableDigestFields(String algorithm) { | ||
| if (!DigestFieldsUtils.SUPPORTED_ALGORITHMS.contains(algorithm)) { | ||
| throw new MisconfigurationException("web.DIGEST_ALGORITHM_UNSUPPORTED", algorithm); | ||
| } |
There was a problem hiding this comment.
The Javadoc says this must be called before the component is started, but unlike addWebSocket(...) there’s no guard against calling it after server is non-null. As-is, calling enableDigestFields(...) post-start will silently do nothing for the running server (options are only applied in start). Consider adding a server != null check and throwing a MisconfigurationException to match the documented contract.
| } | |
| } | |
| if (server != null) { | |
| throw new MisconfigurationException("web.DIGEST_FIELDS_MUST_ENABLE_BEFORE_START", algorithm); | |
| } |
| public UndertowComponent enableDigestFields(String algorithm) { | ||
| if (!DigestFieldsUtils.SUPPORTED_ALGORITHMS.contains(algorithm)) { | ||
| throw new MisconfigurationException("web.DIGEST_ALGORITHM_UNSUPPORTED", algorithm); | ||
| } | ||
| this.digestAlgorithm = algorithm; | ||
| return this; | ||
| } |
There was a problem hiding this comment.
The Javadoc says this must be called before the component is started, but there’s no guard against calling it after startup. Since digestAlgorithm is only copied into the OptionMap during start(), enabling digest fields after server is created will not affect the running instance. Consider rejecting post-start calls (e.g., if (server != null) throw ...) so callers get a clear failure instead of a silent no-op.
| @Override | ||
| public void flushBuffer() { | ||
| // Suppress commit: the Servlet spec commits the response (making headers | ||
| // immutable) on flushBuffer(). We must suppress this so that Repr-Digest / | ||
| // Content-Digest headers can still be set after chain.doFilter() returns. | ||
| } |
There was a problem hiding this comment.
flushBuffer() is overridden as a no-op to keep headers mutable, but this changes semantics for responses that intentionally flush/stream. In particular, ServletUtils.setBody calls servletResponse.flushBuffer() for StreamingBody before writing (enkan-servlet/src/main/java/enkan/servlet/util/ServletUtils.java:104-106). With this wrapper, streaming bodies will no longer flush/commit early and will instead be fully buffered, which can break streaming endpoints and increase memory usage. Consider detecting streaming/flush usage and bypassing digest buffering, or explicitly rejecting/disable digest fields for StreamingBody responses.
- DigestFieldsUtils: catch SfParseException in negotiateAlgorithm and fall back to defaultAlgorithm so a malformed Want-* header does not cause a 500 - DigestConduit: use awaitWritable() instead of breaking on n==0 to prevent partial-write truncation in terminateWrites() - UndertowAdapter/CombinedDigestConduit: replace double-stacked DigestConduit (non-compression path) with a single CombinedDigestConduit that buffers once and sets both Repr-Digest and Content-Digest - DigestFilter: update Javadoc and flushBuffer() comment to accurately reflect full-buffering behaviour and StreamingBody trade-off - JettyComponent/UndertowComponent: throw MisconfigurationException when enableDigestFields() is called after the server has started; add corresponding message keys to misconfiguration*.properties - Tests: add overloaded connect(port, path, acceptEncoding) helper and update compression tests to send Accept-Encoding: gzip so the compression code path is actually exercised; assert Content-Digest against the raw on-wire bytes Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…backed ByteBuffer; add malformed Want-* header tests - DigestConduit, CombinedDigestConduit: use hasArray() fast path to write directly from the backing array instead of allocating a new byte[] per chunk on every write() call - ContentDigestHandler: same fix for the Jetty buffering response wrapper - DigestFieldsUtilsTest: add two tests covering the SfParseException fallback path in negotiateAlgorithm() (malformed Want-* header should fall back to defaultAlgorithm, not throw) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 21 out of 21 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| public long transferFrom(FileChannel src, long position, long count) throws IOException { | ||
| // For file transfers, read into a buffer first | ||
| ByteBuffer buf = ByteBuffer.allocate((int) Math.min(count, 65536)); | ||
| long transferred = 0; | ||
| while (transferred < count) { | ||
| buf.clear(); | ||
| int read = src.read(buf, position + transferred); | ||
| if (read <= 0) break; | ||
| buf.flip(); | ||
| transferred += write(buf); | ||
| } | ||
| return transferred; |
There was a problem hiding this comment.
transferFrom(FileChannel, position, count) can buffer/forward more than count bytes because each loop iteration reads up to the full buffer capacity without limiting to the remaining count - transferred. This violates the StreamSinkConduit contract and can result in sending extra bytes / incorrect digests for file responses. Limit each FileChannel.read to at most the remaining requested bytes (adjust the ByteBuffer limit based on count - transferred).
| public long transferFrom(FileChannel src, long position, long count) throws IOException { | ||
| ByteBuffer buf = ByteBuffer.allocate((int) Math.min(count, 65536)); | ||
| long transferred = 0; | ||
| while (transferred < count) { | ||
| buf.clear(); | ||
| int read = src.read(buf, position + transferred); | ||
| if (read <= 0) break; | ||
| buf.flip(); | ||
| transferred += write(buf); | ||
| } | ||
| return transferred; |
There was a problem hiding this comment.
transferFrom(FileChannel, position, count) may transfer more than count bytes on the last iteration when count is not a multiple of the internal buffer size (e.g., remaining < 65536). The loop reads up to the buffer capacity without capping to count - transferred, which can send extra bytes and break HTTP responses/digests. Cap each read to the remaining requested byte count.
| if (contentDigestHeader != null && !verifyDigest(body, contentDigestHeader)) { | ||
| return builder(HttpResponse.of("Content-Digest mismatch")) | ||
| .set(HttpResponse::setStatus, 400) | ||
| .build(); | ||
| } | ||
| if (reprDigestHeader != null && !verifyDigest(body, reprDigestHeader)) { | ||
| return builder(HttpResponse.of("Repr-Digest mismatch")) | ||
| .set(HttpResponse::setStatus, 400) | ||
| .build(); |
There was a problem hiding this comment.
When the digest header value is malformed (parse failure) verifyDigest(...) returns false and the middleware responds with "Content-Digest mismatch" / "Repr-Digest mismatch". That message is inaccurate for syntax errors and makes debugging harder. Consider distinguishing malformed header vs actual digest mismatch (e.g., return a 400 with a message indicating an invalid/malformed digest header).
| web.WEBSOCKET_MUST_REGISTER_BEFORE_START.problem=WebSocket handler for path {0} was registered after the server has already started. | ||
| web.WEBSOCKET_MUST_REGISTER_BEFORE_START.solution=Call addWebSocket() before starting the JettyComponent. | ||
| web.DIGEST_FIELDS_MUST_ENABLE_BEFORE_START.problem=enableDigestFields("{0}") was called after the server has already started. | ||
| web.DIGEST_FIELDS_MUST_ENABLE_BEFORE_START.solution=Call enableDigestFields() before starting the JettyComponent. |
There was a problem hiding this comment.
This misconfiguration key is defined in both the Jetty and Undertow modules but with different .solution text (JettyComponent vs UndertowComponent). MisconfigurationException merges all META-INF/misconfiguration*.properties resources and later entries win, so the final message will be classpath-order dependent and may mention the wrong component. Use distinct error codes per adapter (e.g., jetty.* / undertow.*), or make the message identical across modules and move it to a shared resource.
| web.DIGEST_FIELDS_MUST_ENABLE_BEFORE_START.solution=Call enableDigestFields() before starting the JettyComponent. | |
| web.DIGEST_FIELDS_MUST_ENABLE_BEFORE_START.solution=Call enableDigestFields() before starting the server. |
| @@ -0,0 +1,2 @@ | |||
| web.DIGEST_FIELDS_MUST_ENABLE_BEFORE_START.problem=enableDigestFields("{0}") was called after the server has already started. | |||
| web.DIGEST_FIELDS_MUST_ENABLE_BEFORE_START.solution=Call enableDigestFields() before starting the UndertowComponent. | |||
There was a problem hiding this comment.
This module defines web.DIGEST_FIELDS_MUST_ENABLE_BEFORE_START.*, but the same key is also defined in the Jetty module with different .solution text. Because misconfiguration messages are merged from all JARs, the resulting message becomes non-deterministic and can refer to the wrong component depending on classpath order. Prefer adapter-specific codes (e.g., undertow.*) or a shared identical message.
| web.DIGEST_FIELDS_MUST_ENABLE_BEFORE_START.solution=Call enableDigestFields() before starting the UndertowComponent. | |
| web.DIGEST_FIELDS_MUST_ENABLE_BEFORE_START.solution=Call enableDigestFields() before starting the web component. |
| throw new MisconfigurationException("web.DIGEST_ALGORITHM_UNSUPPORTED", algorithm); | ||
| } | ||
| if (server != null) { | ||
| throw new MisconfigurationException("web.DIGEST_FIELDS_MUST_ENABLE_BEFORE_START", algorithm); |
There was a problem hiding this comment.
enableDigestFields throws web.DIGEST_FIELDS_MUST_ENABLE_BEFORE_START, but that code currently collides between Jetty and Undertow modules (different .solution strings). Switching to an adapter-specific code here (and in the corresponding properties file) avoids classpath-order-dependent error messages.
| throw new MisconfigurationException("web.DIGEST_FIELDS_MUST_ENABLE_BEFORE_START", algorithm); | |
| throw new MisconfigurationException("web.JETTY_DIGEST_FIELDS_MUST_ENABLE_BEFORE_START", algorithm); |
- Use adapter-specific misconfiguration keys (jetty.*/undertow.*) for DIGEST_FIELDS_MUST_ENABLE_BEFORE_START to avoid classpath-order-dependent error messages when both JARs are present - Cap transferFrom() reads to remaining requested bytes in DigestConduit and CombinedDigestConduit to prevent buffering more than count bytes - Distinguish malformed header vs digest mismatch in DigestValidationMiddleware: introduce VerifyResult enum so the 400 response says "Invalid Content-Digest header" for parse failures and "Content-Digest mismatch" for hash mismatches - Add tests for both error paths in DigestValidationMiddlewareTest Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary
Repr-DigestandContent-Digestresponse header generation for both Jetty and Undertow, including full compression support via two-layer interceptionDigestValidationMiddlewareinenkan-webfor request-sideContent-Digest/Repr-Digestvalidation (returns 400 on mismatch)Want-Content-Digest/Want-Repr-Digestnegotiation — parses SF Dictionary, selects highest-priority supported algorithmsha-256andsha-512Architecture
Response digest (server layer)
Digests must be computed at the server layer because
Content-Digestrequires post-compression bytes that are unavailable inside the middleware chain.Jetty two-layer design:
DigestFilter(Servlet Filter insideServletContextHandler) — buffers pre-compression bytes → setsRepr-DigestContentDigestHandler(Jetty 12 coreHandler.WrapperoutsideCompressionHandler) — buffers post-compression bytes → setsContent-DigestUndertow conduit stacking:
DigestConduitregistered inappHandler(outermost/app-side of gzip) →Repr-DigestDigestOuterHandlerwrappingEncodingHandlerregistersDigestConduitbefore gzip runs (innermost/network-side) →Content-DigestData flow:
app → ReprDigestConduit → GzipConduit → ContentDigestConduit → socketRequest validation (middleware layer)
Request bodies are not compressed, so
DigestValidationMiddlewareinenkan-webhandles both servers without duplication.Configuration
Test plan
DigestFieldsUtilsTest— unit tests for compute/negotiate/toJca (17 tests)DigestValidationMiddlewareTest— unit tests for request validation including body restoration, null body, unsupported algorithm pass-through, malformed header 400 (12 tests)JettyAdapterDigestTest— integration tests: Repr-Digest + Content-Digest without compression, equality check, Want-* negotiation, Repr-Digest with compression (6 tests)UndertowAdapterDigestTest— integration tests: same coverage as Jetty plus Content-Digest with compression (8 tests)Closes #132
🤖 Generated with Claude Code