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: People are hella lazy and don't want to do this themselves.
Test Plan: Generated a symbol file with duplicates and piped it in, got an import under --ignore-duplicates.
Reviewers: kdeggelman, btrahan, vrana, jungejason
Reviewed By: jungejason
CC: aran
Differential Revision: https://secure.phabricator.com/D2145
Summary: These elements look heavy and out of place right now.
Test Plan: Looked at error views in uiexample page.
Reviewers: btrahan, vrana, jungejason
Reviewed By: jungejason
CC: aran
Differential Revision: https://secure.phabricator.com/D2144
Summary: Currently, we show them everything. Instead, show them an explicit notice.
Test Plan: Looked at "My Projects" feed with no projects.
Reviewers: btrahan, vrana, jungejason
Reviewed By: jungejason
CC: aran
Maniphest Tasks: T1015
Differential Revision: https://secure.phabricator.com/D2143
Summary:
- Make some effort to simplify the code.
- Make "Skip Past This Commit" work in Git and Mercurial.
- Make blame work in Mercurial.
- Add tooltip hover state to show more information about commits.
Test Plan: Viewed blame views in SVN, Git, Hg. Clicked line numbers, hovered/clicked commits, hovered/clicked "blame past..."
Reviewers: btrahan, vrana, jungejason
Reviewed By: vrana
CC: aran
Maniphest Tasks: T378
Differential Revision: https://secure.phabricator.com/D2142
Summary: See D2080. The introduction of `arc land`, defaulting to `origin/master`, and --auto enormously simplifies the documentation.
Test Plan: Read documentation.
Reviewers: btrahan
Reviewed By: btrahan
CC: 20after4, aran
Maniphest Tasks: T894
Differential Revision: https://secure.phabricator.com/D2082
Summary:
The older logic was incorrect:
- It chose `change.left` for `data.on_right` being true.
- 'O' and 'N' mean 'old' and 'new', not 'left' and 'right'. In diff-of-diffs, both sides are 'N'.
So, select the changeset ID correctly (pick the right side one for on_right), and select the new file prefix correctly (N for new, O for old).
Test Plan: Waved my mouse over some inline comments in a diff-of-diffs, got reasonable-looking reticles.
Reviewers: vrana, btrahan
Reviewed By: vrana
CC: aran
Maniphest Tasks: T1076
Differential Revision: https://secure.phabricator.com/D2138
I think the issue is that we don't set the left-side changesetID correctly. This seems to work correctly locally, but I'm not sure I got a good repro. Pushing to verify the production test cases provided in T1076.
Auditors: vrana, btrahan
Summary:
`mysql_set_charset()` is available since PHP 5.2.3.
I've searched also for other new functions and this is the newest.
Test Plan: /
Reviewers: epriestley
Reviewed By: epriestley
CC: aran
Differential Revision: https://secure.phabricator.com/D2126
Summary:
The audit tools has many false positive about Author Not
Matching with Revision. The fix is to set the authorPHID which was
missing in the existing code
Test Plan:
run reparse.php and it doesn't generate false positive result
anymore.
Reviewers: nh, epriestley
Reviewed By: epriestley
CC: aran
Differential Revision: https://secure.phabricator.com/D2119
Summary: renderMiniPanel() renders the entire <p>.
Test Plan: Looked at page source for homepage, verified there was no double </p>.
Reviewers: vrana, btrahan
Reviewed By: vrana
CC: aran
Maniphest Tasks: T1079
Differential Revision: https://secure.phabricator.com/D2128
Summary: This separates common MySQL stuff (identifiers and comments escaping, error codes, connection retries) from PHP extension specific stuff (connect, query, fetch, errors, escape string).
Test Plan:
/
Use `AphrontMySQLiDatabaseConnection` in `PhabricatorLiskDAO`, load homepage, edit task, save task.
Reviewers: epriestley
Reviewed By: epriestley
CC: nh, aran
Differential Revision: https://secure.phabricator.com/D2113
Summary: I've found it useful mainly on smaller screen or with lots of comments.
Test Plan: Show Diff
Reviewers: epriestley
Reviewed By: epriestley
CC: aran
Differential Revision: https://secure.phabricator.com/D2122
Summary:
`116.utf8.sql` throws this under MySQL 5.5:
> Column length too big for column 'keyBody' (max = 21845); use BLOB or TEXT instead
I guess that's because MySQL 5.5 changed maximum length of UTF-8 character from 3 bytes to 4.
I've updated also `116.utf8.sql` for people with new installs.
Test Plan:
upgrade_schema.php
Reviewers: epriestley
Reviewed By: epriestley
CC: aran
Differential Revision: https://secure.phabricator.com/D2117
Summary:
This introduces some boundary checking for
PhabricatorOwnersOwner::loadAffiliatedUserPHIDs() if it gets passed an empty
array, which happened when I ran arc diff and it called
differential.createrevision.
Test Plan: ran arc diff
Reviewers: epriestley, meitros, jungejason
CC: aran
Differential Revision: https://secure.phabricator.com/D2112
Summary:
MySQL doesn't treat `\` as escaping character in ##``##.
This isn't probably SQL injection hole because I've found no calls of this method with user input.
But better safe than sorry.
See also [[http://dev.mysql.com/doc/refman/5.1/en/server-sql-mode.html#sqlmode_no_backslash_escapes | NO_BACKSLASH_ESCAPES]].
Test Plan:
lang=sql
SELECT `a\`b`; -- Throws: Syntax error near '`'.
-- Should throw: Unknown column 'a`b'.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran
Differential Revision: https://secure.phabricator.com/D2109
Summary:
I've considered that user may have set editor but not checked out Phabricator repositories.
But stack trace is useful mainly for developers.
Test Plan:
Click on path in Unhandled Exception.
Repeat with disabled editor.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran
Differential Revision: https://secure.phabricator.com/D2107
Summary: ...also make the pager usage in ChatLog use the nice formatWhereClause functionality
Test Plan: set $page_size = 2 and paged around the data a bit
Reviewers: epriestley
Reviewed By: epriestley
CC: aran
Maniphest Tasks: T905
Differential Revision: https://secure.phabricator.com/D2106
Summary: Use Edges to attach Commits and Tasks. Note, no "edit attached commits" interface from tasks yet since the search backend needs a little work to list commits in a sensible way.
Test Plan: Attached commits to tasks. Looked at commits, looked at tasks.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T904
Differential Revision: https://secure.phabricator.com/D2105
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:
We have a lot of cases where we store object relationships, but it's all kind of messy and custom. Some particular problems:
- We go to great lengths to enforce order stability in Differential revisions, but the implementation is complex and inelegant.
- Some relationships are stored on-object, so we can't pull the inverses easily. For example, Maniphest shows child tasks but not parent tasks.
- I want to add more of these and don't want to continue building custom stuff.
- UIs like the "attach stuff to other stuff" UI need custom branches for each object type.
- Stuff like "allow commits to close tasks" is notrivial because of nonstandard metadata storage.
Provide an association-like "edge" framework to fix these problems. This is nearly identical to associations, with a few differences:
- I put edge metadata in a separate table and don't load it by default, to keep edge rows small and allow large metadata if necessary. The on-edge metadata seemed to get abused a lot at Facebook.
- I put a 'seq' column on the edges to ensure they have an explicit, stable ordering within a source and type.
This isn't actually used anywhere yet, but my first target is attaching commits to tasks for T904.
Test Plan: Made a mock page that used Editor and Query. Verified adding and removing edges, overwriting edges, writing and loading edge data, sequence number generation.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran, 20after4
Differential Revision: https://secure.phabricator.com/D2088
Summary: NOTE: This is not produced by a script so there might be errors. Please review carefully.
Test Plan: Browse around Differential.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran
Differential Revision: https://secure.phabricator.com/D2103
Summary: I looooove JS! It makes me giddy with glee!
Test Plan: Picked dates. See screenshots.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D2086
Summary:
When system agent adds a comment then he is added to CC.
When I amend and update then I get message "Commit message references nonexistent ..."
Test Plan: Update revision with system agent in CC.
Reviewers: epriestley
Reviewed By: epriestley
CC: michalburger1, aran
Differential Revision: https://secure.phabricator.com/D2100
