Skip to content

Bind query params only for HTTP GET/DELETE methods - #1727

Merged
lammel merged 2 commits into
labstack:masterfrom
aldas:bind_query_when_get_delete
Dec 25, 2020
Merged

lammel merged 2 commits into
labstack:masterfrom
aldas:bind_query_when_get_delete

Conversation

@aldas

@aldas aldas commented Dec 20, 2020 •

Copy link
Copy Markdown
Contributor

fixes #1670 and continues #1681 (comment) proposed changes

DefaultBinder.Bind() binds now query params to struct only for GET and DELETE methods. For POST/PUT/PATCH query params are not considered anymore. This restores pre v4.1.11 behavior.

@codecov

codecov Bot commented Dec 20, 2020 •

Copy link
Copy Markdown

Codecov Report

Merging #1727 (65ea019) into master (936c48a) will increase coverage by 0.10%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1727      +/-   ##
==========================================
+ Coverage   85.77%   85.87%   +0.10%     
==========================================
  Files          29       29              
  Lines        2017     2018       +1     
==========================================
+ Hits         1730     1733       +3     
+ Misses        181      180       -1     
+ Partials      106      105       -1     
Impacted Files Coverage Δ
bind.go 91.30% <100.00%> (+1.30%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 936c48a...65ea019. Read the comment docs.

@aldas

aldas commented Dec 20, 2020

Copy link
Copy Markdown
Contributor Author

@lammel have you checked https://github.blog/2020-05-06-new-from-satellite-2020-github-codespaces-github-discussions-securing-code-in-private-repositories-and-more/#discussions maybe it is worth checking. Echo official forum is full of spam and I thinks similar thing in github would be much easier to find and use

@aldas aldas changed the title Bind query when get delete Bind query params only for HTTP GET/DELETE methods Dec 20, 2020
@lammel

lammel commented Dec 23, 2020

Copy link
Copy Markdown
Contributor

@lammel have you checked https://github.blog/2020-05-06-new-from-satellite-2020-github-codespaces-github-discussions-securing-code-in-private-repositories-and-more/#discussions maybe it is worth checking. Echo official forum is full of spam and I thinks similar thing in github would be much easier to find and use

Noted! This is being discussed already.

@lammel

lammel commented Dec 25, 2020

Copy link
Copy Markdown
Contributor

Nice work. The tests cover all scenarios I could think off, thanks @aldas

@lammel
lammel merged commit b065180 into labstack:master Dec 25, 2020
@aldas
aldas deleted the bind_query_when_get_delete branch May 23, 2021 06:06
@arvenil

arvenil commented Jul 16, 2021

Copy link
Copy Markdown

But why? :( What's wrong with query params in POST? :( Is there a way to force echo to parse query param for POST? Or I need to stay on older version?

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

Successfully merging this pull request may close these issues.

c.bind() binds both POST request body and query to given struct causing security vulnerability and problems with slices

4 participants