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.

Sign up for GitHub

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

Jump to bottom

Upload files are not getting deleted. #519

Open
varadekd opened this issue Jan 17, 2024 · 5 comments · May be fixed by #521
Open

Upload files are not getting deleted. #519

varadekd opened this issue Jan 17, 2024 · 5 comments · May be fixed by #521

Comments

@varadekd
Copy link

varadekd commented Jan 17, 2024 •
edited
Loading

If an uploaded zip file fails during the scanning phase, we do not remove it from the server storage. This can result in unnecessary space occupation, potentially filling up our server storage.

To reproduce this issue:

  • In your code, ensure that the uploads folder is empty (for easy debugging).
  • Upload a corrupted zip file.
  • You will see an error message in the UI, or if you are checking programmatically.
  • Now, go back to your code and check the upload folder; you will observe the presence of a zip file.

Expectation: This file should be deleted in case of an error since we generate a unique name for the file every time it is uploaded.

Additional Information.

  1. I zipped the gscan code for testing this feature.
  2. Machine information
    1. Machine OS: Ubuntu 22.04 LTS
    2. Node version: v18.18.0
    3. Yarn version: 1.22.21
@varadekd
Copy link
Author

@JohnONolan Just wanted to confirm if this is a valid bug. If so I will fix it.

varadekd added a commit to varadekd/gscan that referenced this issue Jan 18, 2024
@varadekd
🐛 Addressed the issue where files in the uploads folder couldn't be d…
2e08ca6 
…eleted in case of an error.

Closes TryGhost#519

- In the scenario where a user uploaded corrupted files and attempted to scan them using gscan, an error occurred.
- Previously, in this scenario, we failed to delete the uploaded file stored locally under the uploads folder.
- Issue found: The identified bug was that we were only deleting the file when the promise was successfully resolved, and in the case of an error, we were simply moving to the next middleware.
- Issue fixed: To resolve this issue, a finally block has been added to ensure that uploaded files are deleted in any case.
@varadekd varadekd linked a pull request Jan 18, 2024 that will close this issue
Open
@varadekd
Copy link
Author

@JohnONolan Please check the PR created for this issue

@ErisDS
Copy link
Member

ErisDS commented Jan 22, 2024

Hey @varadekd, thanks for the issue and PR. Would you be able to create a test to demonstrate the issue, and how the patch resolves it? Thanks!

@varadekd
Copy link
Author

@ErisDS I can create a test; I just want to make sure that when you say 'create a test to demonstrate the issue,' does this refer to a prefix, a postfix, or both?

My understanding is that it involves a prefix, where we can keep track of exactly what the issue is and how this patch is resolving it.

@varadekd
Copy link
Author

@ErisDS Please let me know if my understanding with respect to the requirement is correct or not

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@ErisDS @varadekd