Skip to content

Commit 8583feb

Browse files
snalliclaude
andauthored
Fix wrong-variable logging in signed-URL blob ID encoding catch (#3252)
* Fix wrong-variable logging in signed-URL blob ID encoding catch The catch in AmbrySecurityService.preProcessRequest's URLEncoder.encode block declares its parameter as `encodingException` but the log statement passes `e` — the enclosing lambda's `(r, e) ->` parameter, which is the result of urlSigningService.verifySignedRequest's callback. The two are unrelated exceptions: `encodingException` is the actual URL-encoding failure we just caught; `e` is the signed-URL verification result. The bug compiles cleanly because both variables are valid in scope, but when this fires in production the log message says "Failed to encode blob id signed url" while the attached stack trace points at the verify operation — silently misdirecting any operator investigating signed-URL blob ID encoding failures. One-character fix: log encodingException instead of e so the message and the stack trace agree. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Also log outer verifySignedRequest exception at the encoding catch Per Satish's review on #3252: surface the outer verifySignedRequest callback exception `e` at the encoding-failure log site too. Plain separate LOGGER.error line, null-guarded since `e` is null on the verification-succeeded path. The primary log (`encodingException`) keeps its accurate "Failed to encode blob id signed url" message; the secondary log surfaces `e` with its own message so both stacks are visible alongside each other in production logs. No behavior change beyond logging — `callback.onCompletion(r, e)` on the line below still forwards `e` to the chained failure callback as before. --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 4700f1a commit 8583feb

1 file changed

Lines changed: 8 additions & 1 deletion

File tree

ambry-frontend/src/main/java/com/github/ambry/frontend/AmbrySecurityService.java

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,14 @@ public void preProcessRequest(RestRequest restRequest, Callback<Void> callback)
123123
URLEncoder.encode((String) restRequest.getArgs().get(Headers.BLOB_ID), StandardCharsets.UTF_8.name());
124124
restRequest.setArg(Headers.BLOB_ID, encodedBlobId);
125125
} catch (Exception encodingException) {
126-
LOGGER.error("Failed to encode blob id signed url: {}", restRequest.getArgs().get(Headers.BLOB_ID), e);
126+
LOGGER.error("Failed to encode blob id signed url: {}", restRequest.getArgs().get(Headers.BLOB_ID),
127+
encodingException);
128+
// Also log the outer verifySignedRequest exception if there was one, so both
129+
// are visible at this site. Null-guarded — `e` is null on the verification-
130+
// succeeded path (encoding can still fail there for malformed input).
131+
if (e != null) {
132+
LOGGER.error("verifySignedRequest also reported an exception", e);
133+
}
127134
}
128135
}
129136
callback.onCompletion(r, e);

0 commit comments

Comments
 (0)