Skip to content

Conversation

@schamberg97
Copy link
Collaborator

Fixes hangs that may occur with multipart/form-data file uploads. Influenced by #13 (comment) .

@schamberg97
Copy link
Collaborator Author

schamberg97 commented May 27, 2021

This fix doesn't fix the inability to use low-http-server with multer, like #13, but is a smaller fix overall.

From what I understand, the remaining issue occurs in multer, but not its dependency, busboy. Here's the code I tried that uses only busboy (basically an example provided with busboy package):

const { router, server } = require('0http')({
  server: require('../src/server')()
})

const Busboy = require('busboy')

router.post('/upload', (req, res) => {
  const busboy = new Busboy({ headers: req.headers })
  busboy.on('file', function (fieldname, file, filename, encoding, mimetype) {
    console.log('File [' + fieldname + ']: filename: ' + filename + ', encoding: ' + encoding + ', mimetype: ' + mimetype)
    file.on('data', function (data) {
      console.log(data.toString()) // this should show the data we are trying to send via curl
      console.log('File [' + fieldname + '] got ' + data.length + ' bytes')
    })
    file.on('end', function () {
      console.log('File [' + fieldname + '] Finished')
    })
  })
  req.pipe(busboy)
})

server.listen(3000, () => { })

I tried using curl to submit file:

cd ~/Desktop/
touch test.txt
echo 1234 > test.txt
curl -X "POST" "http://localhost:3000/upload"  -F "file=@/Users/schamberg/Desktop/test.txt"

The busboy code produced the following result in terminal:

File [file]: filename: test.txt, encoding: 7bit, mimetype: text/plain
1234

File [file] got 5 bytes
File [file] Finished

So, my understanding is that multer uses some additional node.js APIs that low-http-server does not use.

@herudi:
Unfortunately, I think it is going to be time-consuming to figure out what is going on exactly and I am not going to have any more time to investigate it on my own before the 3rd of June. If you'd be willing to investigate the issue further, I'd be more than willing to help you integrate your changes. As a workaround, consider using busboy directly, if feasible. You'll basically need to write chunks into file directly using fs module.

@herudi
Copy link

herudi commented May 27, 2021

I'm try your code using busboy and still hang on twice request . .

server.js

...
     res.onData((bytes, isLast) => {
        if (!bytes.byteLength) {
          if (!isLast) return
          return handler(reqWrapper, resWrapper)
        }
        const chunk = Buffer.from(bytes)
        if (isLast) {
          reqWrapper.push(chunk)
          reqWrapper.push(null)
          if (!res.finished) {
            return handler(reqWrapper, resWrapper)
          }
          return
        }
        return reqWrapper.push(chunk)
      })
...

Example upload

const { router, server } = require('0http')({
  server: require('../src/server')()
})

const Busboy = require('busboy')

router.post('/upload', (req, res) => {
  const busboy = new Busboy({ headers: req.headers })
  busboy.on('file', function (fieldname, file, filename, encoding, mimetype) {
    console.log('File [' + fieldname + ']: filename: ' + filename + ', encoding: ' + encoding + ', mimetype: ' + mimetype)
    file.on('data', function (data) {
      // console.log(data.toString()) // this should show the data we are trying to send via curl
      console.log('File [' + fieldname + '] got ' + data.length + ' bytes')
    })
    file.on('end', function () {
      console.log('File [' + fieldname + '] Finished')
    })
  })
  req.pipe(busboy)
})

server.listen(3000, () => { })

Hang screenshot
hang

no error message or log...

Expect

File ['file]: filename: test.txt, encoding: 7bit, mimetype: text/plain
test 1234
File ['file] got 6 bytes
File ['file] Finished

any idea ? thanks...

@schamberg97
Copy link
Collaborator Author

@herudi have you tried the latest NPM version or the last commit in master

@herudi
Copy link

herudi commented May 27, 2021

@schamberg97 yes, i use the last commit master and latest yarn... but the original last commit master error in here

_read (size) {    
      return this.slice(0, size)  
}

just modify file in request.js and server.js from u right? but still hang.. thanks

@schamberg97
Copy link
Collaborator Author

Yup, sorry, was in a bit of a rush. Yeah, I actually meant changes in this Pull Request. Does it work with changes in this PR? Because for me, it does work

@herudi
Copy link

herudi commented May 27, 2021

sorry, for me it hangs for next requests. next request means, send with curl then stop curl and send again it will hang. so I tried it several times

thanks...

@schamberg97
Copy link
Collaborator Author

@herudi weird, I have no such issue while using this unmerged PR. I have tried it on MacOS, so I will try on Windows, perhaps I can reproduce it there

@herudi
Copy link

herudi commented May 28, 2021

well,, maybe you should try it on windows too. i am also using nodejs 12.x version. in the meantime.

thanks...

@jkyberneees
Copy link
Owner

Hi @schamberg97, thanks for supporting this issue. Should we go ahead and merge this PR?

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.

4 participants