Skip to content

http: removing whitespace from appended XFF headers (take 2)#3828

Closed
alyssawilk wants to merge 1 commit into
envoyproxy:masterfrom
alyssawilk:xff
Closed

http: removing whitespace from appended XFF headers (take 2)#3828
alyssawilk wants to merge 1 commit into
envoyproxy:masterfrom
alyssawilk:xff

Conversation

@alyssawilk
Copy link
Copy Markdown
Contributor

@alyssawilk alyssawilk commented Jul 10, 2018

Reinstating terse XFF appends, now that #3610 has been checked in for a month.

Risk Level: High
Testing: Adapted tests
Docs Changes: n/a
Release Notes: yes

Fixes #3611

…nvoyproxy#3587)" (envoyproxy#3609)"

This reverts commit 75ca181 with minor
docs edits.

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
* http: better handling of HEAD requests. Now sending transfer-encoding: chunked rather than content-length: 0.
* http: no longer adding whitespace when appending X-Forwarded-For headers. **Warning**: this is not
compatible with 1.7.0 builds prior to `9d3a4eb4ac44be9f0651fcc7f87ad98c538b01ee <https://github.com/envoyproxy/envoy/pull/3610>`_.
See `#3611 <https://github.com/envoyproxy/envoy/issues/3611>`_ for details.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Hm, on second thought, I wonder if we should wait one more release cycle on this for good form. If everyone else is running from HEAD or close to it, I think we're fine. If anyone is running on release builds, it'll make rolling from 1.7 to 1.8 correctly a bit tricky which arguably it shouldn't be.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think I agree (although we run from HEAD), sounds like two release should avoid surprises....

@alyssawilk alyssawilk closed this Jul 10, 2018
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