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

empty file issue #226

Closed
koprda opened this issue Jun 23, 2020 · 7 comments
Closed

empty file issue #226

koprda opened this issue Jun 23, 2020 · 7 comments
Labels

Comments

@koprda
Copy link

koprda commented Jun 23, 2020

uploading "empty" zero-length file end with error in both modes memory and temp file

simple reproduction - truncate test file basketball.png and run test

@RomanBurunkov
Copy link
Collaborator

That is strange.
Can you turn module's debug output on and share it.

@RomanBurunkov
Copy link
Collaborator

I was able to reproduce it.

Looking into this.

@RomanBurunkov
Copy link
Collaborator

RomanBurunkov commented Jun 26, 2020

In case of in-memory upload this PR #15 affects empty file uploads.
@r3wt , could you please elaborate #14 issue?

Since we already have the following check in processMultipart:

      // Do not add file instance to the req.files if original name and size are empty.
      // Empty name and zero size indicates empty file field in the posted form.
      if (!name && size === 0) return;

Do we still need that check in fileFactory:

  // see: https://github.com/richardgirges/express-fileupload/issues/14
  // firefox uploads empty file in case of cache miss when f5ing page.
  // resulting in unexpected behavior. if there is no file data, the file is invalid.
  if (!fileUploadOptions.useTempFiles && !options.buffer.length) return;

I've commented

if (!fileUploadOptions.useTempFiles && !options.buffer.length) return;

And it looks checks in processMultipart do the job, so if I reproduced those issue correct, probably we can consider removing check in fileFactory.

@r3wt
Copy link
Contributor

r3wt commented Jun 26, 2020

@RomanBurunkov Maybe. I'm not actually sure, we might have to summon Richard to see if he knows. This was so long ago I barely remember it.

@RomanBurunkov
Copy link
Collaborator

Hi @richardgirges , do you remember anything about issue #14 and PR #15.
I can't figure out steps how to could be reproduced.

@RomanBurunkov
Copy link
Collaborator

Fixed with PR #233

@nusu
Copy link
Contributor

nusu commented Aug 7, 2020

I tried it, this doesn't seem fixed I'm going to fix it and push a following PR, can you open the issue @RomanBurunkov

edit: sorry I misread the issue, nothing wrong with the fix though it creates an empty file while it shouldn't I created a PR for that #241

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants