Skip to content
This repository has been archived by the owner on Mar 2, 2022. It is now read-only.

Fix Echo's Bind behaviour to use the more precise BindBody #43

Merged
merged 1 commit into from
Mar 24, 2021

Conversation

benmarsden
Copy link
Contributor

Previous versions of echo had an issue in which the Bind behaviour would be used generically for path params, query params and then body (in that order). What this means is that if you attempt to use echo's Context.Bind with a context that has either path or query params, it will use those, error early and never bind the JSON body. See labstack/echo#1356 for description.

This becomes an issue for us with our SaaS product, which makes use of an auth middleware that introduces path params to routes that otherwise do not have them.

Luckily, this has been fixed in at least v4.2.1 via either struct tagging or echo.DefaultBinder{}'s BindBody method. This PR simply bumps our dependency and introduces this use instead of the old Bind :)

Signed-off-by: Ben Marsden bmarsden10@gmail.com

Signed-off-by: Ben Marsden <bmarsden10@gmail.com>
@benmarsden benmarsden added the bug Something isn't working label Mar 24, 2021
@benmarsden benmarsden added this to the M5 (0.0.2-alpha) milestone Mar 24, 2021
@benmarsden benmarsden self-assigned this Mar 24, 2021
@benmarsden benmarsden added this to In progress in Bubbly via automation Mar 24, 2021
Copy link
Contributor

@nate-droid nate-droid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice upgrade!

Bubbly automation moved this from In progress to Reviewer approved Mar 24, 2021
Copy link
Contributor

@jlarfors jlarfors left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jlarfors jlarfors merged commit 8920efc into main Mar 24, 2021
Bubbly automation moved this from Reviewer approved to Done Mar 24, 2021
@olliefr olliefr deleted the fix/echo-bind-bug branch April 23, 2021 12:33
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants