http-server icon indicating copy to clipboard operation
http-server copied to clipboard

Create a clean stream using fs.createReadStream

Open Its-Just-Nans opened this issue 4 years ago • 1 comments

Relevant issues

#756 #634

The stream is piped in res. Next the stream generate an error The first argument must be one of type string or Buffer. Received type number and launchs the satus["500"]() function then the status["500"]() function modify headers (after the piped stream), that's the crashing error appear.

To counter that, the stream musn't create error, we can :

  • change stream = Readable.from(bytes) to stream = Readable.from(bytes.toString())
  • remove stream = Readable.from(bytes) because a correct stream is created with line 334

BUT using .toString() format the buffer and a different charset can produce an error (see toString()in npm run test). This PR is the second (and working) option

Contributor checklist
  • [ ] Provide tests for the changes (unless documentation-only)
  • [x] Provide explanation
  • [ ] Documented any new features, CLI switches, etc. (if applicable)
    • [ ] Server --help output
    • [ ] README.md
    • [ ] doc/http-server.1 (use the same format as other entries)
  • [x] The pull request is being made against the master branch
Maintainer checklist
  • [ ] Assign a version triage tag
  • [ ] Approve tests if applicable

Its-Just-Nans avatar Nov 14 '21 15:11 Its-Just-Nans

The idea was to reuse the stream and it works well with most supported versions of Node. Not sure how much of a performance issue it is to recreate the streamm and if it's worth fixing (#756 is not an issue with latest Node 12/14/16 and Node 10 is EOL)

zbynek avatar Aug 25 '22 15:08 zbynek