[FIXED] Gateway message tracing hop header duplication

[johnweldon]

Summary

Fixes a critical bug in gateway message routing where the hop header was added multiple times (once per gateway) instead of once per message, causing header corruption and message parsing failures.

Problem

In production environments (NATS Server 2.11.8) with message tracing enabled and multi-gateway deployments, intermittent malformed headers appeared:

type: ResourceStatus\r\n1\r\ntype: ResourceStatus\r\nNats-Trace-Hop: 2.1.1.1

This caused clients to fail parsing response bodies, breaking message delivery.

Root Cause

The setHopHeader() call was inside the gateway routing loop in sendMsgToGateways(), executing once per outbound gateway instead of once per message. Multiple calls on the same message buffer caused:

1. First call: Added "Nats-Trace-Hop: 1" but corrupted the previous header's trailing \r\n
2. Second call: Failed to remove the old hop header (header removal requires \n before the header name, but it was missing due to corruption)
3. Result: Duplicate hop headers, missing newlines, and parser confusion where the "1" from the hop count bled into the wrong position

Solution

Moved setHopHeader() before the gateway loop (line 2568) so it's called exactly once per message. All gateways now receive identical message content with the same hop count. The existing defer statement properly restores c.pa state.

This matches the pattern used in route handling and aligns with the comment at client.go:4441:

"If others will use this later we need to save and restore original."

Testing

Added two comprehensive tests:

• TestGatewayMsgTraceDuplicateHopHeader - Validates hop count consistency at the client API level
• TestGatewayMsgTraceDuplicateHopHeaderRawProtocol - Validates hop count consistency at the raw HMSG protocol level

Both tests fail without the fix (showing different hop counts per gateway) and pass with it.

Impact

• Severity: High - causes message parsing failures in production
• Scope: All multi-gateway deployments with message tracing enabled
• Fix: Single line moved, minimal change with maximum safety

Signed-off-by: John Weldon jweldon@nats.io

[johnweldon]

@kozlovic @wallyqs

[MauriceVanVeen]

I'm thinking the reason for calling setHopHeader per gateway is such that each will count as separate hops, not just as one? In that case probably c.setHeader would need to be adjusted instead of moving the setHopHeader call.

[kozlovic]

I need to think if this needs to be inside the loop or not. If you look at the route/LN case, it is per route/LN and I do restore the pa at each iteration. It may be that this is what was missing in the gateway case.

[kozlovic]

I do believe that the hop must be set per gateway. This is the only way for a tool to reconstruct the proper routing. I have submitted a PR with the fix (similar to what we do for routes/leaf) and extended the existing trace with gateways to have at least 2 gateways (instead of a single one) that demonstrated the issue and now passes. The test does verify that the hop is different for each gateway.

[johnweldon]

Thank you all