Support custom HTTP header for IP address source #215
Conversation
|
@cevantes Gentle ping. |
|
This works in our app now, but I am thinking maybe the new constructor should take an ordered list of custom headers, so that if one is missing it falls back to the next one, what do you think? Also my problem could be mitigated by altering nginx settings or Middleware but that's out of scope for this PR, just saying for posterity. |
|
Sorry for the delay. |
cevantes
force-pushed
the
feature/custom-source-header
branch
from
50b48a4 to
079f294
Compare
|
Sure, here we go |
|
The code is OK. |
cevantes
force-pushed
the
feature/custom-source-header
branch
from
079f294 to
7c772de
Compare
|
Sure, updated |
|
Merged. |
| -- | From a custom HTTP header, useful in proxied environment. | ||
| | FromHeaderCustom [HeaderName] |
There was a problem hiding this comment.
Since IPAddrSource is exposed to the user, I believe the addition of a data constructor should have caused a MAJOR version bump, at least according to the PVP.
So the new version should have become 2.5, not 2.4.1.
(Apologies for the noise if wai-logger is not actually following the PVP on this!)
There was a problem hiding this comment.
Ah I think you're right, while this is not a breaking change by intention, the compiler should complain about a missing pattern matching case, which could result in a build failure.
There was a problem hiding this comment.
Ah yeah, just to be clear, in point 1 in https://pvp.haskell.org/#version-numbers, it says:
Breaking change. If any entity was removed, or the types of any entities or the definitions of datatypes or classes were changed, or orphan instances were added or any instances were removed, then the new A.B MUST be greater than the previous A.B.
(emphasis mine)
Here, A.B. is referring to the MAJOR version.
There was a problem hiding this comment.
Right, it's a common pitfall and some library authors choose to hide their constructors and instead expose set functions, which is a bit annoying but helps to avoid a major version bump.
There was a problem hiding this comment.
So, should I deprecate 2.4.1 on Hackage and release it as 2.5?
There was a problem hiding this comment.
@kazu-yamamoto I believe that is correct.
|
I'm going to review and merge #218 and release v2.5. |
The case here is we use a proxy which replaces X-Real-IP with something not real and instead provides client's IP address via a custom header.