Summary: PHP 5.4 raises an E_STRICT warning about "/e", and it's a generally awful idea, even though this case doesn't look exploitable. Silence the warning by using preg_replace_callback() instead of "/e".
Test Plan: Sent myself a message with a bunch of multibyte UTF8 characters in it, it came through cleanly.
Reviewers: btrahan, vrana, jungejason
Reviewed By: vrana
CC: aran
Maniphest Tasks: T1056
Differential Revision: https://secure.phabricator.com/D2147
Summary:
various stripe stuff, including
- external stripe library
- payment form
- test controller to play with payment form, sample business logic
My main questions / discussion topics are...
- is the stripe PHP library too big? (ie should I write something more simple just for phabricator?)
-- if its cool, what is the best way to include the client? (ie should I make it a submodule rather than the flat copy here?)
- is the JS I wrote (too) ridiculous?
-- particularly unhappy with the error message stuff being in JS *but* it seemed the best choice given the most juicy error messages come from the stripe JS such that the overall code complexity is lowest this way.
- how should the stripe JS be included?
-- flat copy like I did here?
-- some sort of external?
-- can we just load it off stripe servers at request time? (I like that from the "if stripe is down, stripe is down" perspective)
- wasn't sure if the date control was too silly and should just be baked into the form?
-- for some reason I feel like its good to be prepared to walk away from Stripe / switch providers here, though I think this is on the wrong side of pragmatic
Test Plan: - played around with sample client form
Reviewers: epriestley
Reviewed By: epriestley
CC: aran
Differential Revision: https://secure.phabricator.com/D2096
Summary:
Currently, we sort all results alphabetically. This isn't ideal. Instead, sort them like this:
- If the viewing user appears in the list, always sort them first. This is common in a lot of contexts and some "Ben Evans" guy is sorting first on secure.phabricator.com and causing me no end of aggravation.
- If the tokens match a "priority" component (e.g., username), sort that before results which do not have a "priority" match.
- Within a group (self, priority, everything else) sort tokens alphabetically.
NOTE: I need to go add setUser() to all the tokenizers to make the "self" rule work, but that's trivial so I figured I'd get this out first.
Test Plan:
https://secure.phabricator.com/file/data/4s2a72l5hhyyqqkq4bnd/PHID-FILE-x2r6ubk7s7dz54kxmtwx/Screen_Shot_2012-03-07_at_9.18.03_AM.png
Previously, "aaaaaepriestley" (first alphabetic match) would sort before "epriestley" (the viewing user). Now, "epriestley" sorts first because that is the viewer.
https://secure.phabricator.com/file/data/rmnxgnafz42f23fsjwui/PHID-FILE-yrnn55jl3ysbntldq3af/Screen_Shot_2012-03-07_at_9.18.09_AM.png
Previously, "aaaagopher" (first alphabetic match) would sort before "banana" (the "priority" match). Now, "banana" sorts first because it priority matches on username.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran, epriestley
Maniphest Tasks: T946
Differential Revision: https://secure.phabricator.com/D1807
Summary:
- Update the Javelin submodule to pick up recent fixes (like D1749).
- Update the package definitions do do a slightly better job of packaging
resources.
Test Plan:
Up and down work in tokenizers now. Pages load slightly fewer
resources.
Reviewers: vrana, btrahan
Reviewed By: btrahan
CC: aran, epriestley
Maniphest Tasks: T927
Differential Revision: https://secure.phabricator.com/D1751
Summary:
This is so freaking cool that I will try to implement it also on Facebook.
Idea is from
http://strd6.com/2011/09/html5-javascript-pasting-image-data-in-chrome/.
I don't know how to properly detect support but lying about it is not a big
deal.
Test Plan:
Go to revision comment textarea.
Paste some text data - works as usual.
Paste some image data in Chrome - file is uploaded and a link to it is inserted.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, epriestley
Differential Revision: https://secure.phabricator.com/D1681
Summary:
I typed up like 30 pages here and then got my soul crushed by T895, but here's
the summary:
I looked at several charting libraries. There aren't very many that seem to be
any good and have an open-source license.
I also want the charts to be scriptable in JS so we can add good interactivity
where appropriate.
Raphael is an SVG drawing library which seems very solid. gRaphael is a charting
library on top of Raphael that is a lot less solid, but seems kind of OK.
Overall, I think this selection gives us a lot of flexibility, although we'll
have to pay some costs up front. I'd rather do that then get limited later,
though.
That said, I'm open to other suggestions here if anyone has experience or wants
to take a different stab at researching things.
This is largely for @vii and D1643.
Test Plan: Created a basic, fairly OK chart (see next revision).
Reviewers: btrahan, vii
Reviewed By: btrahan
CC: aran, epriestley
Differential Revision: https://secure.phabricator.com/D1654
Summary: Preview of Add Reviewers looks silly without actually showing them
Test Plan:
Go to any diff
Leap into action: Add Reviewers
Add some reviewers
Write some comment
Preview including Added reviewers should be displayed
Change action to Comment
Added reviewers should disappear
Repeat with Add CCs
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, epriestley, vrana
Differential Revision: https://secure.phabricator.com/D1276
Summary:
- Update Javelin to HEAD -- this doesn't pick up anything in particular, but
lets us smoke test some stuff like {D1217}.
- Do a little more packaging since we've picked up a handful of 10-line
behaviors and such for various UI tweaks.
Test Plan:
- Generally, this should be very low-risk.
- Browed Maniphest, Differential, Diffusion and tried to hit all the JS
interactions.
- Looked over the Javelin changes we're pulling in to see if I forgot
anything. The only API change I caught was removal of "JX.defer()", but that was
already cleared in Phabricator in D803.
Reviewers: aran, btrahan, jungejason
Reviewed By: aran
CC: aran
Differential Revision: 1240
Summary: Routine administration.
Test Plan: Use a tokenizer and browse around
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, epriestley
Differential Revision: 1083
Summary: cpiro reported a cache inconsistency issue from a push a while ago
which this should fix (see #?????), and we haven't sync'd in a while anyway.
Test Plan: Poked some interfaces very gently.
Reviewers: cpiro, cpojer, tomo, jungejason, tuomaspelkonen, aran
Reviewed By: tomo
CC: aran, epriestley, tomo, cpiro
Differential Revision: 859
Summary:
Right now, the "SimpleEmailService" class uses trigger_error() to communicate
error messages. This means they get lost in the error logs and aren't visible in
the MetaMTA interface.
Provide a flag to strengthen them into exceptions, instead.
(I've attempted to emulate the prevailing style so I can offer this upstream.)
Test Plan: Faked an error condition and got a detailed stack trace in MetaMTA
instead of an empty "Message" field.
Reviewed By: jungejason
Reviewers: hunterbridges, codeblock, jungejason, tuomaspelkonen, aran
CC: aran, jungejason
Differential Revision: 783
Summary: Implements an S3 storage engine option for Phabricator.
Test Plan:
- Uploaded files to S3.
- Looked at them.
- Verified they appeared in S3 using the S3 file browser.
Reviewed By: jungejason
Reviewers: jungejason, tuomaspelkonen, aran
CC: aran, jungejason
Differential Revision: 752
Summary:
Mail clients can send messages where the body is represented as 'inline'
attachments. Don't treat any such text attachments as actual attachments.
Test Plan:
toulouse, can you verify this fixes the issue?
Reviewed By: toulouse
Reviewers: toulouse
CC: aran, toulouse, epriestley
Differential Revision: 441
Summary:
Allows you to link to comments with "D123#3" or "T123#3", then adds a pile of JS
to try to make it not terrible. :/
The thing I'm trying to avoid here is when someone says "look at this!
http://blog.com/#comment-239291" and you click and your browser jumps somewhere
random and you have no idea which comment they meant. Since I really hate this,
I've tried to avoid it by making sure the comment is always highlighted.
Test Plan:
Put T1#1 and D1#1 in remarkup and verified they linked properly.
Clicked anchors on individual comments.
Faked all comments hidden in Differential and verified they expanded on anchor
or anchor change.
Reviewed By: aran
Reviewers: aran, tomo, mroch, jungejason, tuomaspelkonen
CC: aran, epriestley
Differential Revision: 383
Summary:
We hit this very short (1s) timeout when the browser chooses to resolve all the
diff requests before the preview request. In the long term we could start the
preview request only after all the diff requests resolve, but this solves the
issue for now and there's no reason for such a short timeout.
The historical reason to have this timeout at all is that intern was megaflaky
and that's no longer a problem.
Test Plan:
Faked it so it would use a 1ms timeout the first time and then a 20s timeout;
got reasonable behavior.
Reviewed By: aran
Reviewers: jungejason, tuomaspelkonen, aran
CC: aran
Differential Revision: 329
Summary:
When a task description is updated, there's currently no way to see the change.
Build an "expanded summary" mode for transactions that shows description change
details. Also include changes in the email.
Test Plan:
Changed task descriptions, clicked "show details", read email.
Reviewed By: aran
Reviewers: tuomaspelkonen, jungejason, aran
CC: anjali, aran, epriestley
Differential Revision: 320
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
Summary:
Javelin is currently embedded in Phabricator via copy-and-paste of prebuilt
packages. This is not so great.
Pull it in as a submodule instead and make all the Phabriator resources declare
proper dependency trees. Add Javelin linting.
Test Plan:
I tried to run through pretty much all the JS functionality on the site. This is
still a high-risk change, but I did a pretty thorough test
Differential: inline comments, revealing diffs, list tokenizers, comment
preview, editing/deleting comments, add review action.
Maniphest: list tokenizer, comment actions
Herald: rule editing, tokenizers, add/remove rows
Reviewed By: tomo
Reviewers: aran, tomo, mroch, jungejason, tuomaspelkonen
CC: aran, tomo, epriestley
Differential Revision: 223
Summary:
Provides support for per-user x per-object unique reply-to email addresses, plus
SMTP integration.
This does not actually make Phabricator use these in outbound email.
Test Plan:
Used test console to validate in-Phabricator routing and handling.
Piped emails into the "mail_handler.php" script to validate mail parsing.
Configured sendmail and sent mail to Phabricator.
Technically I haven't conducted all parts of this test on the same machine since
I lost the will to configure more SMTP servers after configuring phabricator.com
Reviewed By: jungejason
Reviewers: jungejason, tuomaspelkonen, aran
CC: aran, epriestley, jungejason
Differential Revision: 226
