Summary:
Ref T12509. This upgrades a `weakDigest()` callsite to SHA256-HMAC and removes three config options:
- `celerity.resource-hash`: Now hard-coded, since the use case for ever adjusting it was very weak.
- `celerity.enable-deflate`: Intended to make cache inspection easier, but we haven't needed to inspect caches in ~forever.
- `celerity.minify`: Intended to make debugging minification easier, but we haven't needed to debug this in ~forever.
In the latter two cases, the options were purely developer-focused, and it's easy to go add an `&& false` somewhere in the code if we need to disable these features to debug something, but the relevant parts of the code basically work properly and never need debugging. These options were excessively paranoid, based on the static resource enviroment at Facebook being far more perilous.
The first case theoretically had end-user utility for fixing stuck content caches. In modern Phabricator, it's not intuitive that you'd go adjust a Config option to fix this. I don't recall any users ever actually running into problems here, though.
(An earlier version of this change did more magic with `celerity.resource-hash`, but this ended up with a more substantial simplification.)
Test Plan: Grepped for removed configuration options.
Reviewers: amckinley
Reviewed By: amckinley
Maniphest Tasks: T12509
Differential Revision: https://secure.phabricator.com/D19941
Summary: Fixes T10806. Although browsers don't seem to care about this, it's more correct to support it, and the new test console uses normal `cURL` and does care.
Test Plan:
- Hit the error case for providing a chain but no key/cert.
- Used `openssl s_client -connect localhost:22280` to connect to local Aphlict servers.
- With SSL but no chain, saw `openssl` fail to verify the remote.
- With SSL and a chain, saw `openssl` verify the identify of the remote.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10806
Differential Revision: https://secure.phabricator.com/D15709
Summary: Ref T10697. Mostly straightforward. Also allow the server to have multiple logs and log options in the future (e.g., different verbosities or separate admin/client logs or whatever). No specific plans for this, but the default log is pretty noisy today.
Test Plan: Set up a couple of logs, started server, saw it log to them.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10697
Differential Revision: https://secure.phabricator.com/D15702
Summary: Ref T10697. This isn't everything but starts generalizing options and moving us toward a cluster-ready state of affairs.
Test Plan: Started server in various configurations, hit most (all?) of the error cases with bad configs, sent test notifications.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10697
Differential Revision: https://secure.phabricator.com/D15701
Summary:
Ref T2783. This is basically a more refined version of D10400, which churned a bit on things like SSH key storage, the actual way the signing protocol shook out, etc.
- When Phabricator tries to make an intra-cluster service call as the omnipotent user, sign it with the host's device key.
- Add `bin/almanac register` to say "this host is X device, identified by private key Y". This stores the keypair locally, adds the public key to Almanac, and trusts it.
Net effect is that once a host has been registered, the daemons can make calls to other nodes as the omnipotent user. This is primarily necessary so they can access repository API methods on remote hosts.
Test Plan:
- Ran `bin/almanac register` with various valid and invalid inputs.
- Verified keys get generated/added/stored properly.
- Made a device-signed cluster Conduit call.
- Made a normal old user-signed cluster Conduit call.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T2783
Differential Revision: https://secure.phabricator.com/D11158
Summary: just explicitly check if the file doesn't exist *first*, and then do the standard include thing with the more generic error if that doesn't work. Fixes T6255.
Test Plan: re-started apache and phabricator still worked; will ask csilvers to give it a whirl too
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T6255
Differential Revision: https://secure.phabricator.com/D10643
Summary:
Fixes T6230. These files have not been read by default for a long time, but users are frequently confused and try to edit `default.conf.php`.
Remove the actual files. Allow `phabricator_read_config_file(...)` to continue working as though they exist so as to not break config-file-based installs.
Test Plan:
I used this script to make sure that removing `default.conf.php` won't change things for installs which are still using config files:
```
<?php
require_once 'scripts/__init_script__.php';
$file = require 'conf/default.conf.php';
$global = new PhabricatorConfigDefaultSource();
$global_values = $global->getAllKeys();
foreach ($file as $key => $value) {
$global_value = idx($global_values, $key, (object)array());
if ($value !== $global_value) {
echo "{$key}\n\n";
echo "FILE VALUE\n";
var_dump($value);
echo "\n";
echo "DEFAULT VALUE\n";
var_dump($global_value);
return;
}
}
```
These were the keys that had issues:
- `log.access.format` Not specified in default.conf.php, safe to speciy.
- `mysql.pass` Empty string in file, null in global. Same effect.
- `metamta.default-addrress` One used `noreply@example.com`, one `noreply@phabricator.example.com`. These are just human-readable examples so it's safe to change behavior.
- `metamta.domain` same as above, `example.com` vs `phabricator.example.com`.
- `phpmailer.smtp-host` One used null, one empty string.
- `phpmailer.smtp-protocol` As above.
- `files.viewable-mime-types` File version is out of date.
- `repository.default-local-path` Null in file, set in global. This is correct to set to a default value now.
- `pygments.dropdown-choices` File version is out of date.
- `environment.append-paths` File version is empty, global version adds common paths. This //could// change behavior, but the web behavior is better and more reasonable in general, and a system would need to be configured in a very bizarre way for this to be relevant.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T6230
Differential Revision: https://secure.phabricator.com/D10628
Summary:
Fixes T6084. Changes:
- Rename `phabricator.show-beta-applications` to `phabricator.show-prototypes`, to reinforce that these include early-development applications.
- Migrate the config setting.
- Add an explicit "no support" banner to the config page.
- Rename "Beta" to "Prototype" in the UI.
- Use "bomb" icon instead of "half star" icon.
- Document prototype applications in more detail.
- Explicitly document that we do not support these applications.
Test Plan:
- Ran migration.
- Resolved "obsolete config" issue.
- Viewed config setting.
- Browsed prototypes in Applications app.
- Viewed documentation.
Reviewers: chad, btrahan
Reviewed By: btrahan
Subscribers: epriestley, hach-que
Maniphest Tasks: T6084
Differential Revision: https://secure.phabricator.com/D10493
Summary: I'm pretty sure that `@group` annotations are useless now... see D9855. Also fixed various other minor issues.
Test Plan: Eye-ball it.
Reviewers: #blessed_reviewers, epriestley, chad
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, Korvin, hach-que
Differential Revision: https://secure.phabricator.com/D9859
Summary: I could just add these options to my local configuration, but I figured I'd submit these upstream since they are (in my opinion) fairly common file formats.
Test Plan: N/A
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9319
Summary: After T2039, it makes sense to syntax highlight `.arclint` files as JSON.
Test Plan: N/A
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9305
Summary: A small but appreciable number of users find flavor on buttons confusing. Remove this flavor. This retains flavor in headers, error messages, etc., which doesn't cause confusion.
Test Plan: Looked at a revision, task, paste, macro, etc.
Reviewers: chad, btrahan
Reviewed By: btrahan
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D8812
Summary: I haven't been able to understand why this isn't set by default in production environments (since it is recommended to do so anyway).
Test Plan: N/A
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D8743
Summary: Ref T2222. These no longer have an effect, and are obsoleted by `differential.fields`.
Test Plan: `grep`
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2222
Differential Revision: https://secure.phabricator.com/D8468
Summary:
I was trying to set up a http hook, but despite setting the config,
the endpoint wasn't getting a request. I was advised on IRC by balpert to
restart my daemons and it worked great after I did that.
Since this information isn't in the documentation, I am adding it to the
description of the option, so it helps the next person.
Test Plan: N/A
Reviewers: #blessed_reviewers, epriestley
Reviewed By: epriestley
CC: Korvin, epriestley, aran, spicyj
Differential Revision: https://secure.phabricator.com/D8447
Summary: Ref T2222. This will probably have some rough edges for a bit (e.g., weird cases I didn't remember or think of), but there's no change to the underlying data and we can easily revert if things get too messy.
Test Plan: Looked at a variety of revisions and saw sensible output.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2222
Differential Revision: https://secure.phabricator.com/D8361
Summary:
Ref T4420. Tokenizers currently operate in "preload" or "ondemand" modes. In the former mode, which is default, they'll try to load the entire result list when a page loads.
The theory here was that this would slightly improve the experience for small installs, and once they got big enough they could switch to "ondemand". In practice, several issues have arisen:
- We generally don't have a good mechanism for telling installs that they should tweak perf config -- `metamta.send-immediately` is the canonical example here. Some large installs are probably affected negatively by not knowing to change this setting, and having settings like this is generally annoying.
- We have way way too much config now.
- With the advent of ApplicationSearch, pages like Maniphest make many redundant loads to prefill sources like projects. Most of the time, this data is not used. It's far simpler to switch everything to ondemand than try to deal with this, and dealing with this would mean creating two very complex divergent pathways in the codebase for a mostly theoretical performance benefit which only impacts tiny installs.
- We've been using `tokenizer.ondemand` forever on `secure.phabricator.com` since we have many thousands of user accounts, and it doesn't seem sluggish and works properly.
Removing this config is an easy fix which makes the codebase simpler.
I've retained the ability to use preloaded sources, since they may make sense in some cases (in at least one case -- task priorities -- adding a static source pathway might make sense), and they're part of Javelin itself. However, the code will no longer ever go down that pathway.
Test Plan: Used `secure.phabricator.com` for years with this setting enabled.
Reviewers: btrahan, chad
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T4420
Differential Revision: https://secure.phabricator.com/D8232
Summary:
Ref T4310. Fixes T3720. This change:
- Removes concurrent session limits. Instead, unused sessions are GC'd after a while.
- Collapses all existing "web-1", "web-2", etc., sessions into "web" sessions.
- Dramatically simplifies the code for establishing a session (like omg).
Test Plan: Ran migration, checked Sessions panel and database for sanity. Used existing session. Logged out, logged in. Ran Conduit commands.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T4310, T3720
Differential Revision: https://secure.phabricator.com/D7978
Summary:
Ref T3857.
- Always send mail via daemons. This lets us get rid of this config, and is generally much more performant.
- After D7964, we warn if daemons aren't running.
Test Plan: Sent some mail.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3857
Differential Revision: https://secure.phabricator.com/D7965
Summary: Ref T4222. This was used by Facebook while developing Releeph, but should no longer be necessary since Releeph is in the upstream. I can't get an answer out of Facebook about whether they still use it or not (see T4227), so nuke it. We're going to replace it with a more general mechanism (see T4222).
Test Plan: Regenerated celerity map. Browsed some pages, still got resources.
Reviewers: btrahan, hach-que
Reviewed By: hach-que
CC: aran
Maniphest Tasks: T4222
Differential Revision: https://secure.phabricator.com/D7863
Summary: We've had support for this for a long time, but it was conditional on config. Since it more-or-less actually does something now, just enable it unconditionally.
Test Plan: Settings -> SSH Public Keys
Reviewers: btrahan
Reviewed By: btrahan
CC: hach-que, aran
Maniphest Tasks: T2230
Differential Revision: https://secure.phabricator.com/D7426
Summary:
Gets rid of some old Differential-specific nonsense and replaces it with general runtime-pluggable Remarkup rules.
Facebook: This removes two options which may be in use. Have any classes being added via config here just subclass the new abstract bases instead. This should take 5 seconds to fix. You can adjust order by overriding `getPriority()` on the rules, if necessary.
Test Plan: See comments.
Reviewers: btrahan
Reviewed By: btrahan
CC: FacebookPOC, andrewjcg, aran
Differential Revision: https://secure.phabricator.com/D7393
Summary: Fixes T4001. I broke this some time ago and no one has complained. I don't think it gets much use, and we haven't added it for the newer apps. Just get rid of it rather than adapt the URIs for ApplicationSearch.
Test Plan: Unit tests, sent myself some email.
Reviewers: zeeg, btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T4001
Differential Revision: https://secure.phabricator.com/D7355
Summary: Some day we'll just delete this entirely.
Test Plan: loaded Phabricator
Reviewers: zeeg, btrahan
Reviewed By: zeeg
CC: aran, mbishopim3
Differential Revision: https://secure.phabricator.com/D7044
Summary: Ref T418. Maniphest has an obsolete class-based field selector. Replace it with CustomField-based selectors, which use the nice config UI and are generally way easier to use.
Test Plan: Added custom fields; edited and viewed custom fields on tasks. Everything worked as expected.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T418
Differential Revision: https://secure.phabricator.com/D6998
Summary:
Ref T1536.
- Move all the provider-specific help into contextual help in Auth.
- This provides help much more contextually, and we can just tell the user the right values to use to configure things.
- Rewrite account/registration help to reflect the newer state of the word.
- Also clean up a few other loose ends.
Test Plan: {F46937}
Reviewers: chad, btrahan
Reviewed By: chad
CC: aran
Maniphest Tasks: T1536
Differential Revision: https://secure.phabricator.com/D6247
Summary: Ref T1536. This is the last major migration. Moves us over to the DB and drops all the config stuff.
Test Plan:
- Ran the migration.
- Saw all my old config brought forward and respected, with accurate settings.
- Ran LDAP import.
- Grepped for all removed config options.
Reviewers: btrahan, chad
Reviewed By: btrahan
CC: aran, wez
Maniphest Tasks: T1536
Differential Revision: https://secure.phabricator.com/D6243
Summary: At the global level, truncate emails at a user-configured size.
Test Plan: Untested, as I could not get PHP to send emails on my box, but if you can this should be very easy to test. Just set the max size to something like .001 kilobytes and make sure it does the right thing.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin, AnhNhan
Maniphest Tasks: T1392
Differential Revision: https://secure.phabricator.com/D6118
Summary: Using PhabricatorExternalAccount in place maniphest.default-public-author.
Test Plan:
Using receivemail to see if the a new entry is made in the 'phabircator_user.user_externalaccount' table. Few things, I noticed that phabricator creates table 'user_externalaccout'. And now it throws up error 'Unknown column 'dateCreated' in 'field list''. Awaiting your comments.
{F41370}
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin, AnhNhan
Maniphest Tasks: T1205
Differential Revision: https://secure.phabricator.com/D5747
Summary: @edward, you would need to delete these options from FB config to avoid setup warning.
Test Plan: /T1
Reviewers: epriestley, edward
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5620
Summary:
Also join concepts of installed and enabled applications.
Also respect uninstalled Maniphest where disabled Maniphest was checked.
Test Plan:
Visited T1, D1.
Uninstalled Maniphest then visited T1, D1.
Disabled Maniphest then visited T1.
Visited /config/edit/maniphest.enabled/.
Reviewers: epriestley, Afaque_Hussain, edward
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5602
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
Summary: Use UA strings to detect platform; override general monospaced settings with platform-specific ones. Fixes T2868.
Test Plan: whatcouldgowrong
Reviewers: chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T2868
Differential Revision: https://secure.phabricator.com/D5526
Summary:
The welcome.enabled feature is turned on by default; unsure of
whether this is desired by upstream.
Test Plan:
Apply the patch and see the welcome content. It's configurable
in the config.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5339
Summary: T2381
Test Plan:
Include existing setup issues in the ignore config option,
reduces the number of setup issues in the status bar, moves ignored
issues to the bottom of the list, and marks them as ignored.
Also include a string corresponding to no setup issue, and verify that
application does not break.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5072
Summary: Makes sense with `QueryFuture`.
Test Plan: Switched secure.phabricator.com to MySQLi and nothing exploded.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5066
Test Plan: Enter in a url and create a macro. :)
Reviewers: epriestley
Reviewed By: epriestley
CC: epriestley, aran, dctrwatson, Korvin
Differential Revision: https://secure.phabricator.com/D5039
Summary: I had hard times figuring out how this works.
Test Plan: Read it.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin, s.o.butler
Differential Revision: https://secure.phabricator.com/D5031
Summary: Preserving animation of GIF profile Pictures
Test Plan: Uploaded Animated images as profile pictures to check if the animation of gif images is preserved and it does :) somewhat !
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4833
Summary:
After D4825, this information is often available to us in a safe way. Provide it explictly.
This removes or reduces functionality in some cases, but I think we can plug those holes with Conpherence addresses and/or explicit user acknowledgement/config.
Test Plan: Patched a commit with `arc patch` and got the original address out.
Reviewers: btrahan, edward, vrana
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D4828
Summary: Suggest the MySQL mode STRICT_ALL_TABLES during setup if it is not set. Small improvement to the phabricator.developer-mode comments.
Test Plan: Set the global sql_mode to include or exclude STRICT_ALL_TABLES and check for desired behavior.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4803
Summary: Refactor options related to verbose error reporting and forcing disk reads into a single developer option.
Test Plan: Run Phabricator with the developer-mode option set and check that errors print stack traces, static assets are always reloaded, etc.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4780
