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

Fix: split XFF header only by comma #1878

Merged
merged 1 commit into from
Jun 2, 2021

Conversation

osavchenko
Copy link
Contributor

When splitting X-Forwarded-For header by ', ' (comma and space) I got an issue when valid value 127.0.0.1,127.0.1.1 can't be parsed correctly. I'm suggesting splitting just by , (comma) and trim a result.

@codecov
Copy link

codecov bot commented May 25, 2021

Codecov Report

Merging #1878 (2defe74) into master (2acb24a) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1878      +/-   ##
==========================================
+ Coverage   90.21%   90.22%   +0.01%     
==========================================
  Files          31       31              
  Lines        2770     2773       +3     
==========================================
+ Hits         2499     2502       +3     
  Misses        173      173              
  Partials       98       98              
Impacted Files Coverage Δ
context.go 87.19% <100.00%> (ø)
bind.go 89.41% <0.00%> (+0.19%) ⬆️

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 2acb24a...2defe74. Read the comment docs.

@aldas
Copy link
Contributor

aldas commented Jun 2, 2021

It seems that a lot of sites say that comma+space is used. But https://datatracker.ietf.org/doc/html/rfc7239#section-7.1 seems to say that

Note that an HTTP list allows white spaces to occur between the
identifiers, and the list may be split over multiple header fields.
As an example, the header field

so I'll merge this pr.

@aldas aldas merged commit fdacff0 into labstack:master Jun 2, 2021
@osavchenko osavchenko deleted the fix_forwarded_split branch June 3, 2021 06:26
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.

2 participants