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

Upload limit is ignored #336

Open
tessus opened this issue Aug 18, 2024 · 10 comments
Open

Upload limit is ignored #336

tessus opened this issue Aug 18, 2024 · 10 comments
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@tessus
Copy link
Collaborator

tessus commented Aug 18, 2024

I have set max_content_length = "50MB", but whenever I upload a file larger than 50MB, the entire file is uploaded and only then I get the error message that the limit is exceeded.

rpaste

limit_rpaste.mp4

curl

limit_curl.mp4
@orhun orhun changed the title upload limit is ignored Upload limit is ignored Aug 20, 2024
@orhun
Copy link
Owner

orhun commented Aug 20, 2024

The middleware was implemented in #53 and yes, it turns out it is broken. I just reproduced this issue.

What would be the way of going forward with fixing this? Should we check HEAD requests, or do something in the middleware? Any ideas?

@tessus
Copy link
Collaborator Author

tessus commented Aug 20, 2024

What would be the way of going forward with fixing this?

At one point I read a comment (but I am not sure in which repo or issue/discussion) that there is now a native way with the new versions of actix and/or actix-multipart which should render the middleware obsolete.

I've been looking for that comment frantically, but unfortunately haven't found it yet.

@orhun
Copy link
Owner

orhun commented Aug 21, 2024

It would be the best way of moving forward if something like that exists. Custom implementations are not always infallible, like we saw in this case :D

@tessus
Copy link
Collaborator Author

tessus commented Aug 21, 2024

I think I found it: https://stackoverflow.com/a/78399675

@orhun
Copy link
Owner

orhun commented Aug 21, 2024

That's superb, can you try it out?

One thing to note that we probably won't be able to change this on the fly since it's configured while the App is constructed.

@tessus
Copy link
Collaborator Author

tessus commented Aug 21, 2024

Not sure when I get to it, but yes, I would like to.

@orhun orhun added bug Something isn't working help wanted Extra attention is needed labels Aug 22, 2024
@tessus
Copy link
Collaborator Author

tessus commented Sep 16, 2024

I tried it, but unfortunately no difference. Maybe there's an issue with the Byte arithmetic. I am quite puzzled by this.
When I get to it, I'll open a draft PR and you can have a look at my attempt to make it woek with the "new" way of doing it according to the stackoverflow comment. Maybe I fucked it up though...

@orhun
Copy link
Owner

orhun commented Sep 24, 2024

Yup, shoot me a draft whenever you can!

@tessus tessus mentioned this issue Sep 27, 2024
12 tasks
@tessus tessus mentioned this issue Oct 25, 2024
12 tasks
@tessus
Copy link
Collaborator Author

tessus commented Oct 25, 2024

This (#360) was supposed to work, but it doesn't. I am not sure how to solve this. I can't believe nobody ever got this to work.

@orhun
Copy link
Owner

orhun commented Oct 29, 2024

We both seem quite puzzled by this. Maybe a gentle ping to the actix master @robjtede would help us out a bit 😊

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants