diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 849d4d4702..f4a410ad2e 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -860,6 +860,7 @@ phutil_register_library_map(array( 'DiffusionRepositoryEditDangerousController' => 'applications/diffusion/controller/DiffusionRepositoryEditDangerousController.php', 'DiffusionRepositoryEditDeleteController' => 'applications/diffusion/controller/DiffusionRepositoryEditDeleteController.php', 'DiffusionRepositoryEditEngine' => 'applications/diffusion/editor/DiffusionRepositoryEditEngine.php', + 'DiffusionRepositoryEditEnormousController' => 'applications/diffusion/controller/DiffusionRepositoryEditEnormousController.php', 'DiffusionRepositoryEditUpdateController' => 'applications/diffusion/controller/DiffusionRepositoryEditUpdateController.php', 'DiffusionRepositoryFunctionDatasource' => 'applications/diffusion/typeahead/DiffusionRepositoryFunctionDatasource.php', 'DiffusionRepositoryHistoryManagementPanel' => 'applications/diffusion/management/DiffusionRepositoryHistoryManagementPanel.php', @@ -1007,6 +1008,7 @@ phutil_register_library_map(array( 'DrydockBlueprintCustomField' => 'applications/drydock/customfield/DrydockBlueprintCustomField.php', 'DrydockBlueprintDatasource' => 'applications/drydock/typeahead/DrydockBlueprintDatasource.php', 'DrydockBlueprintDisableController' => 'applications/drydock/controller/DrydockBlueprintDisableController.php', + 'DrydockBlueprintDisableTransaction' => 'applications/drydock/xaction/DrydockBlueprintDisableTransaction.php', 'DrydockBlueprintEditConduitAPIMethod' => 'applications/drydock/conduit/DrydockBlueprintEditConduitAPIMethod.php', 'DrydockBlueprintEditController' => 'applications/drydock/controller/DrydockBlueprintEditController.php', 'DrydockBlueprintEditEngine' => 'applications/drydock/editor/DrydockBlueprintEditEngine.php', @@ -1015,12 +1017,15 @@ phutil_register_library_map(array( 'DrydockBlueprintImplementationTestCase' => 'applications/drydock/blueprint/__tests__/DrydockBlueprintImplementationTestCase.php', 'DrydockBlueprintListController' => 'applications/drydock/controller/DrydockBlueprintListController.php', 'DrydockBlueprintNameNgrams' => 'applications/drydock/storage/DrydockBlueprintNameNgrams.php', + 'DrydockBlueprintNameTransaction' => 'applications/drydock/xaction/DrydockBlueprintNameTransaction.php', 'DrydockBlueprintPHIDType' => 'applications/drydock/phid/DrydockBlueprintPHIDType.php', 'DrydockBlueprintQuery' => 'applications/drydock/query/DrydockBlueprintQuery.php', 'DrydockBlueprintSearchConduitAPIMethod' => 'applications/drydock/conduit/DrydockBlueprintSearchConduitAPIMethod.php', 'DrydockBlueprintSearchEngine' => 'applications/drydock/query/DrydockBlueprintSearchEngine.php', 'DrydockBlueprintTransaction' => 'applications/drydock/storage/DrydockBlueprintTransaction.php', 'DrydockBlueprintTransactionQuery' => 'applications/drydock/query/DrydockBlueprintTransactionQuery.php', + 'DrydockBlueprintTransactionType' => 'applications/drydock/xaction/DrydockBlueprintTransactionType.php', + 'DrydockBlueprintTypeTransaction' => 'applications/drydock/xaction/DrydockBlueprintTypeTransaction.php', 'DrydockBlueprintViewController' => 'applications/drydock/controller/DrydockBlueprintViewController.php', 'DrydockCommand' => 'applications/drydock/storage/DrydockCommand.php', 'DrydockCommandError' => 'applications/drydock/exception/DrydockCommandError.php', @@ -5923,6 +5928,7 @@ phutil_register_library_map(array( 'DiffusionRepositoryEditDangerousController' => 'DiffusionRepositoryManageController', 'DiffusionRepositoryEditDeleteController' => 'DiffusionRepositoryManageController', 'DiffusionRepositoryEditEngine' => 'PhabricatorEditEngine', + 'DiffusionRepositoryEditEnormousController' => 'DiffusionRepositoryManageController', 'DiffusionRepositoryEditUpdateController' => 'DiffusionRepositoryManageController', 'DiffusionRepositoryFunctionDatasource' => 'PhabricatorTypeaheadCompositeDatasource', 'DiffusionRepositoryHistoryManagementPanel' => 'DiffusionRepositoryManagementPanel', @@ -6100,6 +6106,7 @@ phutil_register_library_map(array( 'DrydockBlueprintCustomField' => 'PhabricatorCustomField', 'DrydockBlueprintDatasource' => 'PhabricatorTypeaheadDatasource', 'DrydockBlueprintDisableController' => 'DrydockBlueprintController', + 'DrydockBlueprintDisableTransaction' => 'DrydockBlueprintTransactionType', 'DrydockBlueprintEditConduitAPIMethod' => 'PhabricatorEditEngineAPIMethod', 'DrydockBlueprintEditController' => 'DrydockBlueprintController', 'DrydockBlueprintEditEngine' => 'PhabricatorEditEngine', @@ -6108,12 +6115,15 @@ phutil_register_library_map(array( 'DrydockBlueprintImplementationTestCase' => 'PhabricatorTestCase', 'DrydockBlueprintListController' => 'DrydockBlueprintController', 'DrydockBlueprintNameNgrams' => 'PhabricatorSearchNgrams', + 'DrydockBlueprintNameTransaction' => 'DrydockBlueprintTransactionType', 'DrydockBlueprintPHIDType' => 'PhabricatorPHIDType', 'DrydockBlueprintQuery' => 'DrydockQuery', 'DrydockBlueprintSearchConduitAPIMethod' => 'PhabricatorSearchEngineAPIMethod', 'DrydockBlueprintSearchEngine' => 'PhabricatorApplicationSearchEngine', - 'DrydockBlueprintTransaction' => 'PhabricatorApplicationTransaction', + 'DrydockBlueprintTransaction' => 'PhabricatorModularTransaction', 'DrydockBlueprintTransactionQuery' => 'PhabricatorApplicationTransactionQuery', + 'DrydockBlueprintTransactionType' => 'PhabricatorModularTransactionType', + 'DrydockBlueprintTypeTransaction' => 'DrydockBlueprintTransactionType', 'DrydockBlueprintViewController' => 'DrydockBlueprintController', 'DrydockCommand' => array( 'DrydockDAO', diff --git a/src/applications/diffusion/application/PhabricatorDiffusionApplication.php b/src/applications/diffusion/application/PhabricatorDiffusionApplication.php index 5a426ec299..5797b8ba7c 100644 --- a/src/applications/diffusion/application/PhabricatorDiffusionApplication.php +++ b/src/applications/diffusion/application/PhabricatorDiffusionApplication.php @@ -89,6 +89,7 @@ final class PhabricatorDiffusionApplication extends PhabricatorApplication { 'edit/' => array( 'activate/' => 'DiffusionRepositoryEditActivateController', 'dangerous/' => 'DiffusionRepositoryEditDangerousController', + 'enormous/' => 'DiffusionRepositoryEditEnormousController', 'delete/' => 'DiffusionRepositoryEditDeleteController', 'update/' => 'DiffusionRepositoryEditUpdateController', 'testautomation/' => 'DiffusionRepositoryTestAutomationController', diff --git a/src/applications/diffusion/controller/DiffusionCommitController.php b/src/applications/diffusion/controller/DiffusionCommitController.php index 11463ef97d..b4d06989c2 100644 --- a/src/applications/diffusion/controller/DiffusionCommitController.php +++ b/src/applications/diffusion/controller/DiffusionCommitController.php @@ -277,9 +277,9 @@ final class DiffusionCommitController extends DiffusionController { 'This commit is empty and does not affect any paths.')); } else if ($was_limited) { $info_panel = $this->renderStatusMessage( - pht('Enormous Commit'), + pht('Very Large Commit'), pht( - 'This commit is enormous, and affects more than %d files. '. + 'This commit is very large, and affects more than %d files. '. 'Changes are not shown.', $hard_limit)); } else if (!$this->getCommitExists()) { diff --git a/src/applications/diffusion/controller/DiffusionRepositoryEditEnormousController.php b/src/applications/diffusion/controller/DiffusionRepositoryEditEnormousController.php new file mode 100644 index 0000000000..d4eeb118d7 --- /dev/null +++ b/src/applications/diffusion/controller/DiffusionRepositoryEditEnormousController.php @@ -0,0 +1,90 @@ +loadDiffusionContextForEdit(); + if ($response) { + return $response; + } + + $viewer = $this->getViewer(); + $drequest = $this->getDiffusionRequest(); + $repository = $drequest->getRepository(); + + $panel_uri = id(new DiffusionRepositoryBasicsManagementPanel()) + ->setRepository($repository) + ->getPanelURI(); + + if (!$repository->canAllowEnormousChanges()) { + return $this->newDialog() + ->setTitle(pht('Unprotectable Repository')) + ->appendParagraph( + pht( + 'This repository can not be protected from enormous changes '. + 'because Phabricator does not control what users are allowed '. + 'to push to it.')) + ->addCancelButton($panel_uri); + } + + if ($request->isFormPost()) { + $xaction = id(new PhabricatorRepositoryTransaction()) + ->setTransactionType(PhabricatorRepositoryTransaction::TYPE_ENORMOUS) + ->setNewValue(!$repository->shouldAllowEnormousChanges()); + + $editor = id(new PhabricatorRepositoryEditor()) + ->setContinueOnNoEffect(true) + ->setContentSourceFromRequest($request) + ->setActor($viewer) + ->applyTransactions($repository, array($xaction)); + + return id(new AphrontReloadResponse())->setURI($panel_uri); + } + + if ($repository->shouldAllowEnormousChanges()) { + $title = pht('Prevent Enormous Changes'); + + $body = pht( + 'It will no longer be possible to push enormous changes to this '. + 'repository.'); + + $submit = pht('Prevent Enormous Changes'); + } else { + $title = pht('Allow Enormous Changes'); + + $body = array( + pht( + 'If you allow enormous changes, users can push commits which are '. + 'too large for Herald to process content rules for. This can allow '. + 'users to evade content rules implemented in Herald.'), + pht( + 'You can selectively configure Herald by adding rules to prevent a '. + 'subset of enormous changes (for example, based on who is trying '. + 'to push the change).'), + ); + + $submit = pht('Allow Enormous Changes'); + } + + $more_help = pht( + 'Enormous changes are commits which are too large to process with '. + 'content rules because: the diff text for the change is larger than '. + '%s bytes; or the diff text takes more than %s seconds to extract.', + new PhutilNumber(HeraldCommitAdapter::getEnormousByteLimit()), + new PhutilNumber(HeraldCommitAdapter::getEnormousTimeLimit())); + + $response = $this->newDialog(); + + foreach ((array)$body as $paragraph) { + $response->appendParagraph($paragraph); + } + + return $response + ->setTitle($title) + ->appendParagraph($more_help) + ->addSubmitButton($submit) + ->addCancelButton($panel_uri); + } + +} diff --git a/src/applications/diffusion/editor/DiffusionRepositoryEditEngine.php b/src/applications/diffusion/editor/DiffusionRepositoryEditEngine.php index b3baafbd06..dcb79a6c86 100644 --- a/src/applications/diffusion/editor/DiffusionRepositoryEditEngine.php +++ b/src/applications/diffusion/editor/DiffusionRepositoryEditEngine.php @@ -309,6 +309,19 @@ final class DiffusionRepositoryEditEngine ->setConduitDescription(pht('Allow or prevent dangerous changes.')) ->setConduitTypeDescription(pht('New protection setting.')) ->setValue($object->shouldAllowDangerousChanges()), + id(new PhabricatorBoolEditField()) + ->setKey('allowEnormousChanges') + ->setLabel(pht('Allow Enormous Changes')) + ->setIsCopyable(true) + ->setIsConduitOnly(true) + ->setOptions( + pht('Prevent Enormous Changes'), + pht('Allow Enormous Changes')) + ->setTransactionType(PhabricatorRepositoryTransaction::TYPE_ENORMOUS) + ->setDescription(pht('Permit enormous changes to be made.')) + ->setConduitDescription(pht('Allow or prevent enormous changes.')) + ->setConduitTypeDescription(pht('New protection setting.')) + ->setValue($object->shouldAllowEnormousChanges()), id(new PhabricatorSelectEditField()) ->setKey('status') ->setLabel(pht('Status')) diff --git a/src/applications/diffusion/engine/DiffusionCommitHookEngine.php b/src/applications/diffusion/engine/DiffusionCommitHookEngine.php index 7fe45834b3..99df0e54af 100644 --- a/src/applications/diffusion/engine/DiffusionCommitHookEngine.php +++ b/src/applications/diffusion/engine/DiffusionCommitHookEngine.php @@ -34,6 +34,7 @@ final class DiffusionCommitHookEngine extends Phobject { private $rejectCode = PhabricatorRepositoryPushLog::REJECT_BROKEN; private $rejectDetails; private $emailPHIDs = array(); + private $changesets = array(); /* -( Config )------------------------------------------------------------- */ @@ -131,6 +132,15 @@ final class DiffusionCommitHookEngine extends Phobject { $this->applyHeraldRefRules($ref_updates, $all_updates); $content_updates = $this->findContentUpdates($ref_updates); + + try { + $this->rejectEnormousChanges($content_updates); + } catch (DiffusionCommitHookRejectException $ex) { + // If we're rejecting enormous changes, flag everything. + $this->rejectCode = PhabricatorRepositoryPushLog::REJECT_ENORMOUS; + throw $ex; + } + $all_updates = array_merge($all_updates, $content_updates); $this->applyHeraldContentRules($content_updates, $all_updates); @@ -1079,7 +1089,37 @@ final class DiffusionCommitHookEngine extends Phobject { ->setEpoch(time()); } - public function loadChangesetsForCommit($identifier) { + private function rejectEnormousChanges(array $content_updates) { + $repository = $this->getRepository(); + if ($repository->shouldAllowEnormousChanges()) { + return; + } + + foreach ($content_updates as $update) { + $identifier = $update->getRefNew(); + try { + $changesets = $this->loadChangesetsForCommit($identifier); + $this->changesets[$identifier] = $changesets; + } catch (Exception $ex) { + $this->changesets[$identifier] = $ex; + + $message = pht( + 'ENORMOUS CHANGE'. + "\n". + 'Enormous change protection is enabled for this repository, but '. + 'you are pushing an enormous change ("%s"). Edit the repository '. + 'configuration before making enormous changes.'. + "\n\n". + "Content Exception: %s", + $identifier, + $ex->getMessage()); + + throw new DiffusionCommitHookRejectException($message); + } + } + } + + private function loadChangesetsForCommit($identifier) { $byte_limit = HeraldCommitAdapter::getEnormousByteLimit(); $time_limit = HeraldCommitAdapter::getEnormousTimeLimit(); @@ -1126,9 +1166,10 @@ final class DiffusionCommitHookEngine extends Phobject { if (strlen($raw_diff) >= $byte_limit) { throw new Exception( pht( - 'The raw text of this change is enormous (larger than %d '. - 'bytes). Herald can not process it.', - $byte_limit)); + 'The raw text of this change ("%s") is enormous (larger than %s '. + 'bytes).', + $identifier, + new PhutilNumber($byte_limit))); } if (!strlen($raw_diff)) { @@ -1143,6 +1184,20 @@ final class DiffusionCommitHookEngine extends Phobject { return $diff->getChangesets(); } + public function getChangesetsForCommit($identifier) { + if (isset($this->changesets[$identifier])) { + $cached = $this->changesets[$identifier]; + + if ($cached instanceof Exception) { + throw $cached; + } + + return $cached; + } + + return $this->loadChangesetsForCommit($identifier); + } + public function loadCommitRefForCommit($identifier) { $repository = $this->getRepository(); $vcs = $repository->getVersionControlSystem(); diff --git a/src/applications/diffusion/herald/HeraldPreCommitContentAdapter.php b/src/applications/diffusion/herald/HeraldPreCommitContentAdapter.php index f4d7e794a2..c93ba32345 100644 --- a/src/applications/diffusion/herald/HeraldPreCommitContentAdapter.php +++ b/src/applications/diffusion/herald/HeraldPreCommitContentAdapter.php @@ -37,7 +37,7 @@ final class HeraldPreCommitContentAdapter extends HeraldPreCommitAdapter { public function getDiffContent($type) { if ($this->changesets === null) { try { - $this->changesets = $this->getHookEngine()->loadChangesetsForCommit( + $this->changesets = $this->getHookEngine()->getChangesetsForCommit( $this->getObject()->getRefNew()); } catch (Exception $ex) { $this->changesets = $ex; diff --git a/src/applications/diffusion/management/DiffusionRepositoryBasicsManagementPanel.php b/src/applications/diffusion/management/DiffusionRepositoryBasicsManagementPanel.php index 6e527e81b5..af5e9b2881 100644 --- a/src/applications/diffusion/management/DiffusionRepositoryBasicsManagementPanel.php +++ b/src/applications/diffusion/management/DiffusionRepositoryBasicsManagementPanel.php @@ -43,6 +43,7 @@ final class DiffusionRepositoryBasicsManagementPanel $delete_uri = $repository->getPathURI('edit/delete/'); $encoding_uri = $this->getEditPageURI('encoding'); $dangerous_uri = $repository->getPathURI('edit/dangerous/'); + $enormous_uri = $repository->getPathURI('edit/enormous/'); if ($repository->isTracked()) { $activate_label = pht('Deactivate Repository'); @@ -59,6 +60,15 @@ final class DiffusionRepositoryBasicsManagementPanel $can_dangerous = ($can_edit && $repository->canAllowDangerousChanges()); } + $should_enormous = $repository->shouldAllowEnormousChanges(); + if ($should_enormous) { + $enormous_name = pht('Prevent Enormous Changes'); + $can_enormous = $can_edit; + } else { + $enormous_name = pht('Allow Enormous Changes'); + $can_enormous = ($can_edit && $repository->canAllowEnormousChanges()); + } + $action_list->addAction( id(new PhabricatorActionView()) ->setName(pht('Edit Basic Information')) @@ -80,6 +90,13 @@ final class DiffusionRepositoryBasicsManagementPanel ->setDisabled(!$can_dangerous) ->setWorkflow(true)); + $action_list->addAction( + id(new PhabricatorActionView()) + ->setName($enormous_name) + ->setHref($enormous_uri) + ->setDisabled(!$can_enormous) + ->setWorkflow(true)); + $action_list->addAction( id(new PhabricatorActionView()) ->setHref($activate_uri) @@ -198,6 +215,20 @@ final class DiffusionRepositoryBasicsManagementPanel $view->addProperty(pht('Dangerous Changes'), $dangerous); + $can_enormous = $repository->canAllowEnormousChanges(); + if (!$can_enormous) { + $enormous = phutil_tag('em', array(), pht('Not Preventable')); + } else { + $should_enormous = $repository->shouldAllowEnormousChanges(); + if ($should_enormous) { + $enormous = pht('Allowed'); + } else { + $enormous = pht('Not Allowed'); + } + } + + $view->addProperty(pht('Enormous Changes'), $enormous); + return $view; } diff --git a/src/applications/diffusion/ssh/DiffusionMercurialServeSSHWorkflow.php b/src/applications/diffusion/ssh/DiffusionMercurialServeSSHWorkflow.php index 4508ae53da..5701a4bac1 100644 --- a/src/applications/diffusion/ssh/DiffusionMercurialServeSSHWorkflow.php +++ b/src/applications/diffusion/ssh/DiffusionMercurialServeSSHWorkflow.php @@ -66,17 +66,6 @@ final class DiffusionMercurialServeSSHWorkflow ->setWillWriteCallback(array($this, 'willWriteMessageCallback')) ->execute(); - // TODO: It's apparently technically possible to communicate errors to - // Mercurial over SSH by writing a special "\n\n-\n" string. However, - // my attempt to implement that resulted in Mercurial closing the socket and - // then hanging, without showing the error. This might be an issue on our - // side (we need to close our half of the socket?), or maybe the code - // for this in Mercurial doesn't actually work, or maybe something else - // is afoot. At some point, we should look into doing this more cleanly. - // For now, when we, e.g., reject writes for policy reasons, the user will - // see "abort: unexpected response: empty string" after the diagnostically - // useful, e.g., "remote: This repository is read-only over SSH." message. - if (!$err && $this->didSeeWrite) { $repository->writeStatusMessage( PhabricatorRepositoryStatusMessage::TYPE_NEEDS_UPDATE, @@ -109,14 +98,7 @@ final class DiffusionMercurialServeSSHWorkflow $this->didSeeWrite = true; } - $raw_message = $message['raw']; - if ($command == 'capabilities') { - $raw_message = DiffusionMercurialWireProtocol::filterBundle2Capability( - $raw_message); - } - - // If we're good, return the raw message data. - return $raw_message; + return $message['raw']; } protected function raiseWrongVCSException( diff --git a/src/applications/diffusion/ssh/DiffusionMercurialWireClientSSHProtocolChannel.php b/src/applications/diffusion/ssh/DiffusionMercurialWireClientSSHProtocolChannel.php index 5150ef9352..8eb026d8fe 100644 --- a/src/applications/diffusion/ssh/DiffusionMercurialWireClientSSHProtocolChannel.php +++ b/src/applications/diffusion/ssh/DiffusionMercurialWireClientSSHProtocolChannel.php @@ -180,6 +180,15 @@ final class DiffusionMercurialWireClientSSHProtocolChannel $this->state = 'arguments'; } } else if ($this->state == 'data-length') { + + // We're reading the length of a chunk of raw data. It looks like + // this: + // + // \n + // + // The length is human-readable text (for example, "4096"), and + // may be 0. + $line = $this->readProtocolLine(); if ($line === null) { break; @@ -192,10 +201,27 @@ final class DiffusionMercurialWireClientSSHProtocolChannel $this->state = 'data-bytes'; } } else if ($this->state == 'data-bytes') { + + // We're reading some known, nonzero number of raw bytes of data. + + // If we don't have any more bytes on the buffer yet, just bail: + // otherwise, we'll emit a pointless and possibly harmful 0-byte data + // frame. See T13036 for discussion. + if (!strlen($this->buffer)) { + break; + } + $bytes = substr($this->buffer, 0, $this->expectBytes); $this->buffer = substr($this->buffer, strlen($bytes)); $this->expectBytes -= strlen($bytes); + // NOTE: We emit a data frame as soon as we read some data. This can + // cause us to repackage frames: for example, if we receive one large + // frame slowly, we may emit it as several smaller frames. In theory + // this is good; in practice, Mercurial never seems to select a frame + // size larger than 4096 bytes naturally and this may be more + // complexity and trouble than it is worth. See T13036. + $messages[] = $this->newDataMessage($bytes); if (!$this->expectBytes) { diff --git a/src/applications/drydock/conduit/DrydockBlueprintEditConduitAPIMethod.php b/src/applications/drydock/conduit/DrydockBlueprintEditConduitAPIMethod.php index 764c077d87..7a120f7c5b 100644 --- a/src/applications/drydock/conduit/DrydockBlueprintEditConduitAPIMethod.php +++ b/src/applications/drydock/conduit/DrydockBlueprintEditConduitAPIMethod.php @@ -13,8 +13,7 @@ final class DrydockBlueprintEditConduitAPIMethod public function getMethodSummary() { return pht( - 'WARNING: Apply transactions to edit an existing blueprint. This method '. - 'can not create new blueprints.'); + 'Apply transactions to create or edit a blueprint.'); } } diff --git a/src/applications/drydock/controller/DrydockBlueprintDisableController.php b/src/applications/drydock/controller/DrydockBlueprintDisableController.php index 525e55228b..60fc8b0d74 100644 --- a/src/applications/drydock/controller/DrydockBlueprintDisableController.php +++ b/src/applications/drydock/controller/DrydockBlueprintDisableController.php @@ -28,7 +28,8 @@ final class DrydockBlueprintDisableController $xactions = array(); $xactions[] = id(new DrydockBlueprintTransaction()) - ->setTransactionType(DrydockBlueprintTransaction::TYPE_DISABLED) + ->setTransactionType( + DrydockBlueprintDisableTransaction::TRANSACTIONTYPE) ->setNewValue($is_disable ? 1 : 0); $editor = id(new DrydockBlueprintEditor()) diff --git a/src/applications/drydock/editor/DrydockBlueprintEditEngine.php b/src/applications/drydock/editor/DrydockBlueprintEditEngine.php index 020fe7514c..6bd1366ef0 100644 --- a/src/applications/drydock/editor/DrydockBlueprintEditEngine.php +++ b/src/applications/drydock/editor/DrydockBlueprintEditEngine.php @@ -51,6 +51,38 @@ final class DrydockBlueprintEditEngine return $blueprint; } + protected function newEditableObjectFromConduit(array $raw_xactions) { + $type = null; + foreach ($raw_xactions as $raw_xaction) { + if ($raw_xaction['type'] !== 'type') { + continue; + } + + $type = $raw_xaction['value']; + } + + if ($type === null) { + throw new Exception( + pht( + 'When creating a new Drydock blueprint via the Conduit API, you '. + 'must provide a "type" transaction to select a type.')); + } + + $map = DrydockBlueprintImplementation::getAllBlueprintImplementations(); + if (!isset($map[$type])) { + throw new Exception( + pht( + 'Blueprint type "%s" is unrecognized. Valid types are: %s.', + $type, + implode(', ', array_keys($map)))); + } + + $impl = clone $map[$type]; + $this->setBlueprintImplementation($impl); + + return $this->newEditableObject(); + } + protected function newEditableObjectForDocumentation() { // In order to generate the proper list of fields/transactions for a // blueprint, a blueprint's type needs to be known upfront, and there's @@ -112,16 +144,27 @@ final class DrydockBlueprintEditEngine $impl = $object->getImplementation(); return array( + // This field appears in the web UI id(new PhabricatorStaticEditField()) - ->setKey('type') + ->setKey('displayType') ->setLabel(pht('Blueprint Type')) ->setDescription(pht('Type of blueprint.')) ->setValue($impl->getBlueprintName()), + id(new PhabricatorTextEditField()) + ->setKey('type') + ->setLabel(pht('Type')) + ->setIsConduitOnly(true) + ->setTransactionType( + DrydockBlueprintTypeTransaction::TRANSACTIONTYPE) + ->setDescription(pht('When creating a blueprint, set the type.')) + ->setConduitDescription(pht('Set the blueprint type.')) + ->setConduitTypeDescription(pht('Blueprint type.')) + ->setValue($object->getClassName()), id(new PhabricatorTextEditField()) ->setKey('name') ->setLabel(pht('Name')) ->setDescription(pht('Name of the blueprint.')) - ->setTransactionType(DrydockBlueprintTransaction::TYPE_NAME) + ->setTransactionType(DrydockBlueprintNameTransaction::TRANSACTIONTYPE) ->setIsRequired(true) ->setValue($object->getBlueprintName()), ); diff --git a/src/applications/drydock/editor/DrydockBlueprintEditor.php b/src/applications/drydock/editor/DrydockBlueprintEditor.php index 125c36811a..aac3e7a020 100644 --- a/src/applications/drydock/editor/DrydockBlueprintEditor.php +++ b/src/applications/drydock/editor/DrydockBlueprintEditor.php @@ -11,6 +11,14 @@ final class DrydockBlueprintEditor return pht('Drydock Blueprints'); } + public function getCreateObjectTitle($author, $object) { + return pht('%s created this blueprint.', $author); + } + + public function getCreateObjectTitleForFeed($author, $object) { + return pht('%s created %s.', $author, $object); + } + protected function supportsSearch() { return true; } @@ -21,99 +29,7 @@ final class DrydockBlueprintEditor $types[] = PhabricatorTransactions::TYPE_VIEW_POLICY; $types[] = PhabricatorTransactions::TYPE_EDIT_POLICY; - $types[] = DrydockBlueprintTransaction::TYPE_NAME; - $types[] = DrydockBlueprintTransaction::TYPE_DISABLED; - return $types; } - protected function getCustomTransactionOldValue( - PhabricatorLiskDAO $object, - PhabricatorApplicationTransaction $xaction) { - - switch ($xaction->getTransactionType()) { - case DrydockBlueprintTransaction::TYPE_NAME: - return $object->getBlueprintName(); - case DrydockBlueprintTransaction::TYPE_DISABLED: - return (int)$object->getIsDisabled(); - } - - return parent::getCustomTransactionOldValue($object, $xaction); - } - - protected function getCustomTransactionNewValue( - PhabricatorLiskDAO $object, - PhabricatorApplicationTransaction $xaction) { - - switch ($xaction->getTransactionType()) { - case DrydockBlueprintTransaction::TYPE_NAME: - return $xaction->getNewValue(); - case DrydockBlueprintTransaction::TYPE_DISABLED: - return (int)$xaction->getNewValue(); - } - - return parent::getCustomTransactionNewValue($object, $xaction); - } - - protected function applyCustomInternalTransaction( - PhabricatorLiskDAO $object, - PhabricatorApplicationTransaction $xaction) { - - switch ($xaction->getTransactionType()) { - case DrydockBlueprintTransaction::TYPE_NAME: - $object->setBlueprintName($xaction->getNewValue()); - return; - case DrydockBlueprintTransaction::TYPE_DISABLED: - $object->setIsDisabled((int)$xaction->getNewValue()); - return; - } - - return parent::applyCustomInternalTransaction($object, $xaction); - } - - protected function applyCustomExternalTransaction( - PhabricatorLiskDAO $object, - PhabricatorApplicationTransaction $xaction) { - - switch ($xaction->getTransactionType()) { - case DrydockBlueprintTransaction::TYPE_NAME: - case DrydockBlueprintTransaction::TYPE_DISABLED: - return; - } - - return parent::applyCustomExternalTransaction($object, $xaction); - } - - - protected function validateTransaction( - PhabricatorLiskDAO $object, - $type, - array $xactions) { - - $errors = parent::validateTransaction($object, $type, $xactions); - - switch ($type) { - case DrydockBlueprintTransaction::TYPE_NAME: - $missing = $this->validateIsEmptyTextField( - $object->getBlueprintName(), - $xactions); - - if ($missing) { - $error = new PhabricatorApplicationTransactionValidationError( - $type, - pht('Required'), - pht('You must choose a name for this blueprint.'), - nonempty(last($xactions), null)); - - $error->setIsMissingFieldError(true); - $errors[] = $error; - continue; - } - - break; - } - - return $errors; - } - } diff --git a/src/applications/drydock/storage/DrydockBlueprintTransaction.php b/src/applications/drydock/storage/DrydockBlueprintTransaction.php index 0856f01fdb..68eebe8c1d 100644 --- a/src/applications/drydock/storage/DrydockBlueprintTransaction.php +++ b/src/applications/drydock/storage/DrydockBlueprintTransaction.php @@ -1,7 +1,7 @@ getOldValue(); - $new = $this->getNewValue(); - $author_handle = $this->renderHandleLink($this->getAuthorPHID()); - - switch ($this->getTransactionType()) { - case self::TYPE_NAME: - if (!strlen($old)) { - return pht( - '%s created this blueprint.', - $author_handle); - } else { - return pht( - '%s renamed this blueprint from "%s" to "%s".', - $author_handle, - $old, - $new); - } - case self::TYPE_DISABLED: - if ($new) { - return pht( - '%s disabled this blueprint.', - $author_handle); - } else { - return pht( - '%s enabled this blueprint.', - $author_handle); - } - } - - return parent::getTitle(); + public function getBaseTransactionClass() { + return 'DrydockBlueprintTransactionType'; } } diff --git a/src/applications/drydock/xaction/DrydockBlueprintDisableTransaction.php b/src/applications/drydock/xaction/DrydockBlueprintDisableTransaction.php new file mode 100644 index 0000000000..216cea35f4 --- /dev/null +++ b/src/applications/drydock/xaction/DrydockBlueprintDisableTransaction.php @@ -0,0 +1,48 @@ +getIsDisabled(); + } + + public function generateNewValue($object, $value) { + return (bool)$value; + } + + public function applyInternalEffects($object, $value) { + $object->setIsDisabled((int)$value); + } + + public function getTitle() { + $new = $this->getNewValue(); + if ($new) { + return pht( + '%s disabled this blueprint.', + $this->renderAuthor()); + } else { + return pht( + '%s enabled this blueprint.', + $this->renderAuthor()); + } + } + + public function getTitleForFeed() { + $new = $this->getNewValue(); + if ($new) { + return pht( + '%s disabled %s.', + $this->renderAuthor(), + $this->renderObject()); + } else { + return pht( + '%s enabled %s.', + $this->renderAuthor(), + $this->renderObject()); + } + } + +} diff --git a/src/applications/drydock/xaction/DrydockBlueprintNameTransaction.php b/src/applications/drydock/xaction/DrydockBlueprintNameTransaction.php new file mode 100644 index 0000000000..c3c17d0413 --- /dev/null +++ b/src/applications/drydock/xaction/DrydockBlueprintNameTransaction.php @@ -0,0 +1,60 @@ +getBlueprintName(); + } + + public function applyInternalEffects($object, $value) { + $object->setBlueprintName($value); + } + + public function getTitle() { + return pht( + '%s renamed this blueprint from %s to %s.', + $this->renderAuthor(), + $this->renderOldValue(), + $this->renderNewValue()); + } + + public function getTitleForFeed() { + $old = $this->getOldValue(); + $new = $this->getNewValue(); + + return pht( + '%s renamed %s from %s to %s.', + $this->renderAuthor(), + $this->renderObject(), + $this->renderOldValue(), + $this->renderNewValue()); + } + + public function validateTransactions($object, array $xactions) { + $errors = array(); + + $name = $object->getBlueprintName(); + if ($this->isEmptyTextTransaction($name, $xactions)) { + $errors[] = $this->newRequiredError( + pht('Blueprints must have a name.')); + } + + $max_length = $object->getColumnMaximumByteLength('blueprintName'); + foreach ($xactions as $xaction) { + $new_value = $xaction->getNewValue(); + + $new_length = strlen($new_value); + if ($new_length > $max_length) { + $errors[] = $this->newInvalidError( + pht('Blueprint names can be no longer than %s characters.', + new PhutilNumber($max_length))); + } + } + + return $errors; + } + +} diff --git a/src/applications/drydock/xaction/DrydockBlueprintTransactionType.php b/src/applications/drydock/xaction/DrydockBlueprintTransactionType.php new file mode 100644 index 0000000000..ecf021b3ad --- /dev/null +++ b/src/applications/drydock/xaction/DrydockBlueprintTransactionType.php @@ -0,0 +1,4 @@ +getClassName(); + } + + public function applyInternalEffects($object, $value) { + $object->setClassName($value); + } + + public function getTitle() { + // These transactions can only be applied during object creation and never + // generate a timeline event. + return null; + } + + public function validateTransactions($object, array $xactions) { + $errors = array(); + + $name = $object->getClassName(); + if ($this->isEmptyTextTransaction($name, $xactions)) { + $errors[] = $this->newRequiredError( + pht('You must select a blueprint type when creating a blueprint.')); + } + + $map = DrydockBlueprintImplementation::getAllBlueprintImplementations(); + + foreach ($xactions as $xaction) { + if (!$this->isNewObject()) { + $errors[] = $this->newInvalidError( + pht( + 'The type of a blueprint can not be changed once it has '. + 'been created.'), + $xaction); + continue; + } + + $new = $xaction->getNewValue(); + if (!isset($map[$new])) { + $errors[] = $this->newInvalidError( + pht( + 'Blueprint type "%s" is not valid. Valid types are: %s.', + $new, + implode(', ', array_keys($map)))); + continue; + } + } + + return $errors; + } + +} diff --git a/src/applications/files/config/PhabricatorFilesConfigOptions.php b/src/applications/files/config/PhabricatorFilesConfigOptions.php index 0f13a2fb4f..1493c54f59 100644 --- a/src/applications/files/config/PhabricatorFilesConfigOptions.php +++ b/src/applications/files/config/PhabricatorFilesConfigOptions.php @@ -134,7 +134,7 @@ final class PhabricatorFilesConfigOptions "Configure which uploaded file types may be viewed directly ". "in the browser. Other file types will be downloaded instead ". "of displayed. This is mainly a usability consideration, since ". - "browsers tend to freak out when viewing enormous binary files.". + "browsers tend to freak out when viewing very large binary files.". "\n\n". "The keys in this map are viewable MIME types; the values are ". "the MIME types they are delivered as when they are viewed in ". diff --git a/src/applications/maniphest/controller/ManiphestReportController.php b/src/applications/maniphest/controller/ManiphestReportController.php index 66aa154e8f..3ba597fb92 100644 --- a/src/applications/maniphest/controller/ManiphestReportController.php +++ b/src/applications/maniphest/controller/ManiphestReportController.php @@ -75,6 +75,7 @@ final class ManiphestReportController extends ManiphestController { $conn = $table->establishConnection('r'); $joins = ''; + $create_joins = ''; if ($project_phid) { $joins = qsprintf( $conn, @@ -84,6 +85,12 @@ final class ManiphestReportController extends ManiphestController { PhabricatorEdgeConfig::TABLE_NAME_EDGE, PhabricatorProjectObjectHasProjectEdgeType::EDGECONST, $project_phid); + $create_joins = qsprintf( + $conn, + 'JOIN %T p ON p.src = t.phid AND p.type = %d AND p.dst = %s', + PhabricatorEdgeConfig::TABLE_NAME_EDGE, + PhabricatorProjectObjectHasProjectEdgeType::EDGECONST, + $project_phid); } $data = queryfx_all( @@ -99,13 +106,68 @@ final class ManiphestReportController extends ManiphestController { ManiphestTaskMergedIntoTransaction::TRANSACTIONTYPE, )); + // See PHI273. After the move to EditEngine, we no longer create a + // "status" transaction if a task is created directly into the default + // status. This likely impacted API/email tasks after 2016 and all other + // tasks after late 2017. Until Facts can fix this properly, use the + // task creation dates to generate synthetic transactions which look like + // the older transactions that this page expects. + + $default_status = ManiphestTaskStatus::getDefaultStatus(); + $duplicate_status = ManiphestTaskStatus::getDuplicateStatus(); + + // Build synthetic transactions which take status from `null` to the + // default value. + $create_rows = queryfx_all( + $conn, + 'SELECT t.dateCreated FROM %T t %Q', + id(new ManiphestTask())->getTableName(), + $create_joins); + foreach ($create_rows as $key => $create_row) { + $create_rows[$key] = array( + 'transactionType' => 'status', + 'oldValue' => null, + 'newValue' => $default_status, + 'dateCreated' => $create_row['dateCreated'], + ); + } + + // Remove any actual legacy status transactions which take status from + // `null` to any open status. + foreach ($data as $key => $row) { + if ($row['transactionType'] != 'status') { + continue; + } + + $oldv = trim($row['oldValue'], '"'); + $newv = trim($row['oldValue'], '"'); + + // If this is a status change, preserve it. + if ($oldv != 'null') { + continue; + } + + // If this task was created directly into a closed status, preserve + // the transaction. + if (!ManiphestTaskStatus::isOpenStatus($newv)) { + continue; + } + + // If this is a legacy "create" transaction, discard it in favor of the + // synthetic one. + unset($data[$key]); + } + + // Merge the synthetic rows into the real transactions. + $data = array_merge($create_rows, $data); + $data = array_values($data); + $data = isort($data, 'dateCreated'); + $stats = array(); $day_buckets = array(); $open_tasks = array(); - $default_status = ManiphestTaskStatus::getDefaultStatus(); - $duplicate_status = ManiphestTaskStatus::getDuplicateStatus(); foreach ($data as $key => $row) { switch ($row['transactionType']) { case ManiphestTaskStatusTransaction::TRANSACTIONTYPE: diff --git a/src/applications/repository/editor/PhabricatorRepositoryEditor.php b/src/applications/repository/editor/PhabricatorRepositoryEditor.php index 401fca3668..768121de5a 100644 --- a/src/applications/repository/editor/PhabricatorRepositoryEditor.php +++ b/src/applications/repository/editor/PhabricatorRepositoryEditor.php @@ -28,6 +28,7 @@ final class PhabricatorRepositoryEditor $types[] = PhabricatorRepositoryTransaction::TYPE_AUTOCLOSE; $types[] = PhabricatorRepositoryTransaction::TYPE_PUSH_POLICY; $types[] = PhabricatorRepositoryTransaction::TYPE_DANGEROUS; + $types[] = PhabricatorRepositoryTransaction::TYPE_ENORMOUS; $types[] = PhabricatorRepositoryTransaction::TYPE_SLUG; $types[] = PhabricatorRepositoryTransaction::TYPE_SERVICE; $types[] = PhabricatorRepositoryTransaction::TYPE_SYMBOLS_LANGUAGE; @@ -76,6 +77,8 @@ final class PhabricatorRepositoryEditor return $object->getPushPolicy(); case PhabricatorRepositoryTransaction::TYPE_DANGEROUS: return $object->shouldAllowDangerousChanges(); + case PhabricatorRepositoryTransaction::TYPE_ENORMOUS: + return $object->shouldAllowEnormousChanges(); case PhabricatorRepositoryTransaction::TYPE_SLUG: return $object->getRepositorySlug(); case PhabricatorRepositoryTransaction::TYPE_SERVICE: @@ -110,6 +113,7 @@ final class PhabricatorRepositoryEditor case PhabricatorRepositoryTransaction::TYPE_VCS: case PhabricatorRepositoryTransaction::TYPE_PUSH_POLICY: case PhabricatorRepositoryTransaction::TYPE_DANGEROUS: + case PhabricatorRepositoryTransaction::TYPE_ENORMOUS: case PhabricatorRepositoryTransaction::TYPE_SERVICE: case PhabricatorRepositoryTransaction::TYPE_SYMBOLS_LANGUAGE: case PhabricatorRepositoryTransaction::TYPE_SYMBOLS_SOURCES: @@ -184,6 +188,9 @@ final class PhabricatorRepositoryEditor case PhabricatorRepositoryTransaction::TYPE_DANGEROUS: $object->setDetail('allow-dangerous-changes', $xaction->getNewValue()); return; + case PhabricatorRepositoryTransaction::TYPE_ENORMOUS: + $object->setDetail('allow-enormous-changes', $xaction->getNewValue()); + return; case PhabricatorRepositoryTransaction::TYPE_SLUG: $object->setRepositorySlug($xaction->getNewValue()); return; @@ -248,6 +255,7 @@ final class PhabricatorRepositoryEditor case PhabricatorRepositoryTransaction::TYPE_AUTOCLOSE: case PhabricatorRepositoryTransaction::TYPE_PUSH_POLICY: case PhabricatorRepositoryTransaction::TYPE_DANGEROUS: + case PhabricatorRepositoryTransaction::TYPE_ENORMOUS: case PhabricatorRepositoryTransaction::TYPE_SLUG: case PhabricatorRepositoryTransaction::TYPE_SERVICE: case PhabricatorRepositoryTransaction::TYPE_SYMBOLS_SOURCES: diff --git a/src/applications/repository/storage/PhabricatorRepository.php b/src/applications/repository/storage/PhabricatorRepository.php index 702bb42780..435e5749e6 100644 --- a/src/applications/repository/storage/PhabricatorRepository.php +++ b/src/applications/repository/storage/PhabricatorRepository.php @@ -1672,6 +1672,18 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO return (bool)$this->getDetail('allow-dangerous-changes'); } + public function canAllowEnormousChanges() { + if (!$this->isHosted()) { + return false; + } + + return true; + } + + public function shouldAllowEnormousChanges() { + return (bool)$this->getDetail('allow-enormous-changes'); + } + public function writeStatusMessage( $status_type, $status_code, diff --git a/src/applications/repository/storage/PhabricatorRepositoryPushLog.php b/src/applications/repository/storage/PhabricatorRepositoryPushLog.php index 4c449b4a84..4e099209c6 100644 --- a/src/applications/repository/storage/PhabricatorRepositoryPushLog.php +++ b/src/applications/repository/storage/PhabricatorRepositoryPushLog.php @@ -23,12 +23,14 @@ final class PhabricatorRepositoryPushLog const CHANGEFLAG_APPEND = 4; const CHANGEFLAG_REWRITE = 8; const CHANGEFLAG_DANGEROUS = 16; + const CHANGEFLAG_ENORMOUS = 32; const REJECT_ACCEPT = 0; const REJECT_DANGEROUS = 1; const REJECT_HERALD = 2; const REJECT_EXTERNAL = 3; const REJECT_BROKEN = 4; + const REJECT_ENORMOUS = 5; protected $repositoryPHID; protected $epoch; diff --git a/src/applications/repository/storage/PhabricatorRepositoryTransaction.php b/src/applications/repository/storage/PhabricatorRepositoryTransaction.php index e02b670f75..866800161e 100644 --- a/src/applications/repository/storage/PhabricatorRepositoryTransaction.php +++ b/src/applications/repository/storage/PhabricatorRepositoryTransaction.php @@ -16,6 +16,7 @@ final class PhabricatorRepositoryTransaction const TYPE_AUTOCLOSE = 'repo:autoclose'; const TYPE_PUSH_POLICY = 'repo:push-policy'; const TYPE_DANGEROUS = 'repo:dangerous'; + const TYPE_ENORMOUS = 'repo:enormous'; const TYPE_SLUG = 'repo:slug'; const TYPE_SERVICE = 'repo:service'; const TYPE_SYMBOLS_SOURCES = 'repo:symbol-source'; @@ -376,6 +377,16 @@ final class PhabricatorRepositoryTransaction '%s enabled protection against dangerous changes.', $this->renderHandleLink($author_phid)); } + case self::TYPE_ENORMOUS: + if ($new) { + return pht( + '%s disabled protection against enormous changes.', + $this->renderHandleLink($author_phid)); + } else { + return pht( + '%s enabled protection against enormous changes.', + $this->renderHandleLink($author_phid)); + } case self::TYPE_SLUG: if (strlen($old) && !strlen($new)) { return pht( diff --git a/src/applications/transactions/editengine/PhabricatorEditEngine.php b/src/applications/transactions/editengine/PhabricatorEditEngine.php index 8ba49f4d77..1e9d936929 100644 --- a/src/applications/transactions/editengine/PhabricatorEditEngine.php +++ b/src/applications/transactions/editengine/PhabricatorEditEngine.php @@ -630,6 +630,17 @@ abstract class PhabricatorEditEngine return $this->isCreate; } + /** + * Initialize a new object for object creation via Conduit. + * + * @return object Newly initialized object. + * @param list Raw transactions. + * @task load + */ + protected function newEditableObjectFromConduit(array $raw_xactions) { + return $this->newEditableObject(); + } + /** * Initialize a new object for documentation creation. * @@ -2031,6 +2042,8 @@ abstract class PhabricatorEditEngine get_class($this))); } + $raw_xactions = $this->getRawConduitTransactions($request); + $identifier = $request->getValue('objectIdentifier'); if ($identifier) { $this->setIsCreate(false); @@ -2039,7 +2052,7 @@ abstract class PhabricatorEditEngine $this->requireCreateCapability(); $this->setIsCreate(true); - $object = $this->newEditableObject(); + $object = $this->newEditableObjectFromConduit($raw_xactions); } $this->validateObject($object); @@ -2049,7 +2062,11 @@ abstract class PhabricatorEditEngine $types = $this->getConduitEditTypesFromFields($fields); $template = $object->getApplicationTransactionTemplate(); - $xactions = $this->getConduitTransactions($request, $types, $template); + $xactions = $this->getConduitTransactions( + $request, + $raw_xactions, + $types, + $template); $editor = $object->getApplicationTransactionEditor() ->setActor($viewer) @@ -2078,23 +2095,7 @@ abstract class PhabricatorEditEngine ); } - - /** - * Generate transactions which can be applied from edit actions in a Conduit - * request. - * - * @param ConduitAPIRequest The request. - * @param list Supported edit types. - * @param PhabricatorApplicationTransaction Template transaction. - * @return list Generated transactions. - * @task conduit - */ - private function getConduitTransactions( - ConduitAPIRequest $request, - array $types, - PhabricatorApplicationTransaction $template) { - - $viewer = $request->getUser(); + private function getRawConduitTransactions(ConduitAPIRequest $request) { $transactions_key = 'transactions'; $xactions = $request->getValue($transactions_key); @@ -2124,7 +2125,33 @@ abstract class PhabricatorEditEngine $transactions_key, $key)); } + } + return $xactions; + } + + + /** + * Generate transactions which can be applied from edit actions in a Conduit + * request. + * + * @param ConduitAPIRequest The request. + * @param list Raw conduit transactions. + * @param list Supported edit types. + * @param PhabricatorApplicationTransaction Template transaction. + * @return list Generated transactions. + * @task conduit + */ + private function getConduitTransactions( + ConduitAPIRequest $request, + array $xactions, + array $types, + PhabricatorApplicationTransaction $template) { + + $viewer = $request->getUser(); + $results = array(); + + foreach ($xactions as $key => $xaction) { $type = $xaction['type']; if (empty($types[$type])) { throw new Exception( @@ -2137,8 +2164,6 @@ abstract class PhabricatorEditEngine } } - $results = array(); - if ($this->getIsCreate()) { $results[] = id(clone $template) ->setTransactionType(PhabricatorTransactions::TYPE_CREATE);