Summary: Do JOIN in the protocol handler, after we receive 376 ("end of motd").
Test Plan: Ran bot, it joined a channel after receieving a 376 command.
Reviewers: moos3, codeblock, aran, jungejason, tuomaspelkonen
Reviewed By: moos3
CC: aran, moos3
Differential Revision: 855
"user":"authenticationusername",
"pass":"thisuserspassowrd",
This will allow people with internal irc servers to use this if they control access from ldap for irc.
domain
Summary:
See D758, D759.
- Provide a strongly recommended setting which permits configuration of an
alternate domain.
- Lock cookies down better: set them on the exact domain, and use SSL-only if
the configuration is HTTPS.
- Prevent Phabriator from setting cookies on other domains.
This assumes D759 will land, it is not effective without that change.
Test Plan:
- Attempted to login from a different domain and was rejected.
- Logged out, logged back in normally.
- Put install in setup mode and verified it revealed a warning.
- Configured an alterate domain.
- Tried to view an image with an old URI, got a 400.
- Went to /files/ and verified links rendered to the alternate domain.
- Viewed an alternate domain file.
- Tried to view an alternate domain file without the secret key, got a 404.
Reviewers: andrewjcg, erling, aran, tuomaspelkonen, jungejason, codeblock
CC: aran
Differential Revision: 760
Summary:
We don't currently validate CSRF tokens on this workflow. This allows an
attacker to upload arbitrary files on the user's behalf. Although I believe the
tight list of servable mime-types means that's more or less the end of the
attack, this is still a vulnerability.
In the long term, the right solution is probably to pass CSRF tokens on all Ajax
requests in an HTTP header (or just a GET param) or something like that.
However, this endpoint is unique and this is the quickest and most direct way to
close the hole.
Test Plan:
- Drop-uploaded files to Files, Maniphest, Phriction and Differential.
- Modified CSRF vaidator to use __csrf__.'x' and verified uploads and form
submissions don't work.
Reviewers: andrewjcg, aran, jungejason, tuomaspelkonen, erling
Commenters: andrewjcg, pedram
CC: aran, epriestley, andrewjcg, pedram
Differential Revision: 758
Summary:
Provide a {T123} syntax which pulls in the entire name of an object, not just a
link to it. A major use for this is organizing projects using wiki pages. Since
handle links show object status now, this lets you organize stuff in an ad-hoc
way and get a reasonable overview of it. We can make handles richer in the
future, too.
The performance on this isn't perfect (it adds some potential single gets) but I
think it's okay for now and I don't want to make remarkup engine even more
complex until the preprocess/postprocess stuff has had a chance to settle and
I'm more confident it works.
In Differential and Maniphest we'll also incorrectly cache the object
state/name, but that'll fix itself once I move the cache code to use
preprocess/postprocess correctly.
Test Plan:
- See https://secure.phabricator.com/file/view/PHID-FILE-5f9ca32407bec20899b9/
for an example.
- Generated and looked over the documentation.
Reviewed By: jungejason
Reviewers: jungejason, tuomaspelkonen, aran, hunterbridges
CC: skrul, aran, jungejason, epriestley
Differential Revision: 784
Summary:
This accomplishes two goals:
- Data fetching is now grouped across blocks.
- Demonstrates that D737 actually works.
Test Plan: Used @mentions in Phriction preview, they rendered properly. Verified
only one service call was being made across blocks.
Reviewed By: jungejason
Reviewers: hunterbridges, jungejason, tuomaspelkonen, aran
CC: aran, jungejason, epriestley
Differential Revision: 738
query
Summary:
- Provide an example unit test, and document it.
- Document database isolation better.
- When we issue an unsimulated query to the isolated connection, throw a
helpful message.
- Pygments is complaining about my madeup "lang=demo", change it to
"lang=text".
Test Plan:
- Ran the unit test (sanity check).
- Ran all other unit tests (verify I didn't break isolation).
- Added a queryfx(..., 'SELECT 1') to a test and verified it throws.
- Read the documentation.
Reviewed By: edward
Reviewers: edward, jungejason, tuomaspelkonen, aran
CC: aran, edward
Differential Revision: 773
Summary:
This gets all the major pieces working. Allows you to drag-and-drop files in
Differential and Phriction, and embed files in remarkup with {Fxxx} references.
See also task.
I'm explicitly not documenting this yet since it's still pretty rough.
Test Plan: Dragged and dropped stuff into Differential and Phriction.
Reviewed By: jungejason
Reviewers: jungejason, tuomaspelkonen, aran, tomo
Commenters: tomo
CC: aran, tomo, jungejason
Differential Revision: 674
Summary: This has to table scan a ginormous table right now, give it a fighting
chance with a more usable key.
Test Plan:
- Launched GC daemon, no errors.
- Used test console to create a new transcript.
- Viewed some old transcripts.
- Ran EXPLAIN on the SELECT and verified it was utilizing the garbageCollected
key.
Reviewed By: jungejason
Reviewers: jungejason, tuomaspelkonen, aran
CC: aran, epriestley, jungejason
Differential Revision: 735
Summary: This didn't end up getting used but I neglected to delete it.
Test Plan: git grep
Reviewed By: jungejason
Reviewers: tuomaspelkonen, jungejason, aran
CC: aran, jungejason
Differential Revision: 721
problems
Summary: Detect more PHP misconfigurations in setup.
Test Plan: Broke my configuration, ran setup, it seemed to detect all the
problems and issue meaningful error messages.
Reviewed By: jungejason
Reviewers: hunterbridges, 10098, jungejason, tuomaspelkonen, aran
CC: aran, jungejason
Differential Revision: 717
Summary:
See T344. Currently, there's a hard-coded 12MB filesize limit and some awkward
interactions with MySQL's max_allowed_packet. Make this system generally more
robust:
- Move the upload limit to configuration.
- Add setup steps which reconcile max_allowed_packet vs MySQL file storage
limits.
- Add a layer of indirection between uploading files and storage engines.
- Allow the definition of new storage engines.
- Define a local disk storage engine.
- Add a "storage engine selector" class which manages choosing which storage
engines to put files in.
- Document storage engines.
- Document file storage classes.
Test Plan:
Setup mode:
- Disabled MySQL storage engine, misconfigured it, configured it correctly.
- Disabled file storage engine, set it to something invalid, set it to
something valid.
- Verified max_allowed_packet is read correctly.
Application mode:
- Configured local file storage.
- Uploaded large and small files.
- Verified larger files were written to local storage.
- Verified smaller files were written to MySQL blob storage.
Documentation:
- Read documentation.
Reviewed By: jungejason
Reviewers: jungejason, tuomaspelkonen, aran
CC: aran, epriestley, jungejason
Differential Revision: 695
Summary:
Share code with the new PhabricatorDifferenceEngine, which handles diffs with no
changes correctly.
(This isn't the same issue as file moves, but I ran into it while generating a
repro case.)
Test Plan: Previously, changes which didn't change file content (e.g., property
changes) would throw. Now they work.
Reviewed By: tuomaspelkonen
Reviewers: tuomaspelkonen, jungejason, aran
CC: aran, epriestley, tuomaspelkonen
Differential Revision: 698
Summary:
Single brackets are getting some troublesome false positives in Facebook's
install. Particularly, there's a weird convention at Facebook of tagging diffs
by putting stuff like "[perf]" or "[chat]" in the title, although this isn't
turned into structured data at any stage. When commits appear in Diffusion, we
currently link such ad-hoc tags to Phriction.
Wikipedia uses double-bracket sytnax, as do many other wikis, so this seems like
a reasonable burden to place on the lightweightness of the markup. The
alternative is selectively disabling Phriction markup in some interfaces, but
I'd rather allow integration in commit messages and just guard the syntax more
closely.
(I'm not providing any sort of migration plan since this landed less than a week
ago and I'm pretty confident no one has built a huge wiki yet, but I added a
CHANGELOG note.)
Test Plan: Edited a wiki document and added some links. Verified single brackets
were unlinked and double brackets were linked.
Reviewed By: jungejason
Reviewers: hsb, aran, jungejason, tuomaspelkonen
CC: aran, jungejason, epriestley
Differential Revision: 689
Summary:
This is really rough and needs work (particularly, there's some diff code I
really need to refactor since I sort-of-copy-pasted it) but basically
functional.
Show text changes between diffs and allow users to revert to earlier versions.
Differential's line-oriented diff style isn't ideal for large blocks of text but
I'm betting this is probably good enough in most cases. We can see how bad it is
in practice and then fix it if needbe.
I added a bunch of support for "description" but didn't add the feature in this
diff, I'll either follow up or task it out since it should be a pretty
straightforward change.
Test Plan: Looked at history for several Phriction documents, clicked "previous
change" / "next change", clicked revert buttons.
Reviewed By: hsb
Reviewers: hsb, codeblock, jungejason, tuomaspelkonen
CC: aran, hsb, epriestley
Differential Revision: 687
Summary:
- Exceptions on the rendering pathway currently go uncaught and result in a
blank page. Commonly, this is a bad require_celerity_resource() call. Although
we can't safely render a page if the rendering pathway is broken, we can show a
useful message.
- When PHP exits because of a fatal error, there is an opportunity to run code
in the shutdown handler. This allows us to show messages at least some of the
time, e.g. "call to unknown function derp() in somefile.php at line 99"
- flip dem tables
Test Plan: Added fatals ("derp();") and rendering exceptions
("require_celerity_resource('does-not-exist')") to a controller and verified
that the error handling behavior is now more useful.
Reviewed By: aran
Reviewers: jungejason, tuomaspelkonen, aran
CC: aran, epriestley
Differential Revision: 680
Summary: Document linking and some general layout improvements. I'd like to
eventually do more meta-dataey things with links (like store them separately and
check them for 404s) but this is a decent start.
Test Plan:
https://secure.phabricator.com/file/view/PHID-FILE-d756b94a06b69c273fce/
Reviewed By: jungejason
Reviewers: hsb, codeblock, jungejason, tuomaspelkonen, aran
CC: aran, jungejason, epriestley
Differential Revision: 650
Summary:
A bunch of the .sql patch files don't explicitly specify the table engine, but
we should always use InnoDB with the exception of one table which needs MyISAM
for FULLTEXT.
MySQL doesn't no-op an ALTER TABLE statment that changes the engine back to
itself and converting large tables can be time consuming, so convert only the
required tables.
Test Plan: Ran on secure.phabricator.com and my local box, it fixed all the
issues in about 3 seconds on secure.phabricator.com and <<1 second on my local.
Reviewed By: codeblock
Reviewers: codeblock, tuomaspelkonen, jungejason, aran
CC: aran, epriestley, codeblock
Differential Revision: 641
Summary:
If we're going to hardcode a path, at least let's do it in a way that works on RHEL too.
Test Plan:
Successfully ran the setup script on RHEL.
Reviewers:
epriestley
CC:
Differential Revision: 623
Summary:
Add the differential parse cache to the GC. This is the largest object in the
system by a wide margin, I think.
This table is potentially gigantic which is why the script truncates it before
doing a schema change.
Test Plan: Ran the GC daemon, it cleaned up some parse caches.
Reviewed By: jungejason
Reviewers: tuomaspelkonen, jungejason, aran
Commenters: tuomaspelkonen
CC: aran, jungejason, tuomaspelkonen, epriestley
Differential Revision: 620
Summary:
Now that we store language with pastes, we can include this in Conduit.
Test Plan:
Tried it on a paste with a blank language, and one with a specified language.
16:14:50 <@CodeBlock> P1
16:14:51 <@codeblock-phabot> P1: http://phabricator.local/P1 - test.php
16:15:05 <@CodeBlock> P43
16:15:06 <@codeblock-phabot> P43: http://phabricator.local/P43 - sadoijfoisaf (php)
Reviewers:
epriestley, Ttech
CC:
Differential Revision: 616
Summary:
Phabricator generates a bunch of data that we don't need to keep around forever,
add a GC daemon to get rid of it with some basic configuration options.
This needs a couple more diffs to get some of the details but I think this is a
reasonable start.
I also fixed a couple of UI things related to this, e.g. the daemon logs page
going crazy when a daemon gets stuck in a loop and dumps tons of data to stdout.
Test Plan:
- Ran gc daemon in 'phd debug' mode and saw it delete stuff, then sleep once
it had cleaned everything up.
- Mucked around with TTLs and verified they work correctly.
- Viewed gc'd transcripts in the web interface and made sure they displayed
okay.
- Viewed daemon logs before/after garbage collection.
- Running some run-at / run-for tests now, I'll update if the daemon doesn't
shut off in ~10-15 minutes. :P
Reviewed By: tuomaspelkonen
Reviewers: jungejason, tuomaspelkonen, aran
CC: aran, tuomaspelkonen, epriestley
Differential Revision: 583
Summary: When the remote closes the connection, phabot goes into a busy loop
because of PHP's "nothing should ever be an error" semantics. Instead, detect
connection termination.
Test Plan: Disabled the "PONG" response in the protocol handler and let freenode
disconnect phabot. It spun out of control before, now it detects the issue and
exits to await automatic restart.
Reviewed By: jungejason
Reviewers: codeblock, jungejason, aran, tuomaspelkonen
CC: aran, jungejason
Differential Revision: 582
Summary:
Summarize (or at least link to) pastes when P<id> is given in a channel. This depends on D552!
Test Plan:
04:34:03 <@codeblock-phabot> P15 - http://phabricator.local/P15
04:35:22 <@CodeBlock> P1 asdf foo Bar Baz Diet Sprite P4
04:35:24 <@codeblock-phabot> P1 - http://phabricator.local/P1
04:35:24 <@codeblock-phabot> P4 - http://phabricator.local/P4
Reviewers:
epriestley
CC:
Differential Revision: 553
Summary:
Maniphest tasks can now be summarized by referencing their respective T<id> in irc.
Test Plan:
03:29:28 <@CodeBlock> T218
03:29:29 <@codeblock-phabot> T218: NYANCAT (Priority: Wishlist) - https://secure.phabricator.com/T218
03:30:14 <@CodeBlock> T58 asdf foo bar baz mcFoo T163
03:30:15 <@codeblock-phabot> T58: Allow AphrontFormView to render as a <div /> instead of a form (Priority: Wishlist) - https://secure.phabricator.com/T58
03:30:15 <@codeblock-phabot> T163: arc amend is slow (Priority: Needs Triage) - https://secure.phabricator.com/T163
...
03:28:23 <@CodeBlock> Oh. Hah, I'm testing against production Conduit. Ah well.
03:28:40 <@CodeBlock> I don't always test my code...but when I do, I do it in prod! :D
Reviewers:
epriestley
CC:
Differential Revision: 551
Summary:
When a user gets @mentioned in Differential, add them as a CC.
No Maniphest hookup yet since I want to make that one a little more formal.
Depends on D518.
Test Plan:
@mentioned a user and they were added as a CC.
Reviewed By: jungejason
Reviewers: tomo, mroch, jsp, jungejason, aran, tuomaspelkonen
CC: aran, jungejason
Differential Revision: 519
Summary:Make a new directory, src/infrastructure/markup/remarkup/markuprule/paste/
Make a new class called PhabricatorRemarkupRulePaste in that directory.
Add the rule to DifferentialMarkupEngineFactory.
Test Plan: Created a task in maniphest. Put P1 and P2 in the content.
Created P1 and P2 in Paste. Verified P1 and P2 were highlighted and
linked correctly.
Reviewers:epriestley, codeblock
CC:jungejason
Differential Revision: 539
Summary:
The web console already correctly checks for processes which are running but
can't be signaled. Share this check in the CLI.
Test Plan:
Looked at the web console. Poked at the CLI.
Reviewed By: fratrik
Reviewers: fratrik
CC: aran, fratrik
Differential Revision: 544
Summary:
Currently we send logs to the logfile in 'phd debug', but we should send them to
the console instead.
Also fixed some %C stuff which could theoretically cause problems if a user had
percentage symbols in their paths (heaven forbid).
fratrik, this or D535 might have been involved in frustrating your efforts to
debug the "sudo" stuff.
Test Plan:
Ran "phd debug irc derpderp" and "phd launch irc derpderp". In the former case,
the exception appeared in the console. In the latter, it appeared in the log.
Reviewed By: codeblock
Reviewers: codeblock, jungejason, tuomaspelkonen, aran, fratrik
CC: aran, codeblock
Differential Revision: 536
Summary:
Provides basic Remarkup support for @mentions. No application integration yet so
these aren't terribly useful until that happens.
Test Plan:
https://secure.phabricator.com/file/view/PHID-FILE-83d68e7af6085ae928df/
Reviewers: tomo, mroch, jsp
Commenters: tomo
CC: aran, tomo, epriestley
Differential Revision: 517
Summary:
The MySQL MyISAM fulltext backend defaults to a minimum length of 4, but many
3-letter words are useful search terms. Provide instructions for configuring
MySQL to reduce the auto-stopword length.
Test Plan:
Followed instructions, searched for a 3-letter term and got a hit.
Reviewed By: jungejason
Reviewers: aran, tuomaspelkonen, jungejason
CC: aran, jungejason
Differential Revision: 471
Summary:
Tweaks to the paste app:
- I realized that unlike all the other apps, it makes more sense for the
default view of this one to be "create paste" instead of "list pastes" since
when you access the application directly you are most often wanting to share
something. Swap list out of the default slot and make edit the default.
- Make the textarea bigger (usability).
- Allow you to copy an existing paste.
- Implement 'raw view'.
- Tweak/adjust list view (usability, formatting).
- Tweak page titles.
Test Plan:
Created, copied, and listed pastes. Viewed raw paste. Created an invalid paste.
Tried to create a copy of a nonexistant paste.
Reviewed By: codeblock
Reviewers: codeblock, jungejason, aran, tuomaspelkonen
CC: aran, epriestley, codeblock
Differential Revision: 456
