Preamble fix
Needs RevisionPublic
Actions

Authored by theascone on May 22 2015, 6:50 PM.

Details

Reviewers

epriestley

Group Reviewers

Blessed Reviewers

Maniphest Tasks

T7114: $_SERVER stuff from the preamble is getting overwritten in PhabricatorStartup

Required Signatures

L28 Phacility Individual Contributor License Agreement

Summary

Ref T7114
Should make it possible to override values in $_SERVER using the preamble script.

Example (inside preamble.php):

<?php

$GLOBALS['PREAMBLE'] = array (
  'SERVER' => array (
    'REMOTE_ADDR' => $_SERVER['HTTP_X_FORWARDED_FOR'],
    'HTTPS' => true,
  ),
);

Test Plan

$_SERVER values are being overridden using this method.

Diff Detail

Repository

rP Phabricator

Lint

Lint Skipped

Unit

Tests Skipped

Build Status

Buildable 6216
Build 6237: [Placeholder Plan] Wait for 30 Seconds

Event Timeline

theascone updated this revision to Diff 31280.May 22 2015, 6:50 PM

theascone retitled this revision from to Preamble fix.

theascone updated this object.

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

theascone added a reviewer: epriestley.

theascone set the repository for this revision to rP Phabricator.

theascone added a task: T7114: $_SERVER stuff from the preamble is getting overwritten in PhabricatorStartup.

Herald added a reviewer: Blessed Reviewers. · View Herald TranscriptMay 22 2015, 6:50 PM

Herald added a required legal document: L28 Phacility Individual Contributor License Agreement. · View Herald Transcript

Herald added subscribers: epriestley, Korvin. · View Herald Transcript

I think we can just not filter $_SERVER instead. I believe there is no sensible setting for filter.default which causes problems with Phabricator without completely breaking all reasonable applications. The only values I can possibly see an issue with are PHP_AUTH_USER and PHP_AUTH_PW, which could interact poorly with settings like "magic_quotes", "string" or "stripped".

Let's try not resetting $_SERVER (that is, basically remove the INPUT_SERVER case from this function) and see if anyone runs into issues? If it does, we could selectively decline to filter values which make sense to override in the preamble (REMOTE_ADDR, HTTPS) since they are not sensitive to remotely reasonable input filters anyway.

This revision now requires changes to proceed.May 25 2015, 2:57 PM

epriestley mentioned this in D13152: Remove PhabricatorStartup::getGlobal/setGlobal mechanism.Jun 4 2015, 6:59 PM

epriestley mentioned this in rPa15444aa7919: Remove PhabricatorStartup::getGlobal/setGlobal mechanism.Jun 5 2015, 12:27 AM