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

M3u add parsing of forwarded header wip #716

Open
wants to merge 4 commits into
base: next
Choose a base branch
from

Conversation

ArneBab
Copy link
Contributor

@ArneBab ArneBab commented Sep 2, 2020

only merge after m3u is merged!

WIP, because parsing that header directly needs somewhat careful checking.

@Bombe
Copy link
Contributor

Bombe commented Nov 29, 2020

only merge after m3u is merged!

I guess this could now use a merge or a rebase, whichever you prefer. :)

@nextgens
Copy link
Contributor

nextgens commented Apr 6, 2021

Please don't mix formatting changes with code changes... it makes reviewing PRs such as this one very difficult

@ArneBab ArneBab force-pushed the m3u-add-parsing-of-forwarded-header-wip branch 2 times, most recently from f346f45 to 6ee70b1 Compare May 22, 2022 08:19
@ArneBab
Copy link
Contributor Author

ArneBab commented May 22, 2022

First step: rebased and whitespace and imports cleaned up.

@ArneBab ArneBab force-pushed the m3u-add-parsing-of-forwarded-header-wip branch 6 times, most recently from 0d4814d to 92c4424 Compare May 22, 2022 08:33
@ArneBab
Copy link
Contributor Author

ArneBab commented May 22, 2022

Second step: more cleanup and some force pushing, because it should now be much easier to check.

@ArneBab ArneBab force-pushed the m3u-add-parsing-of-forwarded-header-wip branch 4 times, most recently from eecfda3 to cbfac8a Compare May 22, 2022 08:45
@ArneBab ArneBab force-pushed the m3u-add-parsing-of-forwarded-header-wip branch from cbfac8a to 6d51184 Compare October 14, 2023 10:45
@ArneBab
Copy link
Contributor Author

ArneBab commented Oct 14, 2023

@Bombe I added tests and fixed the case sensitivity (keys must be insensitive). But this isn’t tested in production, yet.

@Bombe
Copy link
Contributor

Bombe commented Oct 14, 2023

Hmm, according to RFC 7239 there can be multiple Forwarded headers in a request but I can’t spot any handling of that…

@hernic
Copy link
Contributor

hernic commented May 6, 2024

@Bombe @ArneBab some news about his PR ? still relevant ?

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