jkyberneees / low-http-server

HTTP Server implementation in top of uWebSocket.js
MIT License
50 stars 7 forks source link

multipart/form-data fixes #14

Open schamberg97 opened 3 years ago

schamberg97 commented 3 years ago

Fixes hangs that may occur with multipart/form-data file uploads. Influenced by https://github.com/jkyberneees/low-http-server/pull/13#issue-654476894 .

schamberg97 commented 3 years ago

This fix doesn't fix the inability to use low-http-server with multer, like https://github.com/jkyberneees/low-http-server/pull/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 commented 3 years ago

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 commented 3 years ago

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

herudi commented 3 years ago

@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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

@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 commented 3 years ago

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

thanks...

jkyberneees commented 2 years ago

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