[Hardik K (hardikkaurani)]

Summary

In HeadersInspectionHandler, there were three issues in header inspection and telemetry:

1. Multi-valued Header Overwrites: HeadersInspectionHandler previously created a new HashSet<String>() per header and called put(header.getFirst(), value) on RequestHeaders and ResponseHeaders. When multiple entries existed for the same header name (e.g., Set-Cookie, Accept, Vary), subsequent values replaced the set instead of accumulating, leaving only the last value. By delegating to add(key, value), all values for the header are preserved.
2. Request Isolation: If the same HeadersInspectionOption instance was reused across requests, headers from previous requests were never cleared and leaked into subsequent requests.
3. Telemetry Attribute Accuracy: The OpenTelemetry span attribute com.microsoft.kiota.handler.headersInspection.enable was hardcoded to true even when inspection was disabled; it now reflects inspectionOption.getInspectRequestHeaders() || inspectionOption.getInspectResponseHeaders().

Changes

• Replaced .put(header.getFirst(), value) with .add(header.getFirst(), header.getSecond()) in HeadersInspectionHandler.java.
• Added .clear() calls to getRequestHeaders() and getResponseHeaders() prior to recording headers for each request/response.
• Updated telemetry attribute to check if request or response inspection is enabled.
• Added unit tests in HeadersInspectionHandlerTest.java verifying preservation of multi-valued headers and header isolation across requests.

[Hardik K (hardikkaurani)]

Hi Vincent Biret (@baywet)! This PR fixes multi-valued header handling in HeadersInspectionHandler by calling add() instead of put() (which was replacing sets and overwriting multi-valued headers such as Set-Cookie or Vary), adds .clear() for per-request header isolation when reusing inspection options, and aligns the telemetry enable attribute with active inspection settings. Unit tests covering both multi-valued headers and request isolation are included. Whenever you have a chance, please take a look. Thank you!

[Copilot]

🟡 Changes recommended

Shared-option capture updates can race, and failed requests retain stale response headers.

2 open issues

• Synchronize shared header capture to prevent concurrent-call races · New
• Clear response headers before proceeding to prevent stale headers · New

_{Review effort: Balanced}

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[Hardik K (hardikkaurani)]

Hi Vincent Biret (@baywet) and @andreastman,

I've addressed all Copilot review feedback and resolved the formatting and lint checks. Here's a summary of the changes:

1. Synchronize Shared Header Capture

• Synchronized request and response header capture on the shared inspectionOption instance.
• Prevented race conditions where concurrent requests without request-specific options could clear the internal header map between containsKey() and get() in Headers.addImpl(), avoiding potential NullPointerExceptions and cross-request header contamination.
• Added handlesConcurrentCallsWithoutRaces() to verify race-free execution across concurrent requests using multiple threads.

2. Clear Response Headers Before chain.proceed(request)

• Moved inspectionOption.getResponseHeaders().clear() before chain.proceed(request).
• Ensured that if a request fails (e.g., with an IOException), stale response headers from a previous request using the shared option are not retained.
• Added clearsResponseHeadersWhenRequestFails() to verify that a failed subsequent request clears stale response headers while preserving the request headers captured for that request.

3. Spotless Formatting and CI Checks

• Applied formatting using ./gradlew spotlessApply.
• Verified formatting using ./gradlew spotlessCheck.
• Ran the relevant component checks and tests successfully.

Commits

The PR contains two focused commits:

• 9dbe0f71: fix(http): support multi-valued headers, synchronize shared capture, and prevent stale response headers
• d4bd310c: test(http): add unit tests for multi-valued headers, stale header prevention, and concurrent calls

All local checks and tests pass. The changes are ready for review. Thanks for your time and feedback!

[Copilot]

🟡 Changes recommended

The concurrency test must shut down its executor when workers fail or execution is interrupted.

1 open issue

• Ensure executor shutdown on worker failure or interruption · New

2 resolved since last review

• Synchronize shared header capture to prevent concurrent-call races
• Clear response headers before proceeding to prevent stale headers

_{Review effort: Balanced}