Andrea Cosentino created CAMEL-25660:
----------------------------------------
Summary: camel-azure-servicebus - dead-lettering builds unbounded
reason and description from the exception, and a failure to dead-letter is not
caught
Key: CAMEL-25660
URL: https://issues.apache.org/jira/browse/CAMEL-25660
Project: Camel
Issue Type: Bug
Components: camel-azure-servicebus
Reporter: Andrea Cosentino
h3. Summary
With {{enableDeadLettering=true}} in PEEK_LOCK mode,
{{ServiceBusConsumer.onFailure}} fills both dead-letter fields straight from
the route's exception, with no length limit:
{code:java}
deadLetterOptions.setDeadLetterReason(
String.format("%s: %s", cause.getClass().getName(),
cause.getMessage()));
deadLetterOptions.setDeadLetterErrorDescription(Arrays.stream(cause.getStackTrace())
.map(StackTraceElement::toString)
.collect(Collectors.joining("\n")));
messageContext.deadLetter(deadLetterOptions);
{code}
A full Camel stack trace, with the error-handler and processor frames, is
routinely several thousand characters. Microsoft's reference for
{{ServiceBusReceiver.DeadLetterMessageAsync}} lists 4096 as the ceiling for the
dead-letter reason or the error description, and exceeding it is one of the
documented error conditions for the call. So for an ordinary route failure the
description can be over the limit.
h3. Why it matters
The call is in a {{try}} whose only companion is a {{finally}} that decrements
{{pendingExchanges}} - there is no {{catch}}:
{code:java}
try {
...
messageContext.deadLetter(deadLetterOptions);
...
} finally {
pendingExchanges.decrementAndGet();
}
{code}
So if the broker rejects the dead-letter request, the exception leaves
{{onFailure}} and the message is *not* dead-lettered. Its lock then expires and
Service Bus redelivers it, the route fails the same way, and the same oversized
description is sent again - until {{maxDeliveryCount}} is reached and the
broker dead-letters it on its own terms. The net effect is that
{{enableDeadLettering}} does not deliver the diagnostics it is there for, and
the exchange is processed several more times than intended.
The reason field has the same shape: {{cause.getMessage()}} is unbounded, and
some messages are long (a database error quoting the statement, a validation
report).
h3. Suggested fix
Truncate both fields to what Service Bus accepts before calling {{deadLetter}},
and wrap the call so that a broker rejection is reported through the exception
handler instead of escaping {{onFailure}}. Keeping the head of the stack trace
is the useful part; the tail is error-handler frames.
h3. Evidence and what is NOT verified
* Verified by reading the component on main: both fields are unbounded, and
there is no {{catch}} around {{deadLetter}}.
* Verified that {{com.azure.messaging.servicebus.models.DeadLetterOptions}}
(azure-messaging-servicebus 7.17.16) does no client-side length validation, so
the broker is what rejects it.
* *Not* verified against a live namespace - this has not been reproduced, and
the 4096 figure comes from the SDK reference rather than the Service Bus quotas
page, which also leaves it unclear whether the limit counts characters or
bytes. Someone with a Service Bus namespace should confirm the threshold and
the resulting behaviour before the fix is sized.
Filed unassigned.
Found while sweeping for consumers that echo exception detail. Unlike that
class, this is not an information-disclosure concern: the text goes to the
operator's own dead-letter queue, not to an untrusted sender.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
