Skip to content

fix: use json.Marshal instead of json.MarshalIndent for multipart data_json#823

Merged
dangrondahl merged 3 commits intomainfrom
fix/json-marshal-multipart-data
Apr 23, 2026
Merged

fix: use json.Marshal instead of json.MarshalIndent for multipart data_json#823
dangrondahl merged 3 commits intomainfrom
fix/json-marshal-multipart-data

Conversation

@dangrondahl
Copy link
Copy Markdown
Contributor

Summary

  • Replace json.MarshalIndent with json.Marshal for multipart form data_json fields
  • Pretty-printing inflated payloads ~2x, causing --attestation-data and --user-data to hit the server's 1024 KB per-part limit at ~400-500 KB instead of ~1 MB
  • Add test asserting multipart JSON fields contain no indentation

Closes #822

Test plan

  • New test TestMultipartFieldJSON_IsCompact verifies compact JSON in multipart fields
  • All existing requests tests pass

…a_json

Pretty-printing the JSON in multipart form fields inflated payloads
by ~2x, causing --attestation-data and --user-data to hit the
server's 1024 KB per-part limit at ~400-500 KB on disk instead of
the expected ~1 MB. Compact marshalling is sufficient — the server
deserializes regardless of formatting.

Closes #822
@claude
Copy link
Copy Markdown
Contributor

claude Bot commented Apr 23, 2026

Claude finished @dangrondahl's task in 1m 57s —— View job


PR Review: fix: use json.Marshal instead of json.MarshalIndent for multipart data_json

  • Phase 1: Static analysis (go build, go vet, go mod tidy)
  • Phase 2: Code review (read changed files, assess quality)
  • Phase 3: Post review feedback

Static Analysis

All three checks pass cleanly — go build ./..., go vet ./..., and go mod tidy produce no issues.

Review Summary

This is a clean, well-scoped fix. The root cause (pretty-printed JSON doubling multipart payload size) is correctly identified, and the one-line fix at requests.go:179 is the right call.

What's good:

  • The fix is minimal and targeted — json.Marshal replaces json.MarshalIndent at the single marshalling site
  • The follow-up to pretty-print at the logging site only (PayloadOutput) preserves developer experience without affecting wire payloads
  • Two focused tests cover both sides: TestMultipartFieldJSON_IsCompact verifies compact wire format, TestMultipartDryRunOutput_IsPrettyPrinted verifies readable debug output
  • Clean commit history that follows the repo's TDD convention (fix: then green: commits)

One minor observation (non-blocking, posted as inline comment):

  • The value.([]byte) type assertion in PayloadOutput (pre-existing) would panic on unexpected types. A comma-ok guard would be more defensive, though it's safe today given the single call site.

No issues found with correctness, security, or performance. LGTM.

Comment thread internal/requests/requests.go
Comment thread internal/requests/requests_test.go
…tion

Address PR review feedback:
- Pretty-print compact JSON via json.Indent at PayloadOutput so
  dry-run/debug output remains readable
- Assert raw JSON bytes contain no newlines (catches any indentation
  style, not just 4-space)
Verifies that PayloadOutput pretty-prints compact JSON at the
logging site, so --dry-run output remains human-readable.
Comment thread internal/requests/requests.go
@dangrondahl dangrondahl merged commit 251f3bc into main Apr 23, 2026
20 checks passed
@dangrondahl dangrondahl deleted the fix/json-marshal-multipart-data branch April 23, 2026 13:21
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.

fix: use json.Marshal instead of json.MarshalIndent for multipart data_json field

2 participants