1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-12-04 20:52:43 +01:00
Commit graph

13195 commits

Author SHA1 Message Date
jungejason
5b8577db59 Add documentation for the Owners tool
Summary:
As title.

Please help me to improve the wording!

Test Plan:
generate the documentation from the diviner file; read it; spell
check

Reviewers: epriestley, nh

Reviewed By: nh

CC: aran, dihde14, mpodobnik, prithvi, TomL, epriestley

Differential Revision: https://secure.phabricator.com/D1395
2012-02-11 16:42:18 -08:00
vrana
95feb31fbf Whitespace parameter on Show Raw File is useless
Test Plan: Show Raw File

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1600
2012-02-11 09:13:19 -08:00
vrana
63fbd5db04 Avoid double '/' in Phriction URL
Test Plan:
/project/view/11/
Hover Wiki.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1601
2012-02-11 08:48:24 -08:00
Nick Harper
f2636fcb04 Fix spelling mistake in PhabricatorOwnerRelatedListController
Summary: selected was misspelled

Test Plan: none

Reviewers: epriestley, jungejason

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1586
2012-02-10 16:48:14 -08:00
vrana
cd22d837cb Revive added reviewers and ccs
Test Plan: Display revision with added reviewers.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1598
2012-02-10 11:48:57 -08:00
Evan Priestley
c53ebcec6b Merge pull request #90 from Koolvin/patch-1
D1535
2012-02-10 09:53:59 -08:00
Korvin Szanto
ad9a2ab00c D1535
IRC bot responds to T1000 as if you were referencing the nanomorph mimetic poly-alloy assassin.
2012-02-10 09:47:57 -08:00
epriestley
ecd4b03a4e Unbreak OAuth Registration
Summary:
@vrana patched an important external-CSRF-leaking hole recently (D1558), but
since we are sloppy in building this form it got caught in the crossfire.

We set action to something like "http://this.server.com/oauth/derp/", but that
triggers CSRF protection by removing CSRF tokens from the form. This makes OAuth
login not work.

Instead, use the local path only so we generate a CSRF token.

Test Plan: Registered locally via oauth.

Reviewers: vrana, btrahan

Reviewed By: vrana

CC: aran, epriestley, demo

Maniphest Tasks: T853

Differential Revision: https://secure.phabricator.com/D1597
2012-02-08 13:42:48 -08:00
vrana
8482569a47 Add line link to Paste
Summary: Better something than nothing.

Test Plan: View paste, click on line number.

Reviewers: codeblock, epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1596
2012-02-08 10:54:57 -08:00
epriestley
a1c20638fa Add very very basic reporting to Maniphest
Summary: Rough cut for Quora, we want this too eventually but it's super basic
right now so I'm not linking it anywhere. Once we get a couple more iterations
I'll put it in the UI.

Test Plan: Looked at stats for test data.

Reviewers: btrahan

Reviewed By: btrahan

CC: anjali, aran, epriestley

Differential Revision: https://secure.phabricator.com/D1594
2012-02-08 09:47:14 -08:00
epriestley
7a5ec015d9 Split "Create Another Task" button into "Similar Task" and "Empty Task"
Summary: Looping on this interface is pretty useful but you don't always want to
keep the projects/owners.

Test Plan: Clicked both buttons.

Reviewers: btrahan

Reviewed By: btrahan

CC: anjali, aran, epriestley

Differential Revision: https://secure.phabricator.com/D1593
2012-02-08 09:44:22 -08:00
epriestley
3f46d30e8f Replace home directory list with a dashboard
Summary:
Rough cut that still needs a lot of polish, but replace the directory list with
more of a dashboard type thing:

  - Show "Unbreak Now", triage-in-your-projects, and other stuff that you're
supposed to deal with, then feed.
  - Move tools a click a way behind nav -- this also lets us put more stuff
there and subtools, etc., later.
  - Remove tabs.
  - Merge the category/item editing views.
  - I also added a light blue wash to the side nav, not sure if I like that or
not.

Test Plan:
  - Viewed all elements in empty and nonempty states.
  - Viewed applications, edited items/categories.

Reviewers: btrahan, aran

Reviewed By: btrahan

CC: aran, epriestley, davidreuss

Maniphest Tasks: T21, T631

Differential Revision: https://secure.phabricator.com/D1574
2012-02-07 16:04:48 -08:00
vrana
3e3f73235c Break all old differential comment anchors
Test Plan: Display revision, verify text, click on links.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Maniphest Tasks: T845

Differential Revision: https://secure.phabricator.com/D1591
2012-02-07 15:19:39 -08:00
epriestley
4caa684724 Simplify Project status field
Summary:
This was a sort of speculative feature added by a contributor some time ago and
just serves as a label; for now, simplify it into "active" and "archived" and
remove "archived" projects from the "active" list.

  - Fix a bug where we'd publish a "renamed from X to X" transaction that had no
effect.
  - Publish stories about status changes.
  - Remove the "edit affiliation" controller, which has no links in the UI
(effectively replaced by join/leave links).
  - Add query/conduit support.

Test Plan: Edited the status of several projects.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran, epriestley

Maniphest Tasks: T681

Differential Revision: https://secure.phabricator.com/D1573
2012-02-07 14:59:38 -08:00
epriestley
a5f8846f47 Use a unique random key to identify queries, not a sequential ID
Summary:
We save search information and then redirect to a "/search/<query_id>/" URI in
order to make search URIs short and bookmarkable, and save query data for
analysis/improvement of search results.

Currently, there's a vague object enumeration security issue with using
sequential IDs to identify searches, where non-admins can see searches other
users have performed. This isn't really too concerning but we lose nothing by
using random keys from a large ID space instead.

  - Drop 'authorPHID', which was unused anyway, so searches can not be
personally identified, even by admins.
  - Identify searches by random hash keys, not sequential IDs.
  - Map old queries' keys to their IDs so we don't break any existing bookmarked
URIs.

Test Plan: Ran several searches, got redirected to URIs with random hashes from
a large ID space rather than sequential integers.

Reviewers: arice, btrahan

Reviewed By: arice

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1587
2012-02-07 14:58:46 -08:00
epriestley
4ac29d108c Simplify Aphront transaction code
Summary:
In D1515, I introduced some excessively-complicated semantics for detecting
connections that are lost while transactional. These semantics cause us to
reenter establishConnection() and establish twice as many connections as we need
in the common case.

We don't need a hook there at all -- it's sufficient to throw the exception
rather than retrying the query when we encounter it. This doesn't have
reentrancy problems.

Test Plan:
  - Added some encapsulation-violating hooks and a unit test for them
  - Verified we no longer double-connect.

Reviewers: btrahan, nh

Reviewed By: btrahan

CC: aran, epriestley

Maniphest Tasks: T835

Differential Revision: https://secure.phabricator.com/D1576
2012-02-07 14:58:37 -08:00
epriestley
f19d5fbeae Fix existence test in "reconcile.php"
Summary:
"rev-parse --verify" is actually a terrible test, it only survived my test cases
because I put garbage into the tables with RAND() or similar, not
properly-formatted garbage.

Use "cat-file -t" instead to ensure we perform an object-existence test.

Test Plan: Ran "reconcile.php P" locally, ran "cat-file -t" and "rev-parse
--verify" on properly-formatted but invalid hashes like
"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", worked with @rguerin to resolve
commit issues.

Reviewers: btrahan, rguerin

Reviewed By: rguerin

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1590
2012-02-07 10:46:26 -08:00
epriestley
47631530a5 Remove admin requirement from MetaMTASendGridReceiveController
Summary:
This got caught in the crossfire when we admin-only'd the whole MetaMTA tool. It
should not be admin only.

(Generally, we should probably separate this out better at some point.)

Test Plan: Hit /mail/sendgrid/ as a logged-out, non-admin user (like SendGrid
does).

Reviewers: s, btrahan

Reviewed By: s

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1588
2012-02-06 19:31:20 -08:00
vrana
18ba5fa0ad Separate field for branch in revision
Summary:
The main purpose of this change is to allow selecting the branch by
triple-click.
Plus it is not perfectly clear that the text in brackets means branch.

Test Plan: Display revision.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1585
2012-02-06 17:28:46 -08:00
epriestley
e0c38b0644 Obvious: emit each header once, not the last header N times. 2012-02-06 13:14:17 -08:00
epriestley
36e72639de Reduce visibility of "Host" and "Path" Differential fields by default
Summary:
See discussion in T838. These fields expose information which it isn't necessary
or useful to expose in the general case.

  - Disable fields by default, allow them to be enabled in config (these fields
were useful for me at Facebook when I had access to all the machines).
  - Remove 'sourcePath' from Conduit methods other than differential.query.
  - Condition 'sourcePath' field in Conduit on the caller being the revision
author. This is a bit hacky but not so awful.

Test Plan:
  - Verified fields are gone by default and restored by configuration.
  - Verified Conduit no longer returns these fields other than
differential.query.
  - Verified field presence/absence according to authorship in
differential.query.
  - Grepped around in arcanist to make sure we aren't relying on sourcePath.
There's a workflow in "arc merge" that technically might hit it, but I think
it's unreachable, definitely irrelvant (we never use source path as a
distinguisher under git/hg, and can't 'arc merge' in SVN) and it's going away
Real Soon Now anyway.

Reviewers: btrahan, arice

Reviewed By: arice

CC: aran, epriestley

Maniphest Tasks: T838

Differential Revision: https://secure.phabricator.com/D1582
2012-02-06 12:14:07 -08:00
vrana
15f6216634 Fix displaying of raw files in Differential
Summary:
See D1533#5.
Also deduplicates logic of what is stored to blob in ArcanistDiffWorkflow.

Blame Rev: D1533

Test Plan:
Display raw version of text file.
Display raw version of image.

Reviewers: epriestley, nh

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1583
2012-02-06 12:05:37 -08:00
vrana
d65f62d055 Use constants in DifferentialRevisionUpdateHistoryView
Test Plan: Display diff with whitespace changes.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1581
2012-02-06 11:24:06 -08:00
vrana
1ab2a88605 Reorganize escaping in DifferentialRevisionUpdateHistoryView
Summary:
Escaped $id is compared with non-escaped $max_id.
Escaped $id is escaped again in phutil_render_tag().

Note: $id is numeric :-).

Test Plan: Display diff.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1580
2012-02-06 11:22:43 -08:00
vrana
4ee714d404 Use hsprintf() in lint message
Test Plan: Display revision with lint problems

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1578
2012-02-06 10:10:13 -08:00
epriestley
e8a7d8a905 Provide software protections for HTTP response splitting
Summary:
This addresses a few things:

  - Provide a software HTTP response spliting guard as an extra layer of
security, see http://news.php.net/php.internals/57655 and who knows what HPHP/i
does.
  - Cleans up webroot/index.php a little bit, I want to get that file under
control eventually.
  - Eventually I want to collect bytes in/out metrics and this allows us to do
that easily.
  - We may eventually want to write to a socket or do something else like that,
ala Litespawn.

Test Plan:
  - Ran unit tests.
  - Browsed around, checked headers and HTTP status codes.

Reviewers: btrahan, vrana

Reviewed By: btrahan

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1564
2012-02-06 09:59:34 -08:00
vrana
be424bf381 Utilize hsprintf() in OAuth
Test Plan: /oauth/facebook/login/?error=<a>

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1577
2012-02-04 21:14:06 -08:00
vrana
7a0337054b Add Owners search to Diffusion
Test Plan:
/diffusion/X/browse/:/file
Click Search Owners.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1572
2012-02-04 10:23:36 -08:00
vrana
0f2d2201ce Search owners by repository
Summary: I've chosen passing callsign even if it is more complicated because it
is nicer than PHID and can be written by hand.

Test Plan:
Search without repository.
Search with repository.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1571
2012-02-04 10:22:38 -08:00
vrana
6cb5db60d5 Display compatible inline comments (usually synthetic) on the same row
Summary:
This just looks silly:
{F8088, size=full}

It runs in O(N*N) but it's not a big deal because there are usually only few
comments per line.
I didn't implement it for images.

Test Plan:
View revision with compatible inline comments in two diffs.
View revision with different inline comments on same line in two diffs.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1570
2012-02-04 10:20:37 -08:00
vrana
5e58a016a5 Allow full anchors in remarkup object names
Summary: Remarkup object names require #1 for linking to comments which is not
very intuitive.

Test Plan:
  D1558#4e01328c
  D1558#1
  D1558#comment-1

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1565
2012-02-03 15:50:19 -08:00
epriestley
de7aa2186c Resolve great internal confusion about left vs right inline comments
Summary:
This code was just all kinds of wrong, but got all the common cases anyone cares
about correct.

  - In edit-inline-comments.js, if isOnRight() is true, use data.right, not
data.left (derp).
  - Set data.left correctly, not to the same value as data.right (derp derp).
  - Set "isNewFile" based on $is_new, not $on_right (derp derp derp).

Test Plan:
 - Added JS debugging code to print "OLD" vs "NEW" and "LEFT" vs "RIGHT".
Clicked the left and right sides of diff-vs-base and diff-vs-diff diffs,
verified output was accurate in all cases.
 - Added comments to the left-display-side of a diff-of-diffs, saved them, they
showed up where I put them.

Reviewers: btrahan, vrana

Reviewed By: btrahan

CC: aran, epriestley

Maniphest Tasks: T543

Differential Revision: https://secure.phabricator.com/D1567
2012-02-03 15:26:47 -08:00
Bob Trahan
e15b3fc6f3 Clean up initialization of Differential Show More Behavior in Maniphest
Summary:
add a static variable to the method and use it so we don't init more
than once!

Test Plan:
add a "phlog" and noted only init'd one time.   verified "show more"
links worked correctly.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Maniphest Tasks: T666

Differential Revision: https://secure.phabricator.com/D1553
2012-02-03 13:58:58 -08:00
vrana
2ff36b7cad Move tail to <body>
Summary:
Prevents invalid HTML.
Discussion at D1561#6.

Test Plan:
http://validator.w3.org/check?uri=https%3A%2F%2Fsecure.phabricator.com

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1563
2012-02-03 11:33:38 -08:00
vrana
6bd8542abb Avoid sending CSRF token in GET and external forms
Summary:
Sending CSRF token in GET forms is dangerous because if there are external links
on the target page then the token could leak through Referer header.
The token is not required for anything because GET forms are used only to
display data, not to perform operations.
Sending CSRF tokens to external URLs leaks the token immediately.

Please note that <form action> defaults to GET.

PhabricatorUserOAuthSettingsPanelController suffered from this problem for both
reasons.

Test Plan: Save my settings (POST form).

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1558
2012-02-03 10:58:51 -08:00
vrana
c0efecb561 Specify encoding in <meta>
Summary: Phabricator sends information about encoding in Content-Type header but
when I save the HTML page then this information is lost.

Test Plan: /

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1561
2012-02-03 10:16:12 -08:00
vrana
b7e0f97d33 Use GET form in Owners search
Summary:
The form doesn't perform any action, only displays data.
URL wouldn't exceed its maximum length.
Bookmarking results can be useful.
Linking from other pages can be even more useful.
Also browsing through history is more fluent.

Test Plan:
Search by path.
Search by owner.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1559
2012-02-03 09:32:31 -08:00
vrana
67ea5a0b61 Search by full path in Owners
Summary:
Search tool currently allows only searching by substring which is useful.
But searching by the full path (for packages in which the path is contained) is
probably even more useful.

NOTE: I used a trick to perform the search by superstring.

Packages containing path '/' are found always which could seem strange but it is
correct after all.

Test Plan:
Search for /src/x/a.php. Found package with path /src/x/.
Search for /src/. Found package with path /src/x/.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1560
2012-02-03 09:32:12 -08:00
vrana
17b6649552 Add link to path in Owners package list
Summary: Also changes rCALL in package detail to bold CALL.

Test Plan:
/owners/view/all/, click on link
/owners/package/1/

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1550
2012-02-02 18:36:19 -08:00
vrana
fe4d717cc7 Escape result of PhabricatorOAuthProvider::getProviderName()
Test Plan: /settings/page/facebook/

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1556
2012-02-02 18:35:45 -08:00
vrana
339369dc36 Github is actually GitHub
Test Plan: none

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1557
2012-02-02 17:47:04 -08:00
David Reuss
c3281c8757 Add conduit feed.query method
Summary: This is not totally done yet, and i'm submitting for feedback.

Test Plan: Played with various settings in local conduit console.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Maniphest Tasks: T790

Differential Revision: https://secure.phabricator.com/D1555
2012-02-02 16:44:13 -08:00
David Reuss
8df14b8505 Add remarkup.process conduit method
Summary: This exposes a few remarkup engines over conduit.

Test Plan:
Local conduit console, and playing with

  'cat example.json | arc call-conduit remarkup.process'

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1551
2012-02-02 16:43:37 -08:00
epriestley
a15130b47c Add a maintenance script for reconciling repositories to disk state
Summary:
@rguerin ran into an issue in his install where Phabricator appears to have
discovered commits which no longer exist, and thus is failing to proceed with
its repository import.

It's not clear how we got into this state. Previously, it was possible by, e.g.,
parsing a different repository's working copy and then switching them back, but
there are now safeguards against that.

I'm taking a three-pronged approach to try to sort this out:

  - Provide a script to get out of this state (this script) and reconcile
Phabricator's view of a repository with an authoritative copy of it. This
basically "un-discovers" any discovered commits which don't actually exist (any
queued tasks to parse them will fail permanently when they fail to load the
commit object).
  - Add more logging to the discovery daemon so we can figure out where commits
came from.
  - Improve Diffusion's UI when stuff is partially discovered (T776).

(This script should also clean up some nonsense on secure.phabricator.com from a
botched Diviner import.)

Test Plan: Ran "reconcile.php" with bogus commits and bogus differential/commit
links, had them expunged. Will work with @rguerin to see if this resolves
things.

Reviewers: btrahan, rguerin

Reviewed By: btrahan

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1552
2012-02-02 16:03:50 -08:00
epriestley
c86dfd84d9 Wrap basic diff/revision association in a transaction
Summary:
This doesn't cover every case exhaustively (see comments) but should cover like
98% of the practical cases.

This makes one workflow modification: willWriteRevision() was previously
guaranteed to have a revisionID / revisionPHID and no longer is. I verified that
no field implementations depend on this behavior. Fields which depend on IDs
should be using didWriteRevision() instead.

Test Plan: Inserted a "throw" into the middle of the transactions and created
revisions; they didn't orphan. Created revisions normally, they worked
correctly.

Reviewers: btrahan, nh

Reviewed By: btrahan

CC: aran, epriestley

Maniphest Tasks: T605

Differential Revision: https://secure.phabricator.com/D1541
2012-02-02 16:03:44 -08:00
epriestley
dc36317ea4 Use 'ps <pid>' to test for process existence if posix is not available
Summary: posix may not be loaded on the web/cgi SAPI but we call posix functions
on this pathway, which we hit on /daemon/. Fall back to exec if we don't have
posix.

Test Plan: Added "&& false" and verified the page executed a bunch of "ps"
tests.

Reviewers: Koolvin, btrahan

Reviewed By: btrahan

CC: aran, epriestley

Maniphest Tasks: T821

Differential Revision: https://secure.phabricator.com/D1540
2012-02-02 16:03:36 -08:00
vrana
33fb7117ae XSS in Owners
Test Plan: Display /owners/view/search/ for repository with callsign <i>hack</i>

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1549
2012-02-02 12:31:23 -08:00
Nick Harper
c3543c80cd Fix conduit host check
Summary:
conduit was using getProductionURI instead of getURI for checking that the
request was sent to the correct host, which causes problems in some dev
environments

Test Plan:
  echo '{}' | arc call-conduit --conduit-uri=mydevserver

where my dev server is configured with phabricator.production-uri pointing to
prod instead of my devserver

Reviewers: epriestley, btrahan, jungejason

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1543
2012-02-02 11:38:14 -08:00
vrana
82c6f275bd Add links to empty result in Diffusion
Summary: I think that these are the only links that are useful - commit which
deleted the path and last version of the path.

Test Plan: Display deleted path, click on links.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1536
2012-02-01 11:57:48 -08:00
epriestley
29337a9b3a Minor, add "cov" class to inline comment <td /> so new files with lint in them do not go crazy. 2012-01-31 18:30:22 -08:00