Repository navigation
Return 400 for OTLP JSON requests with malformed ids - #21436
Merged
James Newton-King (JamesNK) merged 1 commit intoNov 10, 2026
Merged
James Newton-King (JamesNK) merged 1 commit into
James Newton-King (JamesNK) merged 1 commit into
Conversation
The OTLP HTTP JSON bind path only caught JsonException and BadHttpRequestException. Converting an invalid hex trace/span id or invalid base64 value throws FormatException/ArgumentException from the protobuf converter after the JSON itself parsed, so these escaped the bind handler and surfaced as an unhandled 500 instead of a 400. Catch FormatException/ArgumentException in the JSON case and return 400, matching the existing invalid-JSON handling. Add integration tests covering non-hex and odd-length trace ids.
Taha Koçal (tahakocal)
requested review from
James Newton-King (JamesNK) and
Adam Ratzman (adamint)
as code owners
October 30, 2026 08:25
Contributor
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 21436Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 21436" |
Contributor
Author
|
@microsoft-github-policy-service agree |
Contributor
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
Contributor
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
James Newton-King (JamesNK)
approved these changes
Nov 10, 2026
James Newton-King (JamesNK)
left a comment
James Newton-King (JamesNK)
left a comment
Member
There was a problem hiding this comment.
No issues found in the OTLP JSON binding error handling or its regression tests.
Validation passed: all 11 targeted integration tests, including non-hex and odd-length trace IDs, valid traces/logs/metrics, invalid JSON, and authentication cases.
Command: dotnet test --project tests\Aspire.Dashboard.Tests\Aspire.Dashboard.Tests.csproj --no-launch-profile -- --filter-class '*.OtlpHttpJsonTests' --filter-not-trait 'quarantined=true' --filter-not-trait 'outerloop=true'
Reviewed commit: d914580.
James Newton-King (JamesNK)
merged commit Nov 10, 2026
9a926d8
into
microsoft:main
462 of 470 checks passed
This was referenced Nov 11, 2026
Closed
Contributor
|
Taha Koçal (@tahakocal) FYI, this PR has been backported to release/17.0 in #22131, as part of a chain of dashboard backports (#22125 → #22132) applied in the order they merged to main. No action needed from you: I'll handle the merging. |
Jose Perez Rodriguez (joperezr)
added a commit
that referenced
this pull request
Nov 12, 2026
Combines the stacked dashboard backports into one PR for `release/17.0`. The commits are cherry-picked in stack order, one per source PR, onto the current `release/17.0` tip: 1. #22125: Store dashboard tags, resource order and metric pins in browser local storage (#21840) 2. #22126: Add dashboard resource-selector dropdown (#21861) 3. #22127: Rename the WithTags trait from Aspire_Dashboard_Tag to Aspire_Tag (#21881) 4. #22128: Restore single-line compact rows in the dashboard resource list (#21880) 5. #22129: Allocate the next dashboard run ID when two dashboards start in the same millisecond (#21913) 6. #22130: Simplify the dashboard around its two main jobs (#21943) 7. #22131: Return 400 for OTLP JSON requests with malformed ids (#21436) 8. #22132: Address deferred review feedback for the modern dashboard (#21766) ## Conflict resolution The stack was cut from an older `release/17.0`. Since then, #22123 (Harden Go SDK runtime) landed and regenerated `TwoPassScanningGeneratedAspire.verified.go`. That conflicted with #21766, which replaces `WithDashboardPosition` with `WithDisplayOrder`. The resolution drops `WithDashboardPosition` and regenerates the snapshot with the current Go generator, so `WithDisplayOrder` now uses the hardened `s.Err()` pattern. The other seven commits applied cleanly. ## Validation All `Aspire.Hosting.CodeGeneration.*.Tests` projects pass locally (Go, Java, Python, Rust, TypeScript). --------- Co-authored-by: Sébastien Ros <1165805+sebastienros@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: James Newton-King <james@newtonking.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: David Fowler <davidfowl@gmail.com> Co-authored-by: davidfowl <95136+davidfowl@users.noreply.github.com> Co-authored-by: Maddy Montaquila <maddyleger1@gmail.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Maddy Montaquila <maddy@MadBook-Pro-20.local> Co-authored-by: Taha Koçal <tahakocalgs@gmail.com> Co-authored-by: Jose Perez Rodriguez <joperezr@microsoft.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
The OTLP HTTP JSON ingestion path returns an unhandled 500 (instead of 400) when a request contains a syntactically valid JSON body with a malformed field value, e.g. a
traceId/spanIdthat is not valid hex, or an invalid base64bytesValue.MessageBindable<TMessage>.BindAsync(src/Aspire.Dashboard/Otlp/Http/OtlpHttpEndpointsBuilder.cs) only catchesJsonExceptionandBadHttpRequestException. The JSON → protobuf conversion inOtlpJsonProtobufConverterruns after the JSON has parsed successfully; converting a bad hex id throwsFormatException(byte.Parse) orArgumentException(odd length), andByteString.FromBase64throwsFormatException. These escape the bind handler and surface as a 500.A malformed id is a bad request, so it should be a 400 like the existing invalid-JSON case.
Fix
Catch
FormatException/ArgumentExceptionin the JSON bind case and return400 Bad Request, mirroring the existingJsonExceptionhandling (debug-logged, no 500).Tests
Added
CallService_Traces_JsonContentType_InvalidTraceId_BadRequesttoOtlpHttpJsonTests, covering a non-hex id (FormatExceptionpath) and an odd-length id (ArgumentExceptionpath). Both assert400. Without the fix they return500.