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

feat: support max_upload_dir_size config #335

Merged

Conversation

Narayanbhat166
Copy link
Contributor

@Narayanbhat166 Narayanbhat166 commented Aug 18, 2024

Description

This Pull Request adds a feature to limit the uploads if the ./uploads dir is full. The size of ./uploads dir can be set by the config key max_upload_dir_size under the server config.

[server]
max_upload_dir_size = "100G"

Motivation and Context

This is required to restrict the number / size of uploads that can be made to the server when hosted publicly.
This fixes #282

How Has This Been Tested?

Changelog Entry

  • Add a check for the max size of uploads directory

Types of Changes

  • New feature (non-breaking change which adds functionality)

Checklist:

  • My code follows the code style of this project.
  • I have updated the documentation accordingly.
  • I have formatted the code with rustfmt.
  • I checked the lints with clippy.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

src/config.rs Outdated Show resolved Hide resolved
Copy link
Owner

@orhun orhun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Had some comments:

Cargo.toml Outdated Show resolved Hide resolved
fixtures/test-server-upload-dir-limit/test.sh Show resolved Hide resolved
@orhun
Copy link
Owner

orhun commented Sep 20, 2024

Let's get this over the finish line soon if possible :)

@orhun
Copy link
Owner

orhun commented Dec 7, 2024

Ping :)

@tessus
Copy link
Collaborator

tessus commented Dec 9, 2024

@Narayanbhat166 can you have a look and make the requested changes?

@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 6.66667% with 28 lines in your changes missing coverage. Please review.

Project coverage is 80.51%. Comparing base (1d5a9c6) to head (aad5f77).
Report is 29 commits behind head on master.

Files with missing lines Patch % Lines
src/util.rs 0.00% 17 Missing ⚠️
src/paste.rs 15.38% 11 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #335      +/-   ##
==========================================
- Coverage   83.27%   80.51%   -2.76%     
==========================================
  Files          11       11              
  Lines        1214     1273      +59     
==========================================
+ Hits         1011     1025      +14     
- Misses        203      248      +45     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@orhun orhun merged commit c5f7377 into orhun:master Dec 11, 2024
8 checks passed
@tessus
Copy link
Collaborator

tessus commented Dec 11, 2024

@orhun I was just wondering what kind of perf impact this has. Isn't this similar to the duplicates=false issue? Every time there is an upload, all files have to be touched. Yes, getting the size of all files is probably a lot faster than calculating the hash for each file, but still. Patterns like this make me nervous, coming from a perf background.

@orhun
Copy link
Owner

orhun commented Dec 12, 2024

Yeah, probably the performance will be degrade in a noticeable way with this feature enabled.

I think we need to add benchmarks using hyperfine to get an idea about that. I did something similar for Zig in https://github.com/orhun/zig-http-benchmarks

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.

Support a total size limit for the upload directory
4 participants