Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #290 #291

Merged
merged 1 commit into from
Oct 8, 2021
Merged

Fix #290 #291

merged 1 commit into from
Oct 8, 2021

Conversation

jeanbmar
Copy link
Contributor

@jeanbmar jeanbmar commented Oct 4, 2021

This PR fixes #290.
To avoid a breaking change, everything but buffer instances remains unchanged.

To avoid a breaking change, anything but buffer instances remain unchanged.
@jeanbmar jeanbmar changed the title Fix https://github.com/richardgirges/express-fileupload/issues/290 Fix #290 Oct 4, 2021
@richardgirges richardgirges merged commit a00dd5d into richardgirges:master Oct 8, 2021
@richardgirges richardgirges mentioned this pull request Oct 8, 2021
@richardgirges
Copy link
Owner

richardgirges commented Oct 8, 2021

Hi @jeanbmar - I had to revert this PR due to a failing test (see below). This failing test is mission-critical and introduces a large breaking change. I appreciate the PR though, and if you can spare some time to resolve, we can get this re-merged. Please open a new PR, thanks!

  162 passing (10s)
  1 failing

  1) Test Single File Upload w/ .mv() Promise
       fail when using HEAD:
     Error: write EPIPE
      at afterWriteDispatched (internal/stream_base_commons.js:154:25)
      at writeGeneric (internal/stream_base_commons.js:145:3)
      at Socket._writeGeneric (net.js:786:11)
      at Socket._write (net.js:798:8)
      at doWrite (_stream_writable.js:403:12)
      at clearBuffer (_stream_writable.js:542:7)
      at Socket.Writable.uncork (_stream_writable.js:338:7)
      at ClientRequest.OutgoingMessage.uncork (_http_outgoing.js:244:17)
      at connectionCorkNT (_http_outgoing.js:690:8)
      at processTicksAndRejections (internal/process/task_queues.js:84:21)

@richardgirges
Copy link
Owner

Nevermind! My mistake. This test was failing regardless of your PR. I apologize for the confusion. I will get this PR re-merged and deployed in the next release. Thank you!

@jeanbmar
Copy link
Contributor Author

jeanbmar commented Oct 8, 2021

Thanks!

@DanRibbens
Copy link

Thanks everyone for working on this! @richardgirges Is there any timeline on next release?
We're waiting on this to close a dependent issue.

@jeanbmar jeanbmar deleted the patch-1 branch November 6, 2021 17:53
@jmikrut
Copy link

jmikrut commented Feb 3, 2022

Hey @richardgirges — just checking on a status update here as Payload requires this to be resolved to close an open issue we have.

Thank you!

@richardgirges
Copy link
Owner

Hi @jmikrut - this has been released in v1.2.1+.

I apologize, as the release notes did not have #291 listed. I've updated the release notes to reflect this update:
https://github.com/richardgirges/express-fileupload/releases/tag/v1.2.1

If you're running on Node v12+, I would recommend upgrading to the latest version (v1.3.1) to take advantage of the latest security patches.

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.

Passing a Buffer body will pollute req.body when used along with processNested
4 participants