Skip to content

Specify set of valid response content types#6535

Open
mrdziuban wants to merge 2 commits into
canton-network:mainfrom
obsidiansystems:mrdziuban/non-json-responses/6483
Open

Specify set of valid response content types#6535
mrdziuban wants to merge 2 commits into
canton-network:mainfrom
obsidiansystems:mrdziuban/non-json-responses/6483

Conversation

@mrdziuban

@mrdziuban mrdziuban commented Jul 23, 2026

Copy link
Copy Markdown

Fixes #6483

This updates HttpClient.createHttpFn to consider a response invalid if its content type is not ContentTypes.NoContentType, application/json, application/octet-stream, or text/plain. This results in httpClientWithErrors calling getApiErrorFromResponse, which produces an HttpCommandException and includes the raw response body in its error message.

ContentType.NoContentType is used for responses with no content type specified, like this, and the other three content types are the only ones used across all the OpenAPI specs in the repo. text/plain is used by devNetOnboardValidatorPrepare, and application/octet-stream is used by bulkStorageDownload.

Note that this allowlist approach does present a risk of future issues if a new endpoint is added using a different content type. A blocklist approach (e.g. only considering text/html invalid) would prevent this issue, but would capture fewer invalid responses.

Pull Request Checklist

Cluster Testing

  • If a cluster test is required, comment /cluster_test on this PR to request it, and ping someone with access to the DA-internal system to approve it.
  • If an upgrade test is required, comment /upgrade_test on this PR to request it, and ping someone with access to the DA-internal system to approve it.
  • If a hard-migration test is required (from the latest release), comment /hdm_test on this PR to request it, and ping someone with access to the DA-internal system to approve it.
  • If a logical synchronizer upgrade test is required (from canton-3.5), comment /lsu_test on this PR to request it, and ping someone with access to the DA-internal system to approve it.

PR Guidelines

  • Include any change that might be observable by our partners or affect their deployment in the release notes.
  • Specify fixed issues with Fixes #n, and mention issues worked on using #n
  • Include a screenshot for frontend-related PRs - see README or use your favorite screenshot tool

Merge Guidelines

  • Make the git commit message look sensible when squash-merging on GitHub (most likely: just copy your PR description).

Fixes canton-network#6483.

This updates `HttpClient.createHttpFn` to consider a response invalid if its content type is not `application/json`, `application/octet-stream`, or `text/plain`. This results in `httpClientWithErrors` calling `getApiErrorFromResponse`, which produces an `HttpCommandException` and includes the raw response body in its error message.

These three content types are the only ones used across all the OpenAPI specs in the repo. `text-plain` is used by [`devNetOnboardValidatorPrepare`](https://github.com/canton-network/splice/blob/main/apps/sv/src/main/openapi/sv-internal.yaml#L506-L508), and `application/octet-stream` is used by [`bulkStorageDownload`](https://github.com/canton-network/splice/blob/main/apps/scan/src/main/openapi/scan-stream-server.yaml#L46-L50).

Signed-off-by: Matt Dziuban <mrdziuban@gmail.com>
@mrdziuban

Copy link
Copy Markdown
Author

Pushed another commit (and updated the PR description) to also allow responses with ContentTypes.NoContentType. This was fortunately surfaced by a test failure, but it's a good example of why the allowlist approach is more risky than the blocklist approach. Let me know if you'd prefer to see a blocklist approach to minimize risk.

Signed-off-by: Matt Dziuban <mrdziuban@gmail.com>
@mrdziuban
mrdziuban force-pushed the mrdziuban/non-json-responses/6483 branch from 1210be8 to eaf6e38 Compare July 24, 2026 20:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Parse body of non-json responses

5 participants