Summary:
See <https://github.com/facebook/phabricator/issues/487>. If an exception is thrown too high in the stack for the main exception handling to deal with it, we currently never report a stack trace. Instead:
- Always report a stack trace to the error log.
- With developer mode, also report a stack trace to the screen.
Test Plan: Added a high-level `throw` and hit both cases. Got traces in the log and traces-under-developer-mode on screen.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D8022
Summary: Ref T3684 for discussion. This could be cleaned up a bit (it would be nice to draw entropy once per request, for instance, and maybe respect CSRF_TOKEN_LENGTH more closely) but should effectively mitigate BREACH.
Test Plan: Submitted forms; submitted forms after mucking with CSRF and observed CSRF error. Verified that source now has "B@..." tokens.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3684
Differential Revision: https://secure.phabricator.com/D6686
Summary: Ensures that weird behavior from filter_input_array does not remove keys from superglobals. Should fix T3677.
Test Plan:
Checked that $_SERVER contained same number of keys before and after
filtering, and that those affected by the original bug continue to be filtered
correctly.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: zorfling, aran, Korvin, wez
Maniphest Tasks: T3677
Differential Revision: https://secure.phabricator.com/D6680
Summary:
Fixes issue where double-encoding of $_SERVER occurs when php.ini forces all input to be sanitized
Ex:
filter.default = full_special_chars
filter.default_flags = 36
Fix line length
Test Plan: Encountered issue on clean install when registring new user (phusr not defined for email verification). php.ini on that server contains above filter settings. nginx/php-fpm with recommended settings for that server block from setup guide.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, epriestley
Differential Revision: https://secure.phabricator.com/D6672
Summary:
These versions are broken, but package distros seem to be picking them up. :/
Since the error you get is completely useless, fatal immediately with a useful message.
Ref T2594.
Test Plan: Faked verisions and hit the issue.
Reviewers: btrahan
Reviewed By: btrahan
CC: brennantaylor, Arijit, aran
Maniphest Tasks: T2594
Differential Revision: https://secure.phabricator.com/D6415
Summary:
D6278 kind of got closed and commited, this is the actual direction.
Ref T3432
Depends on D6277
Test Plan: Keep using the site
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin, mbishopim3
Maniphest Tasks: T3432
Differential Revision: https://secure.phabricator.com/D6283
Summary: Fixes T2965, see that task for discussion. This is dumb but seems like our best bet.
Test Plan:
- Installed newish version of Git.
- Set HOME on the websever to `/var/root` (or any other unreadable directory).
- Hit the error described in T2965 when viewing Diffusion.
- Applied this patch.
- Diffusion works.
Reviewers: btrahan, joel
Reviewed By: btrahan
CC: aran, chad
Maniphest Tasks: T2965
Differential Revision: https://secure.phabricator.com/D5994
Summary: If `jsxmin` is not available, use a pure PHP implementation instead (JsShrink).
Test Plan:
- Ran `arc lint --lintall` on all JS and fixed every relevant warning.
- Forced minification on and browsed around the site using JS behaviors. Didn't hit anything problematic.
Reviewers: vrana, btrahan
Reviewed By: vrana
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5670
Summary: See D5874. PhutilDeferredLog's exception on `write()`/`__destruct()` is not especially important and can come at an awkward time. Instead of throwing, just emit an error message to the log.
Test Plan: Faked failed writes and saw an error message in the log instead of many terrible things everywhere.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3144
Differential Revision: https://secure.phabricator.com/D5875
Summary: At some point, Node started requiring us to read data before we can get the 'end' event, it seems. Fixes T2953.
Test Plan: Ran server in `--foreground` mode on node v0.11.0, made request to `/status/`, got response.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2953
Differential Revision: https://secure.phabricator.com/D5666
Summary: Fixes T2930. We may not have an access log here yet -- I got a little too overeager with D5533.
Test Plan:
Faked gpc quotes and got a nice error instead of a blank screen:
{F39821}
Reviewers: btrahan, chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T2930
Differential Revision: https://secure.phabricator.com/D5630
Summary:
See D5561. Ref T2378.
- Add `support/bin/*` to .gitignore so any symlinks or binaries won't get picked up by Git.
- Add a README so Git preserves the directory and there's at least //some// documentation of its existence.
Test Plan: ummmmm
Reviewers: jevripio, codeblock, btrahan
Reviewed By: jevripio
CC: aran
Maniphest Tasks: T2378
Differential Revision: https://secure.phabricator.com/D5562
Summary:
We have a fair number of conditionals on the existence of the access log. Instead, always build it and just don't write it if the user doesn't want a version on disk.
Also, formalize logged-in user PHID (avoids object existence juggling) in the access log and move microseconds-since-startup to PhabricatorStartup (simplifies index.php).
Depends on D5532. Fixes T2860. Ref T2870.
Test Plan: Disabled access log, verified XHProf writes occurred correctly.
Reviewers: btrahan, chad
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2860, T2870
Differential Revision: https://secure.phabricator.com/D5533
Summary:
The fixed-position side nav background thing tends to make looking at print_r() output hard. Also, it breaks Ajax, etc.
- Loudly call out unexpected output on normal pages, to catch extra spaces before `<?php`, etc.
- Display unexpected output in an attractive panel on normal pages.
- Log unexpected output instead of breaking Ajax.
Test Plan:
{F32267}
Also triggered various fatals and verified they still show the right messages (no blank pages).
Reviewers: vrana, btrahan, chad
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D4892
Summary: This particular misconfiguration results in a difficult-to-debug redirect loop, so stop it early.
Test Plan: Broke my rewrite rule, verified I got yelled at.
Reviewers: btrahan, chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D4591
Summary:
We have a bunch of code duplication now between __init_script__.php and webroot/index.php. Consoldiate these methods and move them into PhabricatorEnv.
Merge PhabricatorRequestOverseer into PhabricatorStartup.
Test Plan: Loaded page, ran script. Wiped PHABRICATOR_ENV; loaded page, ran script; got errors.
Reviewers: btrahan, vrana
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2223
Differential Revision: https://secure.phabricator.com/D4283
Summary:
We have a lot of mess to get through before we can load libphutil and enter Phabricator code properly. Move it to a dedicated class.
I'm probably going to merge PhabricatorRequestOverseer into this, although the check that lives there now is kind of weird. It also does not really need to be a pre-load check and could be handled better.
I stopped shoving stuff in here once I got to ENV stuff, I'm going to tackle that next.
Test Plan: Ran phabricator normally; introduced fatals and misconfigurations. Grepped for changed symbols.
Reviewers: btrahan, vrana
Reviewed By: btrahan
CC: aran, asherkin
Maniphest Tasks: T2223
Differential Revision: https://secure.phabricator.com/D4282
Summary: We allow user to specify the host to listen to but we listen on 127.0.0.1 instead.
Test Plan:
Hardcoded the path and verified that I can connect to the host from other machine.
Verified that `localhost` still doesn't allow remote connections.
Reviewers: ddfisher, epriestley, nh
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4238
Summary:
This commit doesn't change license of any file. It just makes the license implicit (inherited from LICENSE file in the root directory).
We are removing the headers for these reasons:
- It wastes space in editors, less code is visible in editor upon opening a file.
- It brings noise to diff of the first change of any file every year.
- It confuses Git file copy detection when creating small files.
- We don't have an explicit license header in other files (JS, CSS, images, documentation).
- Using license header in every file is not obligatory: http://www.apache.org/dev/apply-license.html#new.
This change is approved by Alma Chao (Lead Open Source and IP Counsel at Facebook).
Test Plan: Verified that the license survived only in LICENSE file and that it didn't modify externals.
Reviewers: epriestley, davidrecordon
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2035
Differential Revision: https://secure.phabricator.com/D3886
Summary:
Lower the barrier to entry for installing and creating skins, so we can kill Wordpress. You can now install skins by dropping them into a directory, and build either "advanced" (full phutil library) skins or "basic" (simple PHP templates) skins.
Next up is getting static resources working in an easy way for skins.
I put these in `externals/` for now so they don't get hit by lint.
Test Plan: Viewed the Pokeblog with the Oblivious skin.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1373
Differential Revision: https://secure.phabricator.com/D3717
Summary: Depends on D2926. Adds a simple CLI client for Aphlict to make it easier to debug stuff.
Test Plan: Ran client, saw debug messages.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D2927
Summary: Add a `notification.debug` setting that shows debug info in the browser. Also improve some logging/error handling stuff and fix a bug with host names.
Test Plan: {F13098}
Reviewers: jungejason, btrahan, vrana
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T944
Differential Revision: https://secure.phabricator.com/D2810
Summary:
- Move to port 22280 by default.
- Warn when running as non-root.
- Allow subscription and publish/admin ports to be configured.
- Allow server to drop root after binding to 843.
- Allow log path to be configured.
- Add /status/ admin URI which shows server status.
- Return HTTP 400 Bad Request for other requests, instead of hanging.
- Minor formatting cleanup.
Test Plan:
Ran without root:
$ node aphlict_server.js
...got a good error message. Ran with --user:
$ sudo node aphlict_server.js --user=epriestley
...verified server dropped permissions. Ran with --port / --admin. Hit /status/ with GET, got status. Hit other URLs with GET, got 400.
Reviewers: allenjohnashton, ddfisher, keebuhm
Reviewed By: ddfisher
CC: aran
Differential Revision: https://secure.phabricator.com/D2737
Summary:
Adds the node.js Aphlict server, the flash Aphlict client, and some
supporting javascript. Built on top of - and requires - D2703 (which is still
in progress). Will likely work with no modification on top of the final
version, though.
The node server is currently run with
sudo node support/aphlict/server/aphlict_server.js
Test Plan: tested locally
Reviewers: epriestley
Reviewed By: epriestley
CC: allenjohnashton, keebuhm, aran, Korvin
Differential Revision: https://secure.phabricator.com/D2704
Summary: Provide a reasonable JS API for the Aphlict client. Provide an example behavior to invoke it.
Test Plan:
Ran "aphlict_server.js" with:
$ sudo node aphlict_server.js
Loaded /aphlict/. Opened console. Got "hello" from the server every second.
Got reasonable errors with the server not present ("Security exception", but this is because it can't connect to port 843 to access the policy server).
Reviewers: ddfisher, keebuhm, allenjohnashton, btrahan
Reviewed By: btrahan
CC: aran, epriestley
Maniphest Tasks: T944
Differential Revision: https://secure.phabricator.com/D1800
Summary:
This is purely a prototype at the moment, but the basic functionality sort of
works.
I'm not sure how far I want to go with this but I think we might be able to get
somewhere without it being gross.
The idea here is to build a notification server WITHOUT using Comet, since Comet
is extremely difficult and complicated.
Instead, I use Flash on the client. LocalConnection allows flash instances to
talk to each other and connect() can be used as a locking primitive. This allows
all the instances to elect a master instance in a race-safe way. The master is
responsible for opening a single connnection to the server.
On the server, I use Node.js since PHP is pretty unsuitable for this task.
See Github Issue #3: https://github.com/facebook/phabricator/issues/3
One thing I need to figure out next is if I can reasonably do SSL/TSL over Flash
(it looks like I can, in theory, with the as3crypto library) or if the server
needs to just send down version information and trigger a separate Ajax call on
the client.
Test Plan:
Created a client pool and connected it to the server, with election and failover
apparently working correctly.
Reviewed By: aran
Reviewers: Girish, aran, jungejason, tuomaspelkonen, davidrecordon
Commenters: Girish, davidrecordon
CC: aran, epriestley, Girish, davidrecordon
Differential Revision: 284
