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(server): upload limit #360

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Conversation

tessus
Copy link
Collaborator

@tessus tessus commented Oct 25, 2024

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
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
Owner

orhun commented Dec 7, 2024

Hmm, what's the status with this?

@tessus
Copy link
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
Owner

orhun commented Dec 9, 2024

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

@tessus
Copy link
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