Q: What’s wrong with the code below (this is very typical way of parsing a multipart request):
// in the body of a http.HandlerFunc
err := r.ParseMultipartForm(32 << 20) // 32Mb
if err != nil {
http.Error(w, err.Error(), http.StatusBadRequest)
}
A: Nothing wrong with the code above, it’s a hidden gotcha which is very easy to be oversight though.
ParseMultipartForm
will parse every single of part of request, if you send 100 files in a “chunked” encoding POST to the above endpoint, it will parse them all. Notice that 32Mb is the bytes allocated to the request body to store in memory, not the limit of request body, when full (says 33Mb), it will write to temporary directory.
A simple triage:
// add this line on top
r.Body = http.MaxBytesReader(w, r.Body, 32<<20+512)
...
Above will limit the POST body size to 32.5Mb and throw error if client attempt to send more than that. At least, we don’t have to worry if client maliciously draining server resource.
But what if we want the full control of data coming in? E.g.: file size limit or file type on specific field. With the standard ParseMultipartForm
, we probably can’t achieve that.
These are the things I want to have in my handler:
- file type validation
- file size validation
- whitelist “field” names (so we don’t write down any file we don’t want to)
- parse fields in order or terminate otherwise
- terminate early if any validation fails
- buffering
So this is what I come up with, for demonstration purpose, POST will have 2 fields named: text_field
and file_field
, bear with me for below extremely verbose code:
// function body of a http.HandlerFunc
r.Body = http.MaxBytesReader(w, r.Body, 32<<20+1024)
reader, err := r.MultipartReader()
if err != nil {
http.Error(w, err.Error(), http.StatusBadRequest)
return
}// parse text field
text := make([]byte, 512)
p, err := reader.NextPart()
// one more field to parse, EOF is considered as failure here
if err != nil {
http.Error(w, err.Error(), http.StatusInternalServerError)
return
}if p.FormName() != "text_field" {
http.Error(w, "text_field is expected", http.StatusBadRequest)
return
}_, err = p.Read(text)
if err != nil && err != io.EOF {
http.Error(w, err.Error(), http.StatusInternalServerError)
return
}// parse file field
p, err = reader.NextPart()
if err != nil && err != io.EOF {
http.Error(w, err.Error(), http.StatusInternalServerError)
return
}if p.FormName() != "file_field" {
http.Error(w, "file_field is expected", http.StatusBadRequest)
return
}buf := bufio.NewReader(p)
sniff, _ := buf.Peek(512)
contentType := http.DetectContentType(sniff)
if contentType != "application/zip" {
http.Error(w, "file type not allowed", http.StatusBadRequest)
return
}f, err := ioutil.TempFile("", "")
if err != nil {
http.Error(w, err.Error(), http.StatusInternalServerError)
return
}
defer f.Close()var maxSize int64 = 32 << 20
lmt := io.MultiReader(buf, io.LimitReader(p, maxSize - 511))
written, err := io.Copy(f, lmt)
if err != nil && err != io.EOF {
http.Error(w, err.Error(), http.StatusInternalServerError)
return
}
if written > maxSize {
os.Remove(f.Name())
http.Error(w, "file size over limit", http.StatusBadRequest)
return
}// schedule for other stuffs (s3, scanning, etc.)
Some key points for the code above:
- Only fetch first 2 parts in POST body, handler will expect
text_field
thenfile_field
- Assert file type using
bufio.Reader
wrapping Part reader (peeking first 512 bytes) - Limit file size with
io.LimitReader
(with 1 byte offset to see if part reader still has some data left)
The code can be definitely refactored to treat file and text fields in general but you got the idea, let me know what you think in the comment section.
Happy coding gophers!