Repository navigation
don't duplicate accept-encoding in vary header - #2796
Conversation
There was a problem hiding this comment.
Better done inside he response.vary() method in a generic way.
e2759c9 to
10f414e
Compare
|
You are right and much easier. Updated the PR. Added 1 more test and kept the old one since I thought it was still relevant. |
f45452b to
c3b7b67
Compare
There was a problem hiding this comment.
Why not just return here?
There was a problem hiding this comment.
I would have but there appears to be opportunities to return early in this function and it was not done. So, I was following the current flow and decided to break. Returning early does save an allocation and I am already doing split so I just changed it. Thanks!
c3b7b67 to
1901987
Compare
|
Any update on this? I would love to get it going so I can do a PR for lts and get h2o2 updated! Thank you :) |
don't duplicate accept-encoding in vary header
|
@nlf could this be included in LTS branch? h2o2 tests fail when upgrading to hapi 9.x.x otherwize. |
|
it will be. i'll release a new LTS after 10.2.0 is released |
|
This thread has been automatically locked due to inactivity. Please open a new issue for related bugs or questions following the new issue template instructions. |
This is in relation hapijs/h2o2#17 .
accept-encoding value in vary header was being duplicated. This fixes the issue and adds a test case to cover the scenario.
I could have fixed it instead in lib/response.js, to not add a value if it already exists in a header but I thought that was too brittle to do there.
PS: forgot to say, I will make a lts PR if this is accepted.