apache / trafficcontrol

Apache Traffic Control is an Open Source implementation of a Content Delivery Network
https://trafficcontrol.apache.org/
Apache License 2.0
1.03k stars 339 forks source link

Traffic Ops Golang Writes Body Twice for Gzipped Headers #1180

Closed rob05c closed 6 years ago

rob05c commented 6 years ago

traffic_ops_golang is broken if the client sents Accept-Encoding: gzip. It will write the gzipped body, followed by the un-gzipped body.

wrappers.go wrapHeaders calls gzipResponse with the real Writer, which calls w.Write. It then calls iw.RealWrite(iw.Body()) which writes to the real writer.

This was a result of merging https://github.com/apache/incubator-trafficcontrol/pull/1146 which I reviewed, and requested a fix for this issue. https://github.com/apache/incubator-trafficcontrol/pull/1146#discussion_r138130404

The code was changed, so the Review comment was hidden, without fixing the issue.

The Review was then Dismissed without Approval: https://github.com/apache/incubator-trafficcontrol/pull/1146#event-1251561294

The PR was then merged with this critical regression: https://github.com/apache/incubator-trafficcontrol/pull/1146#event-1251665312

Incidentally, the breaking commit lists me as the author: https://github.com/apache/incubator-trafficcontrol/commit/8f90a1fc4cbd6f143dad3acfa23c6fc9261f7114 I am not the author of that commit (it may be the result of a merge failure). My original correct commit is here: https://github.com/rob05c/incubator-trafficcontrol/blob/29cc755bcdb2139c9e2c90dced87d9fb1a84c4b7/traffic_ops/traffic_ops_golang/wrappers.go

dewrich commented 6 years ago

Fixed with https://github.com/apache/incubator-trafficcontrol/pull/1182