Paths

Table of Contentst

Differential D17045

Don't combine automatic output compression with "Content-Length"
ClosedPublic
Actions

Authored by epriestley on Dec 13 2016, 10:07 PM.

Tags

None

Referenced Files

	Unknown Object (File)
	Mon, Dec 16, 6:20 PM

	Unknown Object (File)
	Sat, Dec 14, 5:53 PM

	Unknown Object (File)
	Mon, Dec 9, 6:12 AM

	Unknown Object (File)
	Tue, Dec 3, 8:55 PM

	Unknown Object (File)
	Nov 17 2024, 7:37 PM

	Unknown Object (File)
	Nov 16 2024, 8:49 PM

	Unknown Object (File)
	Nov 13 2024, 5:33 PM

	Unknown Object (File)
	Nov 12 2024, 10:34 PM

Subscribers

Details

Reviewers

avivey
chad

Maniphest Tasks

T12013: Enabling zlib.output_compression results in wrong Content-Length

Commits

rP1cd64f9975d6: (stable) Don't combine automatic output compression with "Content-Length"
rP842710608e49: Don't combine automatic output compression with "Content-Length"

Summary

Fixes T12013. Send either "Content-Length" or enable output compression, but not both.

Prefer compression for static resources (CSS, JS, etc).

Test Plan

Ran curl -v ..., no longer saw responses with both compression and Content-Length.

Diff Detail

Repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

epriestley updated this revision to Diff 40997.Dec 13 2016, 10:07 PM

epriestley retitled this revision from to Don't combine automatic output compression with "Content-Length".

epriestley updated this object.

epriestley edited the test plan for this revision. (Show Details)

epriestley added a reviewer: chad.

epriestley added a task: T12013: Enabling zlib.output_compression results in wrong Content-Length.

epriestley mentioned this in T12013: Enabling zlib.output_compression results in wrong Content-Length.Dec 13 2016, 10:10 PM

avivey added a subscriber: avivey.Dec 13 2016, 10:10 PM

avivey added inline comments.

src/aphront/response/AphrontFileResponse.php
134–136	maybe also enable the compression on `else`?

Yeahhhhh

src/aphront/response/AphrontFileResponse.php
134–136	It's enabled globally in `PhabricatorStartup` and I shudder to think what PHP might do if we enable it twice. It would, perhaps, be cleaner to bring the `CompressResponse` stuff up a level to `AphrontResponse`, and have that control `zlib.output_compression` in all cases, although when we fatal the "¯\_(ツ)_/¯" page won't get compressed.

Centralize compression logic in one place, in AphrontResponse.

Yeah, that looks like it will probably not PHP in new ways?

("PHP" in that sentence is short for "break in interesting ways").

This revision is now accepted and ready to land.Dec 13 2016, 10:23 PM

Hahaha. Yeah, this feels a little more solid.

Closed by commit rP842710608e49: Don't combine automatic output compression with "Content-Length" (authored by epriestley, committed by epriestley). · Explain WhyDec 13 2016, 10:25 PM

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Path

Size

src/

aphront/

response/

AphrontFileResponse.php

18 lines

AphrontResponse.php

15 lines

sink/

AphrontHTTPSink.php

2 lines

applications/

celerity/

controller/

CelerityResourceController.php

7 lines

support/

PhabricatorStartup.php

5 lines

Diff 41000

src/aphront/response/AphrontFileResponse.php

Loading...

src/aphront/response/AphrontResponse.php

Loading...

src/aphront/sink/AphrontHTTPSink.php

Loading...

src/applications/celerity/controller/CelerityResourceController.php

Loading...

support/PhabricatorStartup.php

Loading...