Remove redundant code comments for improved readability and maintainability
CI - Test, Publish and Release / run-tests (push) Successful in 18s
CI - Test, Publish and Release / create-release (push) Successful in 18s
CI - Test, Publish and Release / check-and-publish (push) Successful in 13s

This commit is contained in:
2026-06-15 07:24:59 +02:00
parent a0790400e2
commit 6de7e26f33
9 changed files with 6 additions and 35 deletions
@@ -155,8 +155,6 @@ public final class HttpRequestHandler extends SimpleChannelInboundHandler<FullHt
if (handleWebSocketUpgrade(ctx, req)) return; if (handleWebSocketUpgrade(ctx, req)) return;
} }
// Backpressure: stop pulling further requests off this connection until we have answered
// the current one, bounding buffered request memory to one aggregated body per connection.
ctx.channel().config().setAutoRead(false); ctx.channel().config().setAutoRead(false);
req.retain(); req.retain();
@@ -214,8 +212,7 @@ public final class HttpRequestHandler extends SimpleChannelInboundHandler<FullHt
return true; return true;
} }
// Authenticate the upgrade if an auth layer is configured. WebSocket authenticators should
// be cheap (cookie/token validation) since the handshake runs on the event loop.
Principal principal = null; Principal principal = null;
if (authGate != null) { if (authGate != null) {
Request upgradeReq = new Request(req, resolution.pathParams()); Request upgradeReq = new Request(req, resolution.pathParams());
@@ -239,8 +236,6 @@ public final class HttpRequestHandler extends SimpleChannelInboundHandler<FullHt
ChannelPipeline pipeline = ctx.pipeline(); ChannelPipeline pipeline = ctx.pipeline();
String myName = ctx.name(); String myName = ctx.name();
// The plain-HTTP read timeout would close idle WebSocket connections (which legitimately
// stay quiet between frames), so drop it; the WebSocket idle handler governs liveness.
if (pipeline.get("read-timeout") != null) { if (pipeline.get("read-timeout") != null) {
pipeline.remove("read-timeout"); pipeline.remove("read-timeout");
} }
@@ -256,8 +251,7 @@ public final class HttpRequestHandler extends SimpleChannelInboundHandler<FullHt
} }
pipeline.addBefore(myName, "ws-proto", pipeline.addBefore(myName, "ws-proto",
new WebSocketServerProtocolHandler(protoCfg)); new WebSocketServerProtocolHandler(protoCfg));
// Reassemble fragmented (continuation) frames into a single logical message, bounded by
// the configured aggregate size so a stream of fragments cannot amplify memory use.
pipeline.addBefore(myName, "ws-aggregator", pipeline.addBefore(myName, "ws-aggregator",
new WebSocketFrameAggregator(wsConfig.maxAggregatedMessageSize())); new WebSocketFrameAggregator(wsConfig.maxAggregatedMessageSize()));
pipeline.addBefore(myName, "ws-frames", pipeline.addBefore(myName, "ws-frames",
@@ -304,8 +298,6 @@ public final class HttpRequestHandler extends SimpleChannelInboundHandler<FullHt
Request request = new Request(raw, params); Request request = new Request(raw, params);
request.clientIp(clientIp); request.clientIp(clientIp);
// Rate limiting runs before authentication so an unauthenticated flood is shed before
// reaching the (potentially expensive) authenticator.
RateLimiter.Result rlResult = null; RateLimiter.Result rlResult = null;
if (rateLimit != null) { if (rateLimit != null) {
rlResult = rateLimit.check(request, path, clientIp); rlResult = rateLimit.check(request, path, clientIp);
@@ -318,8 +310,6 @@ public final class HttpRequestHandler extends SimpleChannelInboundHandler<FullHt
} }
} }
// Authentication layer: attaches the principal on success, or short-circuits with a
// rejection response (401/500) for protected paths.
if (authGate != null) { if (authGate != null) {
Response rejection = authGate.authenticate(request, path); Response rejection = authGate.authenticate(request, path);
if (rejection != null) { if (rejection != null) {
@@ -360,8 +350,6 @@ public final class HttpRequestHandler extends SimpleChannelInboundHandler<FullHt
if (cors != null) cors.applyHeaders(origin, res); if (cors != null) cors.applyHeaders(origin, res);
send(ctx, res, keepAlive); send(ctx, res, keepAlive);
} catch (Throwable t) { } catch (Throwable t) {
// Last-resort guard: anything escaping the stages above must still produce a response,
// otherwise the connection would hang with auto-read disabled. Close it to be safe.
LOG.log(Level.ERROR, "Unexpected failure while handling request", t); LOG.log(Level.ERROR, "Unexpected failure while handling request", t);
try { try {
send(ctx, new Response().status(500).json("{\"error\":\"Internal Server Error\"}"), false); send(ctx, new Response().status(500).json("{\"error\":\"Internal Server Error\"}"), false);
@@ -237,7 +237,6 @@ public final class HttpServer {
channelClass = NioServerSocketChannel.class; channelClass = NioServerSocketChannel.class;
} }
// Capture configuration into effectively-final locals for the channel initializer.
final TlsConfig tlsCfg = this.tls; final TlsConfig tlsCfg = this.tls;
final CorsHandler corsHandler = this.cors; final CorsHandler corsHandler = this.cors;
final RateLimitGate rateLimitGate = this.gate; final RateLimitGate rateLimitGate = this.gate;
@@ -248,8 +247,7 @@ public final class HttpServer {
final boolean tlsEnabled = tlsCfg != null; final boolean tlsEnabled = tlsCfg != null;
final TrustedProxies proxies = this.trustedProxies; final TrustedProxies proxies = this.trustedProxies;
final int maxContent = this.maxHttpContentLength; final int maxContent = this.maxHttpContentLength;
final long readTimeoutSeconds = (httpReadTimeout != null && !httpReadTimeout.isZero() final long readTimeoutSeconds = (httpReadTimeout != null && !httpReadTimeout.isZero() && !httpReadTimeout.isNegative()) ? Math.max(1, httpReadTimeout.toSeconds()) : 0;
&& !httpReadTimeout.isNegative()) ? Math.max(1, httpReadTimeout.toSeconds()) : 0;
try { try {
new ServerBootstrap() new ServerBootstrap()
@@ -31,8 +31,7 @@ public final class ClientIp {
* @return the resolved client IP * @return the resolved client IP
*/ */
public static String resolve(String socketIp, String forwardedForHeader, TrustedProxies trusted) { public static String resolve(String socketIp, String forwardedForHeader, TrustedProxies trusted) {
if (forwardedForHeader == null || forwardedForHeader.isBlank() if (forwardedForHeader == null || forwardedForHeader.isBlank() || !trusted.isTrusted(socketIp)) {
|| !trusted.isTrusted(socketIp)) {
return socketIp; return socketIp;
} }
@@ -45,8 +44,6 @@ public final class ClientIp {
} }
} }
// Every hop in the chain is a trusted proxy; fall back to the originating (left-most)
// entry, or the socket address if the header was effectively empty.
String first = hops[0].trim(); String first = hops[0].trim();
return first.isEmpty() ? socketIp : first; return first.isEmpty() ? socketIp : first;
} }
@@ -129,7 +129,7 @@ public final class TrustedProxies {
} }
boolean contains(byte[] addr) { boolean contains(byte[] addr) {
if (addr.length != base.length) return false; // different address family if (addr.length != base.length) return false;
int fullBytes = prefixBits / 8; int fullBytes = prefixBits / 8;
for (int i = 0; i < fullBytes; i++) { for (int i = 0; i < fullBytes; i++) {
if (addr[i] != base[i]) return false; if (addr[i] != base[i]) return false;
@@ -137,13 +137,11 @@ public final class TrustedProxies {
int remainingBits = prefixBits % 8; int remainingBits = prefixBits % 8;
if (remainingBits != 0) { if (remainingBits != 0) {
int mask = 0xFF << (8 - remainingBits) & 0xFF; int mask = 0xFF << (8 - remainingBits) & 0xFF;
if ((addr[fullBytes] & mask) != (base[fullBytes] & mask)) return false; return (addr[fullBytes] & mask) == (base[fullBytes] & mask);
} }
return true; return true;
} }
// Records with array components get identity-based equals/hashCode by default; provide
// value semantics so deduplication and tests behave intuitively.
@Override @Override
public boolean equals(Object o) { public boolean equals(Object o) {
return o instanceof Cidr c && prefixBits == c.prefixBits && Arrays.equals(base, c.base); return o instanceof Cidr c && prefixBits == c.prefixBits && Arrays.equals(base, c.base);
@@ -46,9 +46,6 @@ public final class RateLimitConfig {
.sorted((a, c) -> Integer.compare(c.prefix.length(), a.prefix.length())) .sorted((a, c) -> Integer.compare(c.prefix.length(), a.prefix.length()))
.toList(); .toList();
// Collect the distinct limiter instances once so the gate's periodic cleanup can iterate
// them. Identity-based de-duplication keeps a limiter shared across several rules from
// being cleaned multiple times per pass.
Set<RateLimiter> limiters = Collections.newSetFromMap(new IdentityHashMap<>()); Set<RateLimiter> limiters = Collections.newSetFromMap(new IdentityHashMap<>());
if (globalRule != null) limiters.add(globalRule.limiter()); if (globalRule != null) limiters.add(globalRule.limiter());
for (Rule r : exactPathRules.values()) limiters.add(r.limiter()); for (Rule r : exactPathRules.values()) limiters.add(r.limiter());
@@ -126,7 +126,6 @@ public final class RateLimitGate {
try { try {
limiter.cleanup(staleAfterNanos); limiter.cleanup(staleAfterNanos);
} catch (RuntimeException ignored) { } catch (RuntimeException ignored) {
// Best-effort eviction; never let one limiter break the cleanup cycle.
} }
} }
} }
@@ -159,8 +159,6 @@ public final class Router {
* @return the resolution outcome, never {@code null} * @return the resolution outcome, never {@code null}
*/ */
public Resolution resolve(HttpMethod method, String path) { public Resolution resolve(HttpMethod method, String path) {
// Most routes capture no path parameters; defer allocating the map until the first
// segment is actually captured so the common no-param case stays allocation-free.
Map<String, String> params = null; Map<String, String> params = null;
Node node = root; Node node = root;
for (String segment : split(path)) { for (String segment : split(path)) {
@@ -66,8 +66,6 @@ public final class TlsConfig {
try { try {
return new TlsConfig(SslContextBuilder.forServer(certificateChain, privateKey, keyPassword).build()); return new TlsConfig(SslContextBuilder.forServer(certificateChain, privateKey, keyPassword).build());
} catch (SSLException | RuntimeException e) { } catch (SSLException | RuntimeException e) {
// Netty surfaces missing/invalid PEM material as IllegalArgumentException; normalise
// every initialisation failure to a single, predictable exception type.
throw new IllegalStateException("Failed to initialise TLS from PEM files", e); throw new IllegalStateException("Failed to initialise TLS from PEM files", e);
} }
} }
@@ -103,8 +103,6 @@ public final class WebSocketGroup {
public WebSocketGroup broadcastJson(Object value) { public WebSocketGroup broadcastJson(Object value) {
try { try {
byte[] bytes = JsonMapper.MAPPER.writeValueAsBytes(value); byte[] bytes = JsonMapper.MAPPER.writeValueAsBytes(value);
// Build the text frame straight from the serialized UTF-8 bytes; the channel group
// duplicates the payload per recipient, so no String round-trip re-encode is needed.
channels.writeAndFlush(new TextWebSocketFrame(Unpooled.wrappedBuffer(bytes))); channels.writeAndFlush(new TextWebSocketFrame(Unpooled.wrappedBuffer(bytes)));
} catch (JacksonException e) { } catch (JacksonException e) {
throw new RuntimeException("JSON serialization failed", e); throw new RuntimeException("JSON serialization failed", e);