restify / restify/node-restify

bodyParser ignores maxBodySize and buffers whole file in memory for file uploads

Open
#956 5 comments 2 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Bug Critical Good First Issue Help Wanted
Dominant language
JavaScript
Stars
10.7k
Forks
975
Avg merge
1d 6h
Merged PRs (30d)
5

Description

The bodyParser middleware seems to buffer the whole file in memory for file uploads.

Check the following minimum example:

'use strict';

var os = require( 'os' );
var restify = require( 'restify' );

var server = restify.createServer( {} );

server.use( restify.bodyParser( {
  // this should supposedly limit maximum upload size to 10 KiB
  maxBodySize: 10 * 1024,
  mapParms: true,
  mapFiles: true,
  keepExtensions: true,
  uploadDir: os.tmpdir()
} ) );

server.get( '/', function( req, res, next ) {
  res.json( { message: 'hello' } );
  next();
} );

server.post( '/upload', function( req, res, next ) {
  console.log( 'Received files:', req.files );
  res.send( 200 );
  next();
} );

server.listen( 8000 );

Then, upload a really big file to it:

# bigfile.dat has a couple of GiB
curl -F "file=@/path/to/bigfile.dat" localhost:8000/upload

This will send the process into accumulating tons of memory until it eventually runs out of memory and crashes with:

FATAL ERROR: JS Allocation failed - process out of memory

Am I making a mistake in setting up the middleware or is this a bug?

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Reproduce the upload with the provided bodyParser configuration and a large file, then trace restify.bodyParser through the repository to find where maxBodySize is applied. Check how file uploads are buffered and run the relevant existing tests, if present. Done means uploads over the limit are rejected without buffering the entire file or exhausting process memory.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript, node.js
Domain
api, backend
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
42/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.