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

🤗 BodyParser (Refactor) #1067

Closed
moneszarrugh opened this issue Dec 10, 2020 · 1 comment
Closed

🤗 BodyParser (Refactor) #1067

moneszarrugh opened this issue Dec 10, 2020 · 1 comment

Comments

@moneszarrugh
Copy link
Contributor

moneszarrugh commented Dec 10, 2020

BodyParser uses a series of if-else statements with calls to strings.HasPrefix to match the content type

Any reason why it wan't implemented with a simple switch-case?

Also, the else part is useless since it's returning when a type is matched.

I also suggest defining a global fiber.Err type for body parser to better identify body parse errors.

fiber/ctx.go

Lines 247 to 282 in 9049720

// BodyParser binds the request body to a struct.
// It supports decoding the following content types based on the Content-Type header:
// application/json, application/xml, application/x-www-form-urlencoded, multipart/form-data
func (c *Ctx) BodyParser(out interface{}) error {
// Get decoder from pool
schemaDecoder := decoderPool.Get().(*schema.Decoder)
defer decoderPool.Put(schemaDecoder)
// Get content-type
ctype := getString(c.fasthttp.Request.Header.ContentType())
// Parse body accordingly
if strings.HasPrefix(ctype, MIMEApplicationJSON) {
schemaDecoder.SetAliasTag("json")
return json.Unmarshal(c.fasthttp.Request.Body(), out)
} else if strings.HasPrefix(ctype, MIMEApplicationForm) {
schemaDecoder.SetAliasTag("form")
data := make(map[string][]string)
c.fasthttp.PostArgs().VisitAll(func(key []byte, val []byte) {
data[getString(key)] = append(data[getString(key)], getString(val))
})
return schemaDecoder.Decode(out, data)
} else if strings.HasPrefix(ctype, MIMEMultipartForm) {
schemaDecoder.SetAliasTag("form")
data, err := c.fasthttp.MultipartForm()
if err != nil {
return err
}
return schemaDecoder.Decode(out, data.Value)
} else if strings.HasPrefix(ctype, MIMETextXML) || strings.HasPrefix(ctype, MIMEApplicationXML) {
schemaDecoder.SetAliasTag("xml")
return xml.Unmarshal(c.fasthttp.Request.Body(), out)
}
// No suitable content type found
return fmt.Errorf("bodyparser: cannot parse content-type: %v", ctype)
}

Fenny added a commit to Fenny/fiber that referenced this issue Dec 10, 2020
Co-Authored-By: Mones Zarrugh <11161902+moneszarrugh@users.noreply.github.com>

gofiber#1067
@Fenny
Copy link
Member

Fenny commented Dec 11, 2020

Refactored the code, and we now return a 422 error when an invalid content-type is present, see 400 vs 422 for more information.
Tagged in https://github.com/gofiber/fiber/releases/tag/v2.2.5, thanks for the report 👍

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

No branches or pull requests

2 participants