Page MenuHomePhabricator

Allow Phabricator to run with "enable_post_data_reading" disabled
ClosedPublic

Authored by epriestley on Oct 10 2017, 10:26 PM.
Tags
None
Referenced Files
F14057209: D18702.diff
Sun, Nov 17, 1:24 AM
F14011910: D18702.diff
Fri, Nov 1, 5:45 AM
F13980239: D18702.id.diff
Sat, Oct 19, 9:13 AM
F13961036: D18702.diff
Oct 15 2024, 2:45 AM
Unknown Object (File)
Oct 11 2024, 2:44 AM
Unknown Object (File)
Sep 10 2024, 12:49 PM
Unknown Object (File)
Sep 6 2024, 1:22 AM
Unknown Object (File)
Sep 6 2024, 1:22 AM
Subscribers
None

Details

Summary

Ref T13008. Depends on D18701. The overall goal here is to make turning enable_post_data_reading off not break things, so we can run rate limiting checks before we read file uploads.

The biggest blocker for this is that turning it off stops $_FILES from coming into existence.

This appears to mostly work. Specifically:

  • Skip the max_post_size check when POST is off, since it's meaningless.
  • Don't read or scrub $_POST at startup when POST is off.
  • When we rebuild REQUEST and POST before processing requests, do multipart parsing if we need to and rebuild FILES.
  • Skip the is_uploaded_file() check if we built FILES ourselves.

This probably breaks a couple of small things, like maybe __profile__ and other DarkConsole triggers over POST, and probably some other weird stuff. The parsers may also need more work than they've received so far.

I also need to verify that this actually works (i.e., lets us run code without reading the request body) but I'll include that in the change where I update the actual rate limiting.

Test Plan
  • Disabled enable_post_data_reading.
  • Uploaded a file with a vanilla upload form (project profile image).
  • Uploaded a file with drag and drop.
  • Used DarkConsole.
  • Submitted comments.
  • Created a task.
  • Browsed around.

Diff Detail

Repository
rP Phabricator
Branch
post4
Lint
Lint Errors
SeverityLocationCodeMessage
Errorsupport/PhabricatorStartup.php:795XHP45PHP Compatibility
Errorsupport/PhabricatorStartup.php:811XHP45PHP Compatibility
Errorsupport/PhabricatorStartup.php:901XHP45PHP Compatibility
Errorsupport/PhabricatorStartup.php:910XHP45PHP Compatibility
Errorsupport/PhabricatorStartup.php:924XHP45PHP Compatibility
Errorsupport/PhabricatorStartup.php:925XHP45PHP Compatibility
Errorsupport/PhabricatorStartup.php:937XHP45PHP Compatibility
Unit
Tests Passed
Build Status
Buildable 18682
Build 25169: Run Core Tests
Build 25168: arc lint + arc unit

Event Timeline

amckinley added inline comments.
src/aphront/configuration/AphrontDefaultApplicationConfiguration.php
17–18

This comment needs to be updated, no?

This revision is now accepted and ready to land.Oct 13 2017, 7:43 PM
  • Rewrite explanatory comment.
This revision was automatically updated to reflect the committed changes.