Skip to content

Send the transformed file on binaryBody uploads - #2380

Open
enyo wants to merge 1 commit into
mainfrom
binarybody-transformed
Open

enyo wants to merge 1 commit into
mainfrom
binarybody-transformed

Conversation

@enyo

@enyo enyo commented Sep 24, 2026

Copy link
Copy Markdown
Owner

Found while triaging #2238.

The bug

if (this.options.binaryBody) {
  if (files[0].upload.chunked) {
    const chunk = this._getChunk(files[0], xhr)!;
    xhr.send(chunk.dataBlock.data);
  } else {
    xhr.send(files[0]);        // the file as it was dropped
  }
}

files[0] is the original DropzoneFile. What should go on the wire is whatever transformFile returned, and it is sitting right there in dataBlocks[0].data — submitRequest just was not given it.

So with binaryBody: true and no chunking, transformFile is silently discarded. That includes resizeWidth and resizeHeight, which are implemented through transformFile: the preview shows a resized image, the file list shows the resized dimensions, and the server receives the full-size original. No error, no warning.

The two branches around it are both correct — the chunked one uses the chunk's own dataBlock, and the form-data path appends dataBlock.data — which is what makes this a gap rather than a design choice.

This is the presigned-PUT-to-a-bucket path, i.e. precisely where people resize before uploading.

The fix

submitRequest now takes dataBlocks. It is optional, so anything calling the method with three arguments keeps working and falls back to the old behaviour — there is nothing else it could send.

While in the signature: formData is now FormData | null, which is what the binary path has always passed it. That was a null as any at the call site, and xhr.send accepts null anyway, so the cast simply goes away.

Tests

Two in amazon-s3.js, which is where the binaryBody coverage lives:

  • should send what transformFile produced, not the original file — against main: expected File{ status: 'uploading', …(6) } to be Blob{}.
  • should send the file itself when there is nothing to transform — passes before and after. It is there to pin the default path, since the fallback means this is the branch most likely to be broken by a careless change later.

`submitRequest` sent `files[0]` -- the file as it was dropped -- for a
non-chunked binary upload, while the data that should go on the wire sat
unused in `dataBlocks[0].data`. Anything `transformFile` produced was
discarded, which silently includes `resizeWidth` and `resizeHeight`: the
preview showed a resized image and the server received the full-size
original. The chunked branch immediately above already used the chunk's
own `dataBlock`, and form-data uploads append it too.

`submitRequest` takes `dataBlocks` to reach it, optional so an existing
caller passing three arguments keeps working. Its `formData` parameter is
now `FormData | null`, which is what the binary path has always passed --
that was a `null as any` at the call site.
@github-actions

Copy link
Copy Markdown
Contributor

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 79.87% 893 / 1118
🔵 Statements 80.3% 942 / 1173
🔵 Functions 93.42% 199 / 213
🔵 Branches 77.04% 527 / 684
File Coverage
File Stmts Branches Functions Lines Uncovered Lines
Changed Files
packages/dropzone/src/dropzone.ts 78.05% 75.61% 93.33% 77.48% 238, 248, 273, 279-282, 367, 410-411, 470, 582, 604, 623-624, 731, 754-757, 794-796, 826-829, 852, 1026, 1056, 1060, 1149, 1183, 1193-1195, 1212-1213, 1219-1252, 1376, 1434, 1483-1484, 1612, 1636-1637, 1691, 1725, 1745-1776, 1784, 1798-1802, 1813, 1826-1831, 1849-1850, 1854-1855, 1878-1879, 1957-1972, 2017, 2033, 2204-2206, 2241, 2251, 2284-2318, 2325-2426, 2435-2438
Generated in workflow #160 for commit 88901b7 by the Vitest Coverage Report Action

This branch has not been deployed

No deployments
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.

1 participant