Skip to content

[oss-candidate] fix: use --data-binary in cURL sample for application/octet-stream body - #1

Closed
askalf wants to merge 1 commit into
mainfrom
fix/octet-stream-request-sample
Closed

[oss-candidate] fix: use --data-binary in cURL sample for application/octet-stream body#1
askalf wants to merge 1 commit into
mainfrom
fix/octet-stream-request-sample

Conversation

@askalf

@askalf askalf commented Sep 24, 2026 •
edited
Loading

Copy link
Copy Markdown

Summary

  • For an application/octet-stream request body, the cURL request sample (the default sample language) printed --data @<file>. curl's --data strips carriage returns, newlines and NUL bytes from a file it reads, so copying the sample uploads a corrupted file.
  • convertRequestToSample now passes httpsnippet-lite's own cURL option { binary: true } when postData.mimeType is application/octet-stream, so the sample prints --data-binary @<file>, which sends the file byte-for-byte.
  • Diff: 3 lines in convertRequestToSample.ts, plus one test next to the existing tests in convertRequestToSample.spec.ts.
  • Refs Incorrect Request Sample for application/octet-stream Requests stoplightio/elements#2744 (cURL half of it; see Boundaries for the other languages).
$ cd packages/elements-core && npx jest --maxWorkers=2 src/components/RequestSamples/__tests__/convertRequestToSample.spec.ts
# base (convertRequestToSample.ts from main, new test kept)
  application/octet-stream body
    ✕ should send the file unmodified with --data-binary in curl (4 ms)

  ● application/octet-stream body › should send the file unmodified with --data-binary in curl

    expect(received).toContain(expected) // indexOf

    Expected substring: "--data-binary @some-file.bin"
    Received string:    "curl --request POST \\
      --url https://todos.stoplight.io/todos \\
      --header 'Content-Type: application/octet-stream' \\
      --data @some-file.bin"

Tests:       1 failed, 36 passed, 37 total
Snapshots:   34 passed, 34 total

# with the fix
  application/octet-stream body
    ✓ should send the file unmodified with --data-binary in curl (1 ms)

Tests:       37 passed, 37 total
Snapshots:   34 passed, 34 total

Upstream

Bug

When an operation's request body is application/octet-stream (for example type: string, format: binary) and the user picks a file in Try It, buildHarRequest emits postData = { mimeType: 'application/octet-stream', text: '@<file name>' }. convertRequestToSample hands that to HTTPSnippet.convert(language, library) without options, and httpsnippet-lite's cURL target writes --data @<file name>. For --data, curl reads the file in text mode and removes CR, LF and NUL bytes, so the sample a user copies uploads a different body from the file they picked. Anyone documenting a binary upload endpoint with Elements hits this; cURL is the default sample (selectedLanguageAtom defaults to shell, selectedLibraryAtom to curl in RequestSamples.tsx:47-48).

Repro

Generated sample (base) and the same command run against a local echo server that reports the bytes it received (/agent-output/oss/elements/echo2744.js), using a 7-byte file containing CR, LF and NUL:

some-file.bin = 610d0a6200630a (7 bytes)
### base-generated cURL sample (verbatim):
curl --request POST \
  --url https://todos.stoplight.io/todos \
  --header 'Content-Type: application/octet-stream' \
  --data @some-file.bin
### head-generated cURL sample (verbatim):
curl --request POST \
  --url https://todos.stoplight.io/todos \
  --header 'Content-Type: application/octet-stream' \
  --data-binary @some-file.bin
### base sample run, host swapped to local echo server:
received 3 bytes: 616263
### head sample run, host swapped to local echo server:
received 7 bytes: 610d0a6200630a

The samples above were produced by calling the real convertRequestToSample on base and head (probe spec, not committed; outputs /agent-output/oss/elements/probe-{base,head}.txt). curl 8.22.0.

Fix

     const snippet = new HTTPSnippet(request);
-    let converted = await snippet.convert(language, library);
+    // Makes cURL use --data-binary, as --data strips newlines and null bytes from the uploaded file
+    const options = request.postData?.mimeType === 'application/octet-stream' ? { binary: true } : undefined;
+    let converted = await snippet.convert(language, library, options);

binary is an existing option of httpsnippet-lite's cURL client (targets/shell/curl/client.js: const flag = binary ? '--data-binary' : arg('data')). It is read by the cURL client only (grep of dist/cjs/targets for binary finds only shell/curl), so passing it for other targets is a no-op; measured below.

Alternatives rejected:

  • Post-processing the string (converted.replace(/--data\b/, '--data-binary'), like the existing urlencoded rewrite): gives the same output here but reimplements a flag the library already has, and the regex would also need to avoid --data-urlencode.
  • Also gating on language === 'shell' && library === 'curl': behaviourally identical (only cURL reads the option) and longer.
  • Case-insensitive or parameter-tolerant mime comparison: the producer (buildHarRequest line 299) only emits the @<file> body for the exact string application/octet-stream, so a looser check here changes nothing reachable (Boundaries rows 6-7).
  • Base64 or other per-language file handling (the issue's "possible solution"): a feature across ~30 targets, not a fix; out of scope.

Test evidence

Test File Base Head Kind
application/octet-stream body › should send the file unmodified with --data-binary in curl packages/elements-core/src/components/RequestSamples/__tests__/convertRequestToSample.spec.ts FAIL PASS regression (new)

Existing tests that pin the other side of the new condition (unchanged, pass on both arms): the 34 given <language>, convertRequestToSample converts a request to a sample snapshots (JSON body). Mutant "always pass { binary: true }" fails given shell / curl (snapshot --data becomes --data-binary): Tests: 1 failed, 36 passed / Snapshots: 1 failed, 33 passed (/agent-output/oss/elements/mutant-always.txt). Mutant "never pass it" is base, killed by the new test.

Runs:

  • npx jest --maxWorkers=2 src/components/RequestSamples/__tests__/convertRequestToSample.spec.ts base: 1 failed, 36 passed; head: 37 passed; 34 snapshots passed on both (transcripts /agent-output/oss/elements/{base,head}-arm.txt).
  • npx jest --maxWorkers=2 src/components/RequestSamples head: 2 suites, 43 passed, 34 snapshots passed (includes RequestSamples.test.tsx).
  • npx eslint on both touched files: rc=0. npx prettier --check on both: "All matched files use Prettier code style!". npx tsc --noEmit -p packages/elements-core/tsconfig.json: rc=0, no output.

Verification method

executed in the Linux container: Node v24.19.0, yarn 1.22.22 (yarn install --frozen-lockfile --ignore-scripts), jest 26 via packages/elements-core config, curl 8.22.0 for the wire check. No platform-specific code involved. Fork CI: Actions are not yet enabled on sprayberry-code/elements (operator card filed), so no fork run exists at this head; gh pr checks shows none.

Prior art

Policy

  • CONTRIBUTING.md @ main (2024fd9): no mention of AI, LLMs, generated code or agents. Applicable lines, verbatim:
    • "When you add a new component, implement a changing functional requirement, or fix a bug, you are expected to also deliver one or more tests that cover the feature being added/modified."
    • "Unit tests are currently located in a directory __tests__ close to the component being tested"
    • "yarn elements test" / "There is no need to build Elements beforehand, the test runs on the currently saved TypeScript code."
    • "To merge your changes into main you must create a PR for your changes and then have those changes approved by a member of the stoplight elements-owner team." / "Once your changes are complete, ensure all build checks are successfully passing, fill out the pull request template, and assign elements-owner as the reviewer."
    • "In all three packages, patch bumps are primarily for bug fixes." (Release section; versions are bumped by the release process.)
  • .github/pull_request_template.md @ main: "- [ ] Read CONTRIBUTING.md"
  • CODE_OF_CONDUCT.md: links the Stoplight community code of conduct; nothing on AI.
  • Absent at main (404): AGENTS.md, CLAUDE.md, AI_POLICY.md, .github/AI_POLICY.md, AI.md, AGENT_POLICY.md, .github/CONTRIBUTING.md, .changeset/config.json. No CLA, no DCO.
  • Tooling run: jest (file + directory), eslint (repo config), prettier --check (repo config), tsc for elements-core.
  • Note for the operator: most recent fix: commits on main also bump the three packages' package.json versions (e.g. 09edf7f, fe9e657). This candidate does not; add a patch bump only if a maintainer asks.

Disclosure facts for the operator

  • An AI agent read issue Incorrect Request Sample for application/octet-stream Requests stoplightio/elements#2744, located the cause in convertRequestToSample.ts, and confirmed it by running the real function on base and head.
  • The agent wrote the 3-line fix and the regression test, ran jest, eslint, prettier and tsc, and ran the generated cURL commands against a local echo server to show the byte difference.
  • The agent ran a mutant (option always on) to show the existing snapshot tests pin the non-octet-stream side.
  • No human has run the change in a browser or storybook.

Boundaries

# Predicate / input Fixed code does Pinned by
1 postData.mimeType === 'application/octet-stream', text @some-file.bin (file picked in Try It) cURL prints --data-binary @some-file.bin new test should send the file unmodified with --data-binary in curl (fails on base)
2 mimeType application/json (any non-octet-stream body) options undefined, output unchanged 34 existing snapshots; mutant "always binary" fails given shell / curl
3 postData undefined (no file picked, or GET) ?. gives undefined, options undefined, unchanged existing curly-bracket tests (postData: undefined) pass both arms; probe noBody identical on base and head for all 34 targets
4 octet-stream with literal text (no @) cURL prints --data-binary 'raw text'; for a literal argument curl sends the same bytes with either flag probe row only (octetText); not produced by Try It, whose binary body is a File or undefined (TryIt.tsx:150,174)
5 octet-stream with empty text '' cURL target writes no data flag (empty text is skipped), same on both arms probe octetEmptyText identical base/head; not produced by buildHarRequest (File or undefined)
6 mixed case Application/Octet-Stream exact compare, options undefined, unchanged unreachable for the @file body: buildHarRequest:299 also compares exactly and emits params: [] instead of @file for this case (pre-existing, separate). Probe octetUpper identical base/head
7 application/octet-stream; foo=bar options undefined, unchanged unreachable: isBinaryContent (request-body-utils.ts:29) does not treat it as binary, so no file input and no @file body. Probe octetParams identical
8 other targets (httpie, python, node, ...) with octet-stream body option passed but read only by the cURL client; output unchanged probe: 10 HAR shapes x 34 targets = 340 samples, diff probe-base.txt probe-head.txt changes exactly 2 lines, both shell/curl. These targets still print the literal @some-file.bin string as the body (pre-existing, not changed by this PR; the rest of stoplightio#2744)
9 application/x-www-form-urlencoded / multipart/form-data options undefined; urlencoded --data-urlencode rewrite and multipart --form unchanged probe urlencoded, multipart identical base/head

Suggested upstream PR title

fix: use --data-binary in cURL request sample for application/octet-stream bodies

@askalf askalf added the oss-candidate Sprayberry Code candidate for upstream label Sep 24, 2026
@askalf
askalf marked this pull request as ready for review September 24, 2026 12:41
@askalf askalf added the verified Adversarially verified by a fresh run label Sep 25, 2026
@askalf

askalf commented Sep 25, 2026

Copy link
Copy Markdown
Author

Verification

Adversarial verification at head 46fec025db30b98996b6c942b3e8287fe20427ef on fix/octet-stream-request-sample, base 2024fd9e1b47c5a6b1bde701241d924271ed4394 (main now 414608ff, a dependabot prismjs bump touching none of the files below).

Superseded check first: upstream issue stoplightio#2744 is still open; gh api repos/stoplightio/elements/issues/2744/timeline cross-references only this fork PR. No maintainer fix landed.

File test (re-run, verbatim)

npx jest --maxWorkers=2 src/components/RequestSamples/__tests__/convertRequestToSample.spec.ts --verbose

Head: Tests: 37 passed, 37 total / Snapshots: 34 passed, 34 total (includes the new should send the file unmodified with --data-binary in curl).

Base (production file swapped to 2024fd9e1, test file kept at head): Tests: 1 failed, 36 passed, 37 total — the new test fails with:

Expected substring: "--data-binary @some-file.bin"
Received string:    "curl --request POST \
  --url https://todos.stoplight.io/todos \
  --header 'Content-Type: application/octet-stream' \
  --data @some-file.bin"

Production source restored to head afterward; git status clean.

Mutant (always pass { binary: true })

Confirmed independently: the shell/curl snapshot for the existing JSON-body test flips from --data '{...}' to --data-binary '{...}' and fails — Tests: 1 failed, 36 passed, 37 total, Snapshots: 1 failed, 33 passed, 34 total.

Ledger rebuild from the diff

Diff is 3 lines in convertRequestToSample.ts (compute options from postData.mimeType, pass to snippet.convert) plus 1 test. Rebuilt the body's 9-row Boundaries ledger independently against buildHarRequest (producer) and request-body-utils.ts (isBinaryContent). Same 9 rows; no missing row found.

Per ledger-row-needs-its-fixture, attacked every row the body closes by argument (3, 4, 5, 6, 7, 8, 9) rather than trusting the prose. Built a throwaway probe (packages/elements-core/src/components/RequestSamples/__tests__/probe-rows.spec.ts, deleted before commit, never pushed) driving the real convertRequestToSample across 7 HAR shapes (octet-stream raw text, octet-stream empty text, mixed-case mimeType, parameterized mimeType, urlencoded, multipart, no body) x all 34 language/library targets = 238 combinations, run once against head and once against base (production file swapped, restored after):

diff probe-base-lines.txt probe-head-lines.txt — exactly 1 of 238 lines changes: octetText|shell/curl (row 4, already in the body, --data → --data-binary for a literal-text octet-stream body). Every other combination is byte-identical base vs head, confirming:

  • Row 3 (postData undefined): unchanged, and separately pinned by the file's own existing postData: undefined fixtures.
  • Row 5 (empty text): unchanged — curl's target skips the flag entirely when postData.text is falsy.
  • Row 6 (mixed-case mimeType) / Row 7 (parameterized mimeType): unchanged, consistent with the body's claim that buildHarRequest:299's exact-string compare never produces an @file body for these, so the new code path in convertRequestToSample is unreachable through Try It for either.
  • Row 8 (other targets, octet-stream body): unchanged for all targets except shell/curl, confirming binary is read only by httpsnippet-lite's cURL client.
  • Row 9 (urlencoded / multipart): unchanged, the pre-existing --data-urlencode rewrite and --form path are untouched.

No hole found. All 9 rows in the diff-rebuilt ledger match the body's ledger and are each either fixtured (rows 1, 2, 3) or genuinely unreachable/no-op as argued (rows 4-9, now measured directly rather than accepted on prose).

Lint / format / types

  • npx eslint on both touched files: rc=0, no output.
  • npx prettier --check on both touched files: "All matched files use Prettier code style!"
  • npx tsc --noEmit (packages/elements-core): rc=0, no output.

CI

gh pr checks 1 --repo sprayberry-code/elements: "no checks reported on the 'fix/octet-stream-request-sample' branch" — fork Actions not enabled (absence, not failure; consistent with the operator card already on file for this fork tier).

Rules

Rules: ledger-row-needs-its-fixture=covered(probe-rows.spec.ts, 238-case base/head diff) | prior-art-recheck-at-gate=covered(re-checked issue stoplightio#2744 timeline at gate, still open) | no-control-cases-in-the-suite=unreachable(no test in the suite is declared a control) | run-every-ci-step-not-just-the-red-one=unreachable(fork Actions not enabled, nothing to run)

sprayberry-redline
sprayberry-redline approved these changes Sep 25, 2026
View reviewed changes

@sprayberry-redline sprayberry-redline left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review from the Sprayberry Labs fleet code reviewer.
Reviewed by the gating lane (gating review).

Verdict: APPROVE. Ready for the operator to submit upstream. Reviewed at head 46fec025db30b98996b6c942b3e8287fe20427ef against base 2024fd9e1b47c5a6b1bde701241d924271ed4394.

What I checked

Bug is real on the base. I fetched convertRequestToSample.ts at the base sha. It calls snippet.convert(language, library) with no options. In httpsnippet-lite 3.0.5 (dist/cjs/targets/shell/curl/client.js:50) binary defaults to false, and for a raw postData.text body line 136 does const flag = binary ? '--data-binary' : arg('data'), so the HAR that buildHarRequest emits for a picked file (text: '@<name>', build-request.ts:299-303 at base) becomes --data @<name>. curl's --data @file strips CR, LF and NUL, so the copied sample uploads a different body from the file. The body's 7-byte echo-server run (3 bytes received on base, 7 on head) matches what the code says will happen.

The fix. convertRequestToSample.ts:15-17:

const options = request.postData?.mimeType === 'application/octet-stream' ? { binary: true } : undefined;
let converted = await snippet.convert(language, library, options);

binary is a declared member of the library's CurlOptions; convert's third argument is options?: any so the call type-checks. I grepped the shipped targets for binary: only shell/curl reads it, so passing it for the other 33 targets is a no-op, which the verification's 238-case base/head probe measured (one line changed, shell/curl). The existing --data-urlencode rewrite is gated on a urlencoded Content-Type header and cannot coincide with an octet-stream postData.mimeType from the producer, so the two rewrites do not interact.

The test proves it. The new case builds the exact HAR shape the producer emits and asserts toContain('--data-binary @some-file.bin'). On base the cURL client emits --data @some-file.bin, which does not contain that substring, so it fails; on head it passes. The body's verbatim before/after output shows exactly that (1 failed / 36 passed, then 37 passed). The "always binary" mutant is killed by the existing given shell / curl snapshot, so the other side of the predicate is pinned by existing tests.

Boundaries. Walked the one predicate the diff adds (postData?.mimeType === 'application/octet-stream'): postData undefined (existing curly-bracket fixtures, postData: undefined), non-octet-stream mimeType (34 snapshots), exact match (new test). Mixed-case and parameterised mimeTypes cannot reach this path with an @file body because buildHarRequest:299 compares the same exact string, and the ledger says so with the producer line. No unpinned reachable input.

Prior art, re-run. gh search prs --repo stoplightio/elements for octet-stream, data-binary, 2744, binary request sample: no PR about the cURL sample flag (the data-binary hits are dependency bumps and stoplightio#807/stoplightio#1715 file-upload work from years ago). gh search issues for octet-stream: only stoplightio#2744 (still open, 0 comments) and closed stoplightio#2615. The last three commits touching convertRequestToSample.ts on upstream main (stoplightio#2835, stoplightio#2799, stoplightio#2608) do not address this. Not a duplicate.

Policy. CONTRIBUTING at main requires tests with bug fixes in a __tests__ directory next to the component; the diff does that. No AI policy, CLA, DCO, changeset or mandated trailer. Commit 46fec025 has a conventional fix: subject in the repo's style and an empty body; no AI attribution in the commit, branch or title.

Reads-as-generated pass. No em dashes, filler words, control cases, patch narration or history in the diff, the commit, or the title. The test name and fixture shape match the file's existing describe('curly bracket encoding') block. The one source comment states why --data-binary is needed (a fact the code cannot show) and does not restate the line. 23 test lines for a 3-line fix.

CI. gh pr checks reports none (fork Actions not enabled; the body says so and the operator card exists). Verification was executed locally with verbatim jest, eslint, prettier and tsc output.

Notes for the operator

  • Recent fix: commits on upstream main bump the three packages' package.json versions; this PR does not. Add a patch bump only if a maintainer asks (the body already records this).
  • Upstream's PR template has a "Read CONTRIBUTING.md" checkbox and asks for elements-owner as reviewer; tick and assign on the upstream PR.

What's good

Minimal, uses the library's own option instead of post-processing the string, the ledger names the producer line for every "unreachable" row, and the alternatives section rejects the regex rewrite for a concrete reason (--data-urlencode collision).

@askalf askalf added ready-for-operator Gated; operator submits upstream submitted Submitted upstream labels Sep 25, 2026
@askalf

askalf commented Sep 25, 2026

Copy link
Copy Markdown
Author

Submitted upstream for review.

@askalf askalf closed this Sep 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
@sprayberry-redline sprayberry-redline sprayberry-redline approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

oss-candidate Sprayberry Code candidate for upstream ready-for-operator Gated; operator submits upstream submitted Submitted upstream verified Adversarially verified by a fresh run

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@askalf @sprayberry-redline