Skip to content

Avoid roundtrip to Text when inspecting x-forwarded-for header#49

Merged
jezen merged 1 commit into
fpco:masterfrom
alexbiehl:master
Aug 11, 2025
Merged

Avoid roundtrip to Text when inspecting x-forwarded-for header#49
jezen merged 1 commit into
fpco:masterfrom
alexbiehl:master

Conversation

@alexbiehl

@alexbiehl alexbiehl commented Aug 8, 2025

Copy link
Copy Markdown
Contributor

The TE.decodeUtf8 fails with an error on invalid encoding which is the cause for snoyberg/keter#312. http-reverse-proxy needs to be more careful when looking at user inputs and leave the rest to users.

@ulidtko ulidtko left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Decent fix, LGTM

@jezen

jezen commented Aug 8, 2025

Copy link
Copy Markdown
Collaborator

I'm not a maintainer of this repository, but I think the next steps are:

  1. Bump the version in this package
  2. Add an entry to the CHANGELOG
  3. Merge
  4. Upload to Hackage
  5. Bump http-reverse-proxy dependency version in keter

@snoyberg Happy to help out here if you'll grant me the appropriate access rights.

@snoyberg

snoyberg commented Aug 8, 2025

Copy link
Copy Markdown
Member

Thanks @jezen, I've added you as a maintainer both here on GitHub and on Hackage.

@jezen

jezen commented Aug 8, 2025

Copy link
Copy Markdown
Collaborator

I suspect CI is failing because the resolvers we're building against are now too old. I remember we saw this failure for MacOS builds elsewhere.

Here's the offending line: https://github.com/fpco/http-reverse-proxy/blob/master/.github/workflows/tests.yml#L17

Is it worth continuing to rely on stack for CI here instead of just cabal-install? Not sure… 🤔 Here's an example of what that change might look like: snoyberg/keter@20a33d9

@alexbiehl @ulidtko What do you think?

@alexbiehl

alexbiehl commented Aug 8, 2025

Copy link
Copy Markdown
Contributor Author

Go for it. I prefer cabal these days as well. But don’t let that hold off merging this patch first — CIs that ran are all green.

@jezen jezen merged commit 2e8af72 into fpco:master Aug 11, 2025
5 of 10 checks passed
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.

4 participants