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

proposal: net/http: maximum size and number of parts in ParseMultipartForm #68889

Open
jech opened this issue Aug 15, 2024 · 7 comments · May be fixed by #70933
Open

proposal: net/http: maximum size and number of parts in ParseMultipartForm #68889

jech opened this issue Aug 15, 2024 · 7 comments · May be fixed by #70933
Labels
Milestone

Comments

@jech
Copy link

jech commented Aug 15, 2024

Proposal Details

Please provide a way to limit the maximum size and number of parts when using http.ParseMultipartForm.

http.ParseMultipartForm is handy when the application knows that the files being uploaded are small; in that case, one does not need to go through the hassle of http.MultipartReader. However, there appears to be no way to cause http.ParseMultipartForm to reject the upload if the parts are larger than a given size (say, a megabyte), and no way to reject posts with more than a given number of parts (say, 10).

I propose the addition of the following function (name to be reconsidered):

func (r *Request) ParseMultipartFormLimited(maxMemory int64, maxPartSize int64, maxParts int) error

This is just like ParseMultipartForm, except that:

  • if maxPartSize is strictly larger than 0, then any of the parts is larger than maxPartSize bytes, the function returns http.ErrMessageTooLarge;
  • if maxParts is strictly larger than 0, then if there are more than maxParts parts the function returns http.ErrMessageTooLarge.

If the function returns http.ErrMesageTooLarge, then the body of the request has been closed.

@jech jech added the Proposal label Aug 15, 2024
@gopherbot gopherbot added this to the Proposal milestone Aug 15, 2024
@gabyhelp
Copy link

Related Issues and Documentation

(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)

@seankhliao
Copy link
Member

I believe the expectation is to wrap the handler in MaxBytesHandler

@ianlancetaylor
Copy link
Member

It helps us if you have a specific proposal. Thanks.

@ianlancetaylor ianlancetaylor moved this to Incoming in Proposals Aug 15, 2024
@jech
Copy link
Author

jech commented Aug 15, 2024

It helps us if you have a specific proposal.

I've done my best (edited).

@ianlancetaylor
Copy link
Member

CC @neild

@starius
Copy link
Contributor

starius commented Dec 19, 2024

Currently package mime/multipart has a hardcoded constant maxMIMEHeaderSize equal to 10 megabytes.

I propose to add a public field MaxMIMEHeaderSize to multipart.Reader.
Methods NextPart() and NextRawPart() should use the value of that field if it is not 0, otherwise they should use the constant.

To use this new field, one would call request.MultipartReader() method in http package, set the field to the desired value (e.g. 1024) and then parse the force using reader.NextPart() method without the risk that multipart headers eat 10 megabytes of RAM.

starius added a commit to starius/go that referenced this issue Dec 19, 2024
If the field is set, it is used instead of maxMIMEHeaderSize constant, allowing
to further constraint memory usage when parsing multipart streams.

Fix golang#68889
starius added a commit to starius/go that referenced this issue Dec 19, 2024
If the field is set, it is used instead of maxMIMEHeaderSize constant, allowing
to further constraint memory usage when parsing multipart streams.

Fixes golang#68889
@starius starius linked a pull request Dec 19, 2024 that will close this issue
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/637915 mentions this issue: mime/multipart: add field Reader.MaxMIMEHeaderSize

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Incoming
Development

Successfully merging a pull request may close this issue.

6 participants