1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-21 14:22:41 +01:00
Commit graph

159 commits

Author SHA1 Message Date
Joshua Spence
e40b18fb75 Fix call to debug.log.
Summary: As pointed out by @epriestley in D9458#62, this call to `debug.log` is missing an argument.

Test Plan: meh..

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D9485
2014-06-14 11:28:28 -07:00
Joshua Spence
cbd95b91b5 Implement unsubscription in the Aphlict client.
Summary: Ref T5284. When an `AphlictClient` is closed, it will eventually be purged from the pool by the `AphlictMaster`. When this happens, also unsubscribe the purged client from all notifications, and send an `unsubscribe` command to the Aphlict server if possible.

Test Plan:
Verified the output of the Aphlict server (running in debug mode):

```
[Wed Jun 11 2014 23:21:31 GMT+0000 (UTC)] <FlashPolicy> Policy Request From ::ffff:192.168.1.1
[Wed Jun 11 2014 23:21:31 GMT+0000 (UTC)] <Listener/2> Connected from ::ffff:192.168.1.1
[Wed Jun 11 2014 23:21:31 GMT+0000 (UTC)] <Listener/2> Received data: {"command":"subscribe","data":["PHID-USER-cb5af6p4oepy5tlgqypi"]}
[Wed Jun 11 2014 23:21:31 GMT+0000 (UTC)] <Listener/2> Subscribed to: ["PHID-USER-cb5af6p4oepy5tlgqypi"]
[Wed Jun 11 2014 23:21:39 GMT+0000 (UTC)] <Listener/2> Received data: {"command":"subscribe","data":["PHID-TASK-l2dtbs5xrt2b7abgh5a6"]}
[Wed Jun 11 2014 23:21:39 GMT+0000 (UTC)] <Listener/2> Subscribed to: ["PHID-TASK-l2dtbs5xrt2b7abgh5a6"]
[Wed Jun 11 2014 23:21:57 GMT+0000 (UTC)] <Listener/2> Received data: {"command":"unsubscribe","data":["PHID-TASK-l2dtbs5xrt2b7abgh5a6"]}
[Wed Jun 11 2014 23:21:57 GMT+0000 (UTC)] <Listener/2> Unsubscribed from: ["PHID-TASK-l2dtbs5xrt2b7abgh5a6"]
```

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley, Korvin

Maniphest Tasks: T5284

Differential Revision: https://secure.phabricator.com/D9492
2014-06-11 20:02:01 -07:00
Joshua Spence
aa534c69f0 Move subscription updates to the register function.
Summary: Currently, the `AphlictClient` will only send its subscriptions to the `AphlictMaster` once. If the original `AphlictMaster` is closed and a new master is created, then client subscriptions will be lost.

Test Plan: Opened two separate tabs. Closed the "master" tab and noticed that the subscriptions were re-sent to the server.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D9487
2014-06-11 14:09:41 -07:00
Joshua Spence
84d259cea2 Modify the Aphlict server to transmit messages instead of broadcasting them.
Summary: Ref T4324. Ref T5284. This adds server-side support for keeping track of a set of PHIDs that the Aphlict clients have subscribed to. Instead of broadcasting a notification to all clients (after which the clients can poll `/notification/individual` in order to determine whether or not they are interested in the notification), transmit notifications only to clients that have subscribed to a PHID that is relevant to the notification.

Test Plan:
I opened up two clients on the same host (incognito tabs in Chrome). Here is the output from the server:

```
> sudo ./bin/aphlict debug
Starting Aphlict server in foreground...
Launching server:

    $ 'nodejs' '/usr/src/phabricator/src/applications/aphlict/management/../../../../support/aphlict/server/aphlict_server.js' --port='22280' --admin='22281' --host='localhost' --user='aphlict'

[Wed Jun 11 2014 19:10:27 GMT+0000 (UTC)] Started Server (PID 4546)
[Wed Jun 11 2014 19:10:36 GMT+0000 (UTC)] <FlashPolicy> Policy Request From ::ffff:192.168.1.1
[Wed Jun 11 2014 19:10:37 GMT+0000 (UTC)] <Listener/1> Connected from ::ffff:192.168.1.1
[Wed Jun 11 2014 19:10:37 GMT+0000 (UTC)] <Listener/1> Received data: {"command":"subscribe","data":["PHID-USER-cb5af6p4oepy5tlgqypi"]}
[Wed Jun 11 2014 19:10:37 GMT+0000 (UTC)] <Listener/1> Subscribed to: ["PHID-USER-cb5af6p4oepy5tlgqypi"]
[Wed Jun 11 2014 19:10:39 GMT+0000 (UTC)] <Listener/1> Received data: {"command":"subscribe","data":["PHID-USER-kfohe3ca5oe6ygykmioq"]}
[Wed Jun 11 2014 19:10:39 GMT+0000 (UTC)] <Listener/1> Subscribed to: ["PHID-USER-kfohe3ca5oe6ygykmioq"]
[Wed Jun 11 2014 19:10:42 GMT+0000 (UTC)] notification: {"key":"6023751084283587681","type":"notification","subscribers":["PHID-USER-cb5af6p4oepy5tlgqypi"]}
[Wed Jun 11 2014 19:10:42 GMT+0000 (UTC)] <Listener/1> Wrote Message
```

I verified (using the "Network" tab in Chrome) that an AJAX request to `/notification/individual/` was only made in the tab belonging to the user that triggered the test notification.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin

Maniphest Tasks: T5284, T4324

Differential Revision: https://secure.phabricator.com/D9458
2014-06-11 12:17:29 -07:00
epriestley
60d2b743d9 Print out stack traces when Aphlict server dies
Summary: Printing out `err` is less informative than `err.stack`, which has the message, type, //and// a trace.

Test Plan:
Faked an exception, then:

```
$ sudo ./bin/aphlict debug
Starting Aphlict server in foreground...
Launching server:

    $ 'node' '/INSECURE/devtools/phabricator/src/applications/aphlict/management/../../../../support/aphlict/server/aphlict_server.js' --port='22280' --admin='22281' --host='localhost'

[Wed Jun 11 2014 10:20:39 GMT-0700 (PDT)]
<<< UNCAUGHT EXCEPTION! >>>
Error: Example Exception
    at Object.<anonymous> (/INSECURE/devtools/phabricator/support/aphlict/server/aphlict_server.js:73:7)
    at Module._compile (module.js:456:26)
    at Object.Module._extensions..js (module.js:474:10)
    at Module.load (module.js:356:32)
    at Function.Module._load (module.js:312:12)
    at Function.Module.runMain (module.js:497:10)
    at startup (node.js:119:16)
    at node.js:901:3
>>> Server exited!
```

Reviewers: joshuaspence

Reviewed By: joshuaspence

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D9481
2014-06-11 10:40:04 -07:00
Joshua Spence
ab4324148a Make the Aphlict server more resilient.
Summary:
Currently, the Aphlict server will crash if invalid JSON data is `POST`ed to it. I have fixed this to, instead, return a 400. Also made some minor formatting changes.

Ref T4324. Ref T5284. Also, modify the data structure that is passed around (i.e. `POST`ed to the Aphlict server and broadcast to the Aphlict clients) to include the subscribers. Initially, I figured that we shouldn't expose this information to the clients... however, it is necessary for T4324 that the `AphlictMaster` is able to route a notification to the appropriate clients.

Test Plan:
Making the following `curl` request: `curl --data "{" http://localhost:22281/`.

**Before**
```
sudo ./bin/aphlict debug
Starting Aphlict server in foreground...
Launching server:

    $ 'nodejs' '/usr/src/phabricator/src/applications/aphlict/management/../../../../support/aphlict/server/aphlict_server.js' --port='22280' --admin='22281' --host='localhost' --user='aphlict'

[Wed Jun 11 2014 17:07:51 GMT+0000 (UTC)] Started Server (PID 2033)
[Wed Jun 11 2014 17:07:55 GMT+0000 (UTC)]
<<< UNCAUGHT EXCEPTION! >>>

SyntaxError: Unexpected end of input
>>> Server exited!
```

**After**
(No output... the bad JSON is caught and a 400 is returned)

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley, Korvin

Maniphest Tasks: T4324, T5284

Differential Revision: https://secure.phabricator.com/D9480
2014-06-11 10:17:12 -07:00
Joshua Spence
0a62f13464 Change double quotes to single quotes.
Summary: Ran `arc lint --apply-patches --everything` over rP, mainly to change double quotes to single quotes where appropriate. These changes also validate that the `ArcanistXHPASTLinter::LINT_DOUBLE_QUOTE` rule is working as expected.

Test Plan: Eyeballed it.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley, Korvin, hach-que

Differential Revision: https://secure.phabricator.com/D9431
2014-06-09 11:36:50 -07:00
Joshua Spence
7d255aedba Improve error handling for Aphlict.
Summary: Currently, any error thrown when instantiating an `AphlictMaster` will be assumed to be due to the master already existing. This is a bit overzealous because the [[http://help.adobe.com/en_US/FlashPlatform/reference/actionscript/3/flash/net/LocalConnection.html#connect() | documentation]] specifically states than an `ArgumentError` will be throw if "the `LocalConnection` instance is already connected".

Test Plan: Inspected the log message.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D9422
2014-06-07 15:16:36 -07:00
Joshua Spence
7f9a6626a0 Mark some ActionScript methods as final.
Summary: These methods shouldn't be overridden. Marking them as `final` will enforce this.

Test Plan: N/A

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D9411
2014-06-07 11:35:17 -07:00
Joshua Spence
ddf5412cbb Add a ./bin/aphlict build workflow.
Summary:
Currently, it is a bit tricky to build the Aphlict client SWF from the ActionScript source. Provide a `./bin/aphlict build` workflow that simplifies this process.

Depends on D9226.

Test Plan:
Executed the workflow:

```
> ./bin/aphlict build
Done.
```

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D9338
2014-06-07 11:34:07 -07:00
Joshua Spence
8a7a7dcbf1 Make ./bin/aphlict behave more like a service.
Summary: Fixes T5126. Provide `start`, `stop`, `restart`, `debug` and `status` workflows for `./bin/aphlict`. This makes it easier to manage Aphlict as if it were a service.

Test Plan:
```
> sudo ./bin/aphlict status
Aphlict is not running.

> sudo ./bin/aphlict stop
Aphlict is not running.

> sudo ./bin/aphlict start
Aphlict Server started.

> sudo ./bin/aphlict status
Aphlict (12880) is running.

> sudo ./bin/aphlict restart
Stopping Aphlict Server (12880)...
Aphlict Server (12880) exited normally.
Aphlict Server started.

> sudo ./bin/aphlict stop
Stopping Aphlict Server (12895)...
Aphlict Server (12895) exited normally.

> sudo ./bin/aphlict debug
Starting Aphlict server in foreground...
Launching server:

    $ node '/usr/src/phabricator/src/applications/aphlict/management/../../../../support/aphlict/server/aphlict_server.js' --port='22280' --admin='22281' --host='localhost' --user='aphlict'

[Fri May 30 2014 09:56:14 GMT+0000 (UTC)] Started Server (PID 12911)
```

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: hach-que, epriestley, Korvin

Maniphest Tasks: T5126

Differential Revision: https://secure.phabricator.com/D9226
2014-06-05 12:13:45 -07:00
Joshua Spence
fe98aa6839 Publish additional context to the Aphlict server.
Summary:
Ref T4324. As well as sending the key for the notification, also publish the notification type and a list of subscribers to the Aphlict server.

The idea here is that the Aphlict server passes anything within the `data` key to the clients, whereas other keys (such as `subscribers`) will be used by the server to determine where the notifications should be routed.

Note that these changes don't do anything useful, but are a prerequisite for further work on T4324.

Test Plan:
Sent myself test notifications at `/notification/status/`. Also inspected the Aphlict server debug output:

```
> sudo ./bin/aphlict --foreground
Starting server in foreground, ignoring pidfile...
Launching server:

    $ node '/usr/src/phabricator/support/aphlict/server/aphlict_server.js' --port='22280' --admin='22281' --host='localhost' --user='aphlict' --log='/var/log/aphlict.log'

[Thu Jun 05 2014 18:38:14 GMT+0000 (UTC)] Started Server (PID 15437)
[Thu Jun 05 2014 18:38:16 GMT+0000 (UTC)] <FlashPolicy> Policy Request From ::ffff:10.0.0.1
[Thu Jun 05 2014 18:38:16 GMT+0000 (UTC)] <Listener/1> Connected from ::ffff:10.0.0.1
[Thu Jun 05 2014 18:38:19 GMT+0000 (UTC)] notification: {"data":{"key":"6021516228036848559","type":"notification"},"subscribers":["PHID-USER-cb5af6p4oepy5tlgqypi"]}
[Thu Jun 05 2014 18:38:19 GMT+0000 (UTC)] <Listener/1> Wrote Message
```

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin

Maniphest Tasks: T4324

Differential Revision: https://secure.phabricator.com/D9396
2014-06-05 12:09:26 -07:00
Joshua Spence
3202f0f23d Post data to the Aphlict server in JSON encoded form.
Summary:
Ref T4324. Currently, notifications data is `POST`ed to the Aphlict server in the `application/x-www-form-urlencoded` format. This works fine for simple data but is problematic for nested data. For example:

```lang=php
array(
  'data' => array(
    'key'  => '6021329908492455737',
    'type' => 'PhabricatorNotificationAdHocFeedStory',
  ),
  'subscribers' => array(
    'PHID-USER-y7ofqm276ejs62yqghge',
  ),
);
```

Is encoded as `data%5Bkey%5D=6021329908492455737&data%5Btype%5D=PhabricatorNotificationAdHocFeedStory&subscribers%5B0%5D=PHID-USER-y7ofqm276ejs62yqghge`. This string is then (incorrectly) decoded by `querystring.parse` as:

```lang=javascript
> querystring.parse('data%5Bkey%5D=6021329908492455737&data%5Btype%5D=PhabricatorNotificationAdHocFeedStory&subscribers%5B0%5D=PHID-USER-y7ofqm276ejs62yqghge');
{ 'data[key]': '6021329908492455737',
  'data[type]': 'PhabricatorNotificationAdHocFeedStory',
  'subscribers[0]': 'PHID-USER-y7ofqm276ejs62yqghge' }
```

Test Plan: Sent test notifications from `/notification/status/` and verified that the notifications still worked.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley, Korvin

Maniphest Tasks: T4324

Differential Revision: https://secure.phabricator.com/D9386
2014-06-05 09:47:33 -07:00
Joshua Spence
8033a69746 Catch errors that may occur whilst receiving data from the Aphlict server.
Summary: Ref T4324. Currently, if the `AphlictMaster` receives dodgy data from the Aphlict server (invalid JSON, for example) then a syntax error will be thrown and the `AphlictMaster` will die. Instead, catch errors and raise a notification.

Test Plan: {F163466}

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley, Korvin

Maniphest Tasks: T4324

Differential Revision: https://secure.phabricator.com/D9380
2014-06-05 09:43:41 -07:00
Joshua Spence
b01f57bfdb Explicitly enable strict compilation for the Aphlict client SWF.
Summary:
Strict mode is enabled by default, but it would be best to explicitly enable it... just in case.

Strict mode prints undefined property and function calls; also performs compile-time type checking on assignments and options supplied to method calls. See http://help.adobe.com/en_US/flex/using/WS2db454920e96a9e51e63e3d11c0bf69084-7a92.html

Test Plan: N/A

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D9354
2014-06-03 06:10:28 -07:00
Joshua Spence
802463cb94 Don't set the default size and background color for the Aphlict client SWF.
Summary: These settings don't seem to make any difference and are somewhat confusing.

Test Plan: Sent myself test notifications via `/notification/status`. Things seemed to work.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D9337
2014-05-30 03:17:39 -07:00
Joshua Spence
e05f427f3d Compress the Aphlict client SWF.
Summary: Remove the `-debug=true` flags from the script used to build the Aplhict client SWF.

Test Plan:
**Before**
```
> du -h webroot/rsrc/swf/aphlict.swf
20K
```

**After**
```
> du -h webroot/rsrc/swf/aphlict.swf
16K
```

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D9333
2014-05-29 14:48:23 -07:00
Joshua Spence
a42ec32c98 Modify the Aphlict client to use LocalConnection.
Summary:
Ref T4324. Currently, an Aphlict client (with a corresponding connection to the Aphlict Server) is created for every tab that a user has open. This significantly affects the scalability of Aphlict as a service. Instead, we can use `LocalConnection` instances to coordinate the communication of multiple Aphlict clients to the server.

Similar functionality existed prior to D2704, but was removed as the author was not able to get this functionality working as intended. It seems that the main issue with the initial attempt was the use of the `setTimeout` function, which seemed to be a blocking call which prevented messages from being received. I have instead used an event-based model using a `Timer` object.

Roughly this works as follows:

# The first instance will create an `AphlictClient` and an `AphlictMaster`. The `AphlictClient` will register itself with the `AphlictMaster` and will consequently be notified of incoming messages.
# The `AphlictClient` is then responsible for pinging the `AphlictMaster` at regular intervals. If the client does not ping the master in a given period of time, the master will assume that the client is dead and will remove the client from the pool.
# Similarly, the `AphlictMaster` is required to respond to pings with a "pong" response. The pong response lets the clients know that the `AphlictMaster` is still alive. If the clients do not receive a pong in a given period of time, then the clients will attempt to spawn a new master.

Test Plan: I have tested this on our Phabricator install with a few tabs opened and inspecting the console output. I will upload a screencast of my test results.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley, Korvin

Maniphest Tasks: T4324

Differential Revision: https://secure.phabricator.com/D9327
2014-05-29 07:04:22 -07:00
epriestley
c887871d30 Disable rate limiting by default in general
Summary: This is still too rough to enable by default. We can turn it on for `secure.phabricator.com` and tweak it a bit first, and then release it in general with higher defaults and more sensible behavior in edge cases.

Test Plan: Loaded some pages.

Reviewers: btrahan, spicyj

Reviewed By: spicyj

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D8736
2014-04-09 11:52:34 -07:00
epriestley
4d0935ba5e Rate limit requests by IP
Summary:
Fixes T3923. On `secure.phabricator.com`, we occasionally get slowed to a crawl when someone runs a security scanner against us, or 5 search bots decide to simultaneously index every line of every file in Diffusion.

Every time a user makes a request, give their IP address some points. If they get too many points in 5 minutes, start blocking their requests automatically for a while.

We give fewer points for logged in requests. We could futher refine this (more points for a 404, more points for a really slow page, etc.) but let's start simply.

Also, provide a mechanism for configuring this, and configuring the LB environment stuff at the same time (this comes up rarely, but we don't have a good answer right now).

Test Plan: Used `ab` and reloading over and over again to hit rate limits. Read documentation.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: chad, epriestley

Maniphest Tasks: T3923

Differential Revision: https://secure.phabricator.com/D8713
2014-04-08 18:36:21 -07:00
Joshua Spence
6270114767 Various linter fixes.
Summary:
- Removed trailing newlines.
- Added newline at EOF.
- Removed leading newlines.
- Trimmed trailing whitespace.
- Spelling fix.
- Added newline at EOF

Test Plan: N/A

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley

CC: hach-que, chad, Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D8344
2014-02-26 12:44:58 -08:00
epriestley
f302bfc8f8 Break Aphlict's flash policy server into a separate class
Summary: Ref T4324. One of the server we start just sends pre-canned XML responses. Separate it out of the main file and hand it all the objects it interacts with in structured, reasonable ways.

Test Plan: Hit "Send Test Notification", saw notification, saw flash policy info in the log.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T4324

Differential Revision: https://secure.phabricator.com/D8257
2014-02-17 16:01:09 -08:00
epriestley
28fe44da0a Break some of Aphlict into reasonable classes with sensible responsibilities
Summary:
Ref T4324.

  - Create `Listener` to represent listening clients.
  - Create `ListenerList` to represent the current list of clients.
  - Create `Logfile` to handle logging.

Test Plan: Clicked "Send Test Notification", verified logs, status and notifications all work correctly.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T4324

Differential Revision: https://secure.phabricator.com/D8256
2014-02-17 16:00:51 -08:00
epriestley
260eb5344b Allow Aphlict to load Javelin and use Javelin class definitions
Summary:
Ref T4324. The server code is probably going to get a fair amount more complicated, so allow it to load Javelin classes in a mostly-reasonable way.

This integration has a few warts, but should be good enough to let us manage complexity through the next iteration of the server.

(Mostly I just want the concicse Javelin mechanism for defining new classes.)

Version bump is just so I can figure stuff out if this creates any issues for users based on which version of things they're running.

Test Plan: Started server, posted some messages through it.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T4324

Differential Revision: https://secure.phabricator.com/D8253
2014-02-17 16:00:01 -08:00
epriestley
a1e7a4ccca Version the Aphlict notification server and prompt users to upgrade if they're out of date
Summary: Ref T4324. Add some version information to the server status output, and setup checks to test for an unreachable or out-of-date server.

Test Plan:
  - With server down, hit reasonable setup check.
  - With server up and at a bad version, hit reasonable setup check.
  - Viewed `/notification/status/`.
  - The CSS thing fixes this:

{F114445}

Reviewers: btrahan, chad

Reviewed By: chad

CC: chad, aran

Maniphest Tasks: T4324

Differential Revision: https://secure.phabricator.com/D8251
2014-02-17 15:59:39 -08:00
epriestley
1a964f71bb Disable SimpleXML entity loader in Phabricator
Summary: See D8049. Same deal as that one, but this is in the Phabricator web stack.

Test Plan: Man oh man.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D8050
2014-01-23 14:00:44 -08:00
epriestley
56bcb33a18 Improve exception reporting behavior for core exceptions
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
2014-01-21 14:03:09 -08:00
epriestley
f88a2b735d Remove spurious "+x" from files that shouldn't have it
Summary: We have a bunch of files with +x that aren't actually executable.
Remove +x from PNGs, etc.
2013-10-05 05:18:17 -07:00
epriestley
7298589c86 Proof of concept mitigation of BREACH
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
2013-08-07 16:09:05 -07:00
Eric Stern
b20a0eed13 Filter only possibly-tainted keys from superglobals
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
2013-08-05 11:45:21 -07:00
Eric Stern
44a883f941 Pass raw QUERY_STRING to parser
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
2013-08-04 18:07:35 -07:00
epriestley
a7e4b846d6 Fix Aphlict server for newer Node
Summary: Fixes T3630.

Test Plan: Ran server on 0.11.0, got notifications.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T3630

Differential Revision: https://secure.phabricator.com/D6588
2013-07-27 16:24:12 -07:00
epriestley
4a19b51b92 Catch more known-bad versions of APC
Summary: Fixes T3550. @nmalcolm ended up with "3.1.15-dev".

Test Plan: n/a

Reviewers: btrahan, nmalcolm

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T3550

Differential Revision: https://secure.phabricator.com/D6473
2013-07-16 11:45:29 -07:00
epriestley
c05e026e65 Detect and warn about APC 3.1.14 / 3.1.15
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
2013-07-10 13:20:00 -07:00
epriestley
e54a2c1325 Minor, fix obvious typo in Aphlict server error handling. 2013-06-28 14:42:46 -07:00
Gareth Evans
b26549b5fa Implement PhutilRequest parser #2
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
2013-06-24 08:22:26 -07:00
epriestley
c48f64b391 Specify HOME when invoking Git commands
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
2013-05-21 14:14:31 -07:00
epriestley
0569218201 Use JsShrink if jsxmin is not available
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
2013-05-18 17:04:22 -07:00
epriestley
dc6c1bf435 Fail quietly when failing to write access log
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
2013-05-09 16:08:26 -07:00
epriestley
2b8adb3a83 Don't hang on /status/ for newer Node
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
2013-04-11 15:45:50 -07:00
epriestley
0ad05e911b Fix startup issue with access log
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
2013-04-09 11:27:37 -07:00
epriestley
8b6fc615f4 Ignore and README for support/bin
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
2013-04-03 12:58:39 -07:00
epriestley
cde1416446 Guarantee the existence of the Phabricator access log
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
2013-04-02 09:53:56 -07:00
epriestley
f1a36cf3c8 Make it easier to use print_r() debugging
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
2013-02-11 11:06:59 -08:00
epriestley
3c3e00a74f Raise early fatal for bad rewrite with no "/"
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
2013-01-22 17:17:37 -08:00
epriestley
9e6d59829c Consolidate environmental initialization
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
2012-12-25 06:15:28 -08:00
epriestley
ed58f6c5f4 Move a lot of pre-request checks to PhabricatorStartup
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
2012-12-25 06:11:39 -08:00
Nick Harper
53115007ff Make bin/aphlict behave slightly better when forking
Summary:
Due to how file descriptors get inherited when a process forks and the way
our script to start/restart stuff works, having bin/aphlict not close
STDOUT caused our script to hang. This diff provides a solution; however,
something like https://secure.phabricator.com/diffusion/PHU/browse/master/src/daemon/PhutilDaemonOverseer.php;b45eea975f6d9afc$127-129
may be better or more robust.

Test Plan:
Run our script that calls bin/aphlict in passthru() and see that it no
longer hangs.

Reviewers: epriestley, vrana

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4240
2012-12-20 10:38:13 -08:00
vrana
776a240870 Let Aphlict listen on the specified host
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
2012-12-19 15:44:17 -08:00
vrana
ef85f49adc Delete license headers from files
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
2012-11-05 11:16:51 -08:00
epriestley
b3ad8507af Allow simple template-based skin definitions
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
2012-10-17 08:36:48 -07:00
epriestley
78d2f08fcd Add an Aphlict CLI client
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
2012-07-05 16:04:04 -07:00
epriestley
5e184ee593 Improve debug support for notifications
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
2012-06-20 13:20:47 -07:00
epriestley
45a3421e9b Minor, fix Aphlict wrapper signal handling issue under dash. 2012-06-18 15:20:25 -07:00
epriestley
bad22cb303 Add a bin/aphlict wrapper to handle aphlict config / daemonization
Summary: Simple wrapper script to configure, launch, daemonize and restart the Aphlict server.

Test Plan: Ran "bin/aphlict", checked /notification/status/. Ran "bin/aphlict --foreground".

Reviewers: jungejason, vrana

Reviewed By: jungejason

CC: aran

Maniphest Tasks: T944

Differential Revision: https://secure.phabricator.com/D2784
2012-06-18 15:11:19 -07:00
epriestley
86040227b0 Improve Aphlict server
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
2012-06-14 06:12:54 -07:00
David Fisher
f8f195b329 Make Notifications Realtime
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
2012-06-11 17:51:12 -07:00
epriestley
f8431bbfee Make Aphlict client somewhat more approachable
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
2012-03-06 20:14:03 -08:00
epriestley
84731e8f00 Aphlict, simple notification server
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
2011-05-17 10:32:41 -07:00