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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

馃悰 BodyParser does not save empty string to field #1084

Closed
sandro opened this issue Dec 21, 2020 · 6 comments
Closed

馃悰 BodyParser does not save empty string to field #1084

sandro opened this issue Dec 21, 2020 · 6 comments

Comments

@sandro
Copy link

sandro commented Dec 21, 2020

Fiber version
Fiber v2.1.0

Issue description
I'm trying to use BodyParser to save an empty string to a field in a struct, but the empty string will not overwrite the existing data in the field.

The bug disappears when I'm able to set the decoder's zeroEmpty to true, i.e. decoder.ZeroEmpty(true).

I wonder if there's a way to allow users to change the decoder when necessary. Perhaps by allowing the user to pass a custom decoder into the BodyParser function?

Code Sample

package main

import (
	"fmt"
	"log"
	"net/http"

	"github.com/gofiber/fiber/v2"
	"github.com/gofiber/fiber/v2/utils"
)

type Post struct {
	Title string `form:"title"`
	Body  string `form:"body"`
}


func main() {
	app := fiber.New()

        // curl -d "title=&body=new_body" localhost:3000/posts
	app.Post("/posts", func(c *fiber.Ctx) error {
		p := Post{Title: "Existing title", Body: "Existing Body"}
		c.BodyParser(&p)
		fmt.Printf("form value of title %q\n", c.FormValue("title"))
		fmt.Printf("form value of title %q\n", c.FormValue("body"))
		fmt.Printf("%+v\n", p)
		return c.SendStatus(http.StatusOK)
	})

        // curl -d "title=&body=new_body" localhost:3000/posts_use_empty_string
	app.Post("/posts_use_empty_string", func(c *fiber.Ctx) error {
		p := Post{Title: "Existing title", Body: "Existing Body"}
		var decoder = NewDecoder()
		decoder.IgnoreUnknownKeys(true)
		decoder.ZeroEmpty(true)
		decoder.SetAliasTag("form")
		data := make(map[string][]string)
		c.Context().PostArgs().VisitAll(func(key []byte, val []byte) {
			data[utils.UnsafeString(key)] = append(data[utils.UnsafeString(key)], utils.UnsafeString(val))
		})
		decoder.Decode(&p, data)
		fmt.Printf("form value of title %q\n", c.FormValue("title"))
		fmt.Printf("form value of title %q\n", c.FormValue("body"))
		fmt.Printf("%+v\n", p)
		return c.SendStatus(http.StatusOK)
	})

	log.Fatal(app.Listen(":3000"))
}

Sending an empty string to BodyParser does not overwrite the existing value

$ curl -d "title=&body=new_body" localhost:3000/posts

form value of title ""
form value of title "new_body"
{Title:Existing title Body:new_body}

But when sending an empty string when the decoder has ZeroEmpty set to true, the decoder does overwrite the existing value.

curl -d "title=&body=new_body" localhost:3000/posts_use_empty_string

form value of title ""
form value of title "new_body"
{Title: Body:new_body}
@sandro
Copy link
Author

sandro commented Dec 23, 2020

@Fenny Thanks for your consideration. I've added a code sample.

@si3nloong
Copy link
Contributor

Hi @sandro , is this issue still exists in the latest version?

@si3nloong
Copy link
Contributor

si3nloong commented Apr 17, 2021

If you use echo, you will realise they overwrite the default value if query string or form consists the key with empty value, i think we should enable ZeroEmpty(true) by default for decoder

Here is my pr, #1285

@sandro
Copy link
Author

sandro commented Apr 20, 2021

@si3nloong Thanks for the update. I haven't checked against the latest version, but I agree that setting ZeroEmpty to true by default makes sense. Unfortunately, that could break apps that depend on it being false. Is there any way to make ZeroEmpty configurable?

@si3nloong
Copy link
Contributor

si3nloong commented Apr 21, 2021

yes, it is, but I think the break will be minor. Since other famous frameworks also practicing the same way, we should also follow the same practice, to allow the user migrate their app from echo to fiber / fiber to echo seamlessly.

@Fenny
Copy link
Member

Fenny commented May 23, 2021

#1285 merged 馃憤

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants