Simple change to remove hop-by-hop headers when relaying respon…

[rogersolsvik]

…se from upstream

Checklist

• Tests have been added (for bug fixes / features)
• Docs have been added / updated (for bug fixes / features)

What kind of change does this PR introduce?

Bug fix

What is the current behavior? What is the new behavior?

When running prism proxy it relays all headers from upstream source, even after adding new ones that might be incompatible with the original response.

More specifically in my case, my application is sending the Transfer-Encoding: chunked header, but since Prism is buffering the whole response to do response validation, is also adds the Content-Length header. These headers are incompatible and causes certain clients to fail. For example, Postman fails with a «Parse error» without a decent cause description.

Read more about the Transfer-Encoding header here

Does this PR introduce a breaking change?

Should be no breaking changes with this PR as far as I can see

[XVincentX]

Oh ok — interesting issue. The fix seems legit, however I would like to check it out deeply before I can merge and release this.

Thanks for the good investigation!

[rogersolsvik]

No problem @XVincentX. It might be appropriate make this fix for other "hop-by-hop" headers as well, but did not investigate which other might cause an issue too.
Other "hop-by-hop" headers are:

Connection
Keep-Alive
Public
Proxy-Authenticate
Transfer-Encoding
Upgrade

[XVincentX]

Yeah that is exactly my point — more investigation might be required to make this work properly in all the circumstances.

[rogersolsvik]

Good. Let me know if I can help. I you want to remove all hop-by-hop headers as a part of this pull request, I can make the appropriate updates.

[XVincentX]

Ok I've read a little bit around and yes, we should be removing these headers in case

1. The http-server is in Proxy mode
2. The hop-by-hop header is not being returned as part of the mocked response

Would you be willing to complete the PR with such changes and add the relative tests @rogersolsvik

[rogersolsvik]

A little premature push. Will be fixing test soon

[rogersolsvik]

I have not setup the harness tests locally. Any input on what's going wrong @XVincentX ?

[rogersolsvik]

Did an update from master now which triggered a rebuild. The harness tests seem to work now.

[XVincentX]

Thanks for checking this out — Ill review.

[XVincentX]

Some comments — check them out if you have time or I can make it over the finish line

[XVincentX]

@rogersolsvik Thanks for the follow up. I'll check it out again but I think this might be good to merge now. Thanks!

[XVincentX]

Thanks, @rogersolsvik