Don’t parse everything from client multipart POST (Golang)

This post is about handling file upload from server side, if you are also looking to send file in multipart with minimal memory from client side, please see this post.

“I’m not a hoarder”

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 then file_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!