Skip to content

fix(server): upload limit#360

Draft
tessus wants to merge 1 commit intoorhun:masterfrom
tessus:fix/upload-limit
Draft

fix(server): upload limit#360
tessus wants to merge 1 commit intoorhun:masterfrom
tessus:fix/upload-limit

Conversation

@tessus
Copy link
Copy Markdown
Collaborator

@tessus tessus commented Oct 25, 2024

Caution

This is PR does not work!
It implements the changes suggested on SO to limit the upload file size.
It is not clear whether it is really possible with actix.

Description

Update code to limit the file upload.

Motivation and Context

see #336
and https://stackoverflow.com/a/78399675

How Has This Been Tested?

It does not seem to work.

Changelog Entry

Types of Changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation (no code change)
  • Refactor (refactoring production code)
  • Other

Checklist:

Will be done after moving it out of draft status.

  • 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.

@tessus
Copy link
Copy Markdown
Collaborator Author

tessus commented Oct 25, 2024

I didn't adjust the tests, which is why the pipeline complains.

But I was testing manually and it doesn't work.

@orhun
Copy link
Copy Markdown
Owner

orhun commented Dec 7, 2024

Hmm, what's the status with this?

@tessus
Copy link
Copy Markdown
Collaborator Author

tessus commented Dec 7, 2024

This PR implements the limit as was shown at stackoverflow. You can build the app and test it. But it doesn't work. Same behavior as before, except that now you don't even get an error message after the file has been uploaded. Apparently limiting the fie upload is just not possible with this framework or every single information out there about how one can accomplish this is wrong.

You pinged the author here #336 (comment) but nothing so far.

@orhun
Copy link
Copy Markdown
Owner

orhun commented Dec 9, 2024

I see. I might come back to this later then.

@tessus
Copy link
Copy Markdown
Collaborator Author

tessus commented Dec 9, 2024

Yea, this one is a draft and was just to show that it doesn't work as described in the stackoverflow article.
We can leave it for reference for now. Who knows maybe Rob will check it out... ;-)

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.

2 participants