[justadityaraj]

Fixes #1120.

Problem

Base64 multipart bodies may contain CRLF line wrapping. Formidable counted those characters when choosing four-character decode chunks, so decoded file data could be truncated or corrupted.

Fix

Remove CRLF from base64 data before calculating the next quartet boundary. The multipart integration regression covers a line-wrapped base64 upload and verifies its decoded bytes.

User impact

Line-wrapped base64 uploads now preserve their original file contents.

Verification

• node --experimental-vm-modules ./node_modules/jest/bin/jest.js --runInBand test/standalone/content-transfer-encoding.test.js — 2 tests passed, including the new regression.
• node --test ./test-node/**/*.test.js — 11 tests passed.
• node ./node_modules/eslint/bin/eslint.js --quiet src/plugins/multipart.js — passed.
• node ./node_modules/prettier/bin-prettier.js --check src/plugins/multipart.js test/standalone/content-transfer-encoding.test.js — passed.
• pnpm run build-package — Rollup build passed.
• Full Jest suite — 85 passed; 10 failures are limited to existing Windows path expectations and an ECONNRESET assertion outside the changed files.

Greptile Summary

This PR fixes line-wrapped base64 multipart uploads by removing CRLF characters before calculating decode quartet boundaries.

• Normalizes base64 part chunks before incremental decoding.
• Adds an integration regression test verifying the decoded file bytes.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename	Overview
src/plugins/multipart.js	Removes CRLF characters before calculating complete base64 quartets, preserving decoded bytes across wrapped input chunks.
test/standalone/content-transfer-encoding.test.js	Adds multipart coverage for a CRLF-wrapped base64 upload and verifies the complete decoded payload.

_{Reviews (4): Last reviewed commit: "Merge master into fix/base64-crlf-chunks" | Re-trigger Greptile}

Context used:

• Knowledge Base — Multipart upload handling

[GrosSacASac]

Why are you using a regex instead of a string ?

[justadityaraj]

Because this is streaming input: \r and \n can arrive in separate parser chunks, and the body may contain multiple line breaks. A string replacement would remove only the first complete \r\n pair; replaceAll() is also unavailable on the supported Node 14.0.0. /[\r\n]/g removes every line-break byte independently while preserving that compatibility.

[GrosSacASac]

But formidable project only supports node lts

[justadityaraj]

But formidable project only supports node lts

right, updated in fc38c60 to use separate string replacements for \r and \n preserving split chunk handling

[tunnckoCore]

@justadityaraj thanks.

---

weird.. why i need to approve workflow runs for prs? ooh.. i think i touched a setting recently, ha. 😆

[kilisamemarisaaa]

I independently exercised the full form.parse(req) path with adversarial input chunk boundaries on base a78b8f48d54783a9d524bdde88838fc257e6f01e and current head 6f99334f80ec2fe9abb9c95ec98c8e119d410b57:

• Base: 32 failed / 16 passed. Head: all 48 passed.
• Wrapped base64: widths 4, 5 and 76; whole-body, byte-by-byte, and CR/LF split across input chunks; binary payloads covering both padding lengths and no padding, plus UTF-8 text containing actual newline bytes.
• All ten unwrapped-base64/binary controls pass on both versions. Two additional cases mix consecutive base64 and binary file parts.
• The probe checks exact output bytes and file sizes. A progress observer verifies that the intended request chunks actually reach the parser separately; asynchronous output writes also exercise pause/resume.

This confirms the split-CR/LF behavior beyond the existing whole-body regression. Existing content-transfer-encoding tests pass (2), as do the Node tests (11).

Environment/limits: Windows, Node.js 24.12.0, pnpm 10.8.1 with the frozen lockfile; installation scripts disabled. Inputs are asynchronous in-memory request streams with headers and real multipart parsing, and outputs use the public custom Writable option. No live HTTP server, persistent-file I/O, Node 18/20/22 or full Jest-suite verification in this independent probe. ESLint reports only checkout CRLF formatting errors on the changed source; feeding the same file with LF line endings through ESLint's stdin interface passes without changing the checkout.

Complete independent 48-case probe

Save as formidable-1122-chunks.mjs outside the checkout, install dependencies in each checkout, then run:

node formidable-1122-chunks.mjs <base-checkout>
node formidable-1122-chunks.mjs <head-checkout>

import test from 'node:test'
import assert from 'node:assert/strict'
import { Readable, Writable } from 'node:stream'
import { finished } from 'node:stream/promises'
import { setImmediate } from 'node:timers/promises'
import { resolve } from 'node:path'
import { pathToFileURL } from 'node:url'

const root = resolve(process.argv[2])
const { default: formidable } = await import(pathToFileURL(resolve(root, 'src/index.js')))
const boundary = 'independent-formidable-1122'

function multipart(parts) {
  const buffers = []
  for (const part of parts) {
    buffers.push(Buffer.from(`--${boundary}\r\nContent-Disposition: form-data; name="${part.name}"; filename="${part.name}.bin"\r\nContent-Type: application/octet-stream\r\nContent-Transfer-Encoding: ${part.encoding}\r\n\r\n`))
    if (part.encoding === 'base64') {
      const encoded = part.bytes.toString('base64')
      const lines = part.width ? encoded.match(new RegExp(`.{1,${part.width}}`, 'g')) : [encoded]
      buffers.push(Buffer.from(lines.join('\r\n')))
    } else {
      buffers.push(part.bytes)
    }
    buffers.push(Buffer.from('\r\n'))
  }
  buffers.push(Buffer.from(`--${boundary}--\r\n`))
  return Buffer.concat(buffers)
}

function chunkBody(body, delivery) {
  if (delivery === 'whole') return [body]
  const chunks = []
  if (delivery === 'split-crlf') {
    let start = 0
    for (let index = 0; index < body.length; index++) {
      if (body[index] === 13) {
        chunks.push(body.subarray(start, index + 1))
        start = index + 1
      }
    }
    if (start < body.length) chunks.push(body.subarray(start))
  } else {
    const size = delivery === 'bytewise' ? 1 : 13
    for (let index = 0; index < body.length; index += size) {
      chunks.push(body.subarray(index, index + size))
    }
  }
  return chunks
}

async function check(parts, delivery) {
  const body = multipart(parts)
  const input = chunkBody(body, delivery)
  const req = Readable.from((async function * () {
    for (const chunk of input) {
      yield chunk
      // Separate turns ensure the requested chunks reach Formidable separately.
      await setImmediate()
    }
  })(), { objectMode: false })
  req.headers = {
    'content-type': `multipart/form-data; boundary=${boundary}`,
    'content-length': String(body.length)
  }
  const outputs = new Map()
  const receivedSizes = []
  let previousReceived = 0
  const form = formidable({
    fileWriteStreamHandler(file) {
      const chunks = []
      outputs.set(file.originalFilename, chunks)
      return new Writable({
        write(chunk, encoding, done) {
          chunks.push(Buffer.from(chunk))
          // Exercise the existing pause/resume path, without replacing it.
          setImmediate().then(() => done(), done)
        }
      })
    }
  })
  form.on('progress', (received) => {
    if (received !== 0) {
      receivedSizes.push(received - previousReceived)
      previousReceived = received
    }
  })
  const sourceFinished = finished(req)
  try {
    const [fields, files] = await form.parse(req)
    // The closing multipart delimiter can complete parsing before the source's
    // trailing CRLF arrives. Wait for source end before checking delivery.
    await sourceFinished
    await setImmediate()
    assert.equal(Object.keys(fields).length, 0)
    assert.deepEqual(receivedSizes, input.map(chunk => chunk.length), 'request chunk delivery')
    assert.equal(previousReceived, body.length)
    assert.equal(Object.keys(files).length, parts.length)
    assert.equal(outputs.size, parts.length)
    for (const part of parts) {
      const actual = Buffer.concat(outputs.get(`${part.name}.bin`))
      assert.equal(actual.length, part.bytes.length, `${part.name}: byte length`)
      assert.deepEqual(actual, part.bytes, `${part.name}: exact bytes`)
      assert.equal(files[part.name].length, 1)
      assert.equal(files[part.name][0].size, part.bytes.length)
    }
  } finally {
    req.destroy()
  }
}

const payloads = [
  ['binary256-padding2', Buffer.from(Array.from({ length: 256 }, (_, index) => index))],
  ['binary257-padding1', Buffer.from(Array.from({ length: 257 }, (_, index) => index % 256))],
  ['binary258-no-padding', Buffer.from(Array.from({ length: 258 }, (_, index) => index % 256))],
  ['unicode-and-newlines', Buffer.from('中文\r\nline two\nend')]
]

for (const [label, bytes] of payloads) {
  for (const width of [4, 5, 76]) {
    for (const delivery of ['whole', 'bytewise', 'split-crlf']) {
      await test(`wrapped ${label}, width ${width}, ${delivery}`, { timeout: 5000 }, () =>
        check([{ name: 'upload', bytes, encoding: 'base64', width }], delivery))
    }
  }
  for (const delivery of ['whole', 'bytewise']) {
    await test(`unwrapped control ${label}, ${delivery}`, { timeout: 5000 }, () =>
      check([{ name: 'upload', bytes, encoding: 'base64' }], delivery))
  }
}

for (const delivery of ['whole', 'bytewise']) {
  await test(`binary CR/LF control, ${delivery}`, { timeout: 5000 }, () =>
    check([{ name: 'raw', bytes: payloads[0][1], encoding: 'binary' }], delivery))
}

for (const delivery of ['bytewise', 'chunks13']) {
  await test(`mixed consecutive parts, ${delivery}`, { timeout: 5000 }, () => check([
    { name: 'first', bytes: payloads[0][1], encoding: 'base64', width: 5 },
    { name: 'raw', bytes: payloads[1][1], encoding: 'binary' },
    { name: 'last', bytes: payloads[3][1], encoding: 'base64', width: 76 }
  ], delivery))
}

[justadityaraj]

Thanks @kilisamemarisaaa, that's a far more thorough check than the regression test I added, especially the CR/LF split across chunk boundaries.

On the ESLint note: both committed files have LF endings (no CR bytes in either), so those errors come from the Windows checkout converting line endings, not from the change itself.

@tunnckoCore the CI and CodeQL runs for this PR expired on Oct 10 after waiting 30 days for workflow approval, so they never actually ran. Could you re-run them when you get a chance? The branch is up to date with master.