(stable) Promote 2018 Week 24

src/__phutil_library_map__.php

@@ -4026,7 +4026,9 @@ phutil_register_library_map(array(
     'PhabricatorProjectWorkboardBackgroundTransaction' => 'applications/project/xaction/PhabricatorProjectWorkboardBackgroundTransaction.php',
     'PhabricatorProjectWorkboardProfileMenuItem' => 'applications/project/menuitem/PhabricatorProjectWorkboardProfileMenuItem.php',
     'PhabricatorProjectWorkboardTransaction' => 'applications/project/xaction/PhabricatorProjectWorkboardTransaction.php',
+    'PhabricatorProjectsAllPolicyRule' => 'applications/project/policyrule/PhabricatorProjectsAllPolicyRule.php',
     'PhabricatorProjectsAncestorsSearchEngineAttachment' => 'applications/project/engineextension/PhabricatorProjectsAncestorsSearchEngineAttachment.php',
+    'PhabricatorProjectsBasePolicyRule' => 'applications/project/policyrule/PhabricatorProjectsBasePolicyRule.php',
     'PhabricatorProjectsCurtainExtension' => 'applications/project/engineextension/PhabricatorProjectsCurtainExtension.php',
     'PhabricatorProjectsEditEngineExtension' => 'applications/project/engineextension/PhabricatorProjectsEditEngineExtension.php',
     'PhabricatorProjectsEditField' => 'applications/transactions/editfield/PhabricatorProjectsEditField.php',
@@ -4449,6 +4451,7 @@ phutil_register_library_map(array(
     'PhabricatorSystemDAO' => 'applications/system/storage/PhabricatorSystemDAO.php',
     'PhabricatorSystemDestructionGarbageCollector' => 'applications/system/garbagecollector/PhabricatorSystemDestructionGarbageCollector.php',
     'PhabricatorSystemDestructionLog' => 'applications/system/storage/PhabricatorSystemDestructionLog.php',
+    'PhabricatorSystemObjectController' => 'applications/system/controller/PhabricatorSystemObjectController.php',
     'PhabricatorSystemReadOnlyController' => 'applications/system/controller/PhabricatorSystemReadOnlyController.php',
     'PhabricatorSystemRemoveDestroyWorkflow' => 'applications/system/management/PhabricatorSystemRemoveDestroyWorkflow.php',
     'PhabricatorSystemRemoveLogWorkflow' => 'applications/system/management/PhabricatorSystemRemoveLogWorkflow.php',
@@ -9893,7 +9896,9 @@ phutil_register_library_map(array(
     'PhabricatorProjectWorkboardBackgroundTransaction' => 'PhabricatorProjectTransactionType',
     'PhabricatorProjectWorkboardProfileMenuItem' => 'PhabricatorProfileMenuItem',
     'PhabricatorProjectWorkboardTransaction' => 'PhabricatorProjectTransactionType',
+    'PhabricatorProjectsAllPolicyRule' => 'PhabricatorProjectsBasePolicyRule',
     'PhabricatorProjectsAncestorsSearchEngineAttachment' => 'PhabricatorSearchEngineAttachment',
+    'PhabricatorProjectsBasePolicyRule' => 'PhabricatorPolicyRule',
     'PhabricatorProjectsCurtainExtension' => 'PHUICurtainExtension',
     'PhabricatorProjectsEditEngineExtension' => 'PhabricatorEditEngineExtension',
     'PhabricatorProjectsEditField' => 'PhabricatorTokenizerEditField',
@@ -9902,7 +9907,7 @@ phutil_register_library_map(array(
     'PhabricatorProjectsMailEngineExtension' => 'PhabricatorMailEngineExtension',
     'PhabricatorProjectsMembersSearchEngineAttachment' => 'PhabricatorSearchEngineAttachment',
     'PhabricatorProjectsMembershipIndexEngineExtension' => 'PhabricatorIndexEngineExtension',
-    'PhabricatorProjectsPolicyRule' => 'PhabricatorPolicyRule',
+    'PhabricatorProjectsPolicyRule' => 'PhabricatorProjectsBasePolicyRule',
     'PhabricatorProjectsSearchEngineAttachment' => 'PhabricatorSearchEngineAttachment',
     'PhabricatorProjectsSearchEngineExtension' => 'PhabricatorSearchEngineExtension',
     'PhabricatorProjectsWatchersSearchEngineAttachment' => 'PhabricatorSearchEngineAttachment',
@@ -10402,6 +10407,7 @@ phutil_register_library_map(array(
     'PhabricatorSystemDAO' => 'PhabricatorLiskDAO',
     'PhabricatorSystemDestructionGarbageCollector' => 'PhabricatorGarbageCollector',
     'PhabricatorSystemDestructionLog' => 'PhabricatorSystemDAO',
+    'PhabricatorSystemObjectController' => 'PhabricatorController',
     'PhabricatorSystemReadOnlyController' => 'PhabricatorController',
     'PhabricatorSystemRemoveDestroyWorkflow' => 'PhabricatorSystemRemoveWorkflow',
     'PhabricatorSystemRemoveLogWorkflow' => 'PhabricatorSystemRemoveWorkflow',

src/applications/differential/editor/DifferentialTransactionEditor.php

@@ -592,12 +592,13 @@ final class DifferentialTransactionEditor
   }
 
   protected function buildMailTemplate(PhabricatorLiskDAO $object) {
-    $id = $object->getID();
+    $monogram = $object->getMonogram();
     $title = $object->getTitle();
-    $subject = "D{$id}: {$title}";
 
     return id(new PhabricatorMetaMTAMail())
-      ->setSubject($subject);
+      ->setSubject(pht('%s: %s', $monogram, $title))
+      ->setMustEncryptSubject(pht('%s: Revision Updated', $monogram))
+      ->setMustEncryptURI($object->getURI());
   }
 
   protected function getTransactionsForMail(

src/applications/metamta/adapter/PhabricatorMailImplementationTestAdapter.php

@@ -123,4 +123,12 @@ final class PhabricatorMailImplementationTestAdapter
     return $this;
   }
 
+  public function getBody() {
+    return idx($this->guts, 'body');
+  }
+
+  public function getHTMLBody() {
+    return idx($this->guts, 'html-body');
+  }
+
 }

src/applications/metamta/storage/PhabricatorMetaMTAMail.php

@@ -291,17 +291,31 @@ final class PhabricatorMetaMTAMail
   }
 
   public function setMustEncrypt($bool) {
-    $this->setParam('mustEncrypt', $bool);
+    return $this->setParam('mustEncrypt', $bool);
-    return $this;
   }
 
   public function getMustEncrypt() {
     return $this->getParam('mustEncrypt', false);
   }
 
+  public function setMustEncryptURI($uri) {
+    return $this->setParam('mustEncrypt.uri', $uri);
+  }
+
+  public function getMustEncryptURI() {
+    return $this->getParam('mustEncrypt.uri');
+  }
+
+  public function setMustEncryptSubject($subject) {
+    return $this->setParam('mustEncrypt.subject', $subject);
+  }
+
+  public function getMustEncryptSubject() {
+    return $this->getParam('mustEncrypt.subject');
+  }
+
   public function setMustEncryptReasons(array $reasons) {
-    $this->setParam('mustEncryptReasons', $reasons);
+    return $this->setParam('mustEncryptReasons', $reasons);
-    return $this;
   }
 
   public function getMustEncryptReasons() {
@@ -787,7 +801,11 @@ final class PhabricatorMetaMTAMail
           // If mail content must be encrypted, we replace the subject with
           // a generic one.
           if ($must_encrypt) {
-            $subject[] = pht('Object Updated');
+            $encrypt_subject = $this->getMustEncryptSubject();
+            if (!strlen($encrypt_subject)) {
+              $encrypt_subject = pht('Object Updated');
+            }
+            $subject[] = $encrypt_subject;
           } else {
             $vary_prefix = idx($params, 'vary-subject-prefix');
             if ($vary_prefix != '') {
@@ -845,6 +863,23 @@ final class PhabricatorMetaMTAMail
     $body = $raw_body;
     if ($must_encrypt) {
       $parts = array();
+
+      $encrypt_uri = $this->getMustEncryptURI();
+      if (!strlen($encrypt_uri)) {
+        $encrypt_phid = $this->getRelatedPHID();
+        if ($encrypt_phid) {
+          $encrypt_uri = urisprintf(
+            '/object/%s/',
+            $encrypt_phid);
+        }
+      }
+
+      if (strlen($encrypt_uri)) {
+        $parts[] = pht(
+          'This secure message is notifying you of a change to this object:');
+        $parts[] = PhabricatorEnv::getProductionURI($encrypt_uri);
+      }
+
       $parts[] = pht(
         'The content for this message can only be transmitted over a '.
         'secure channel. To view the message content, follow this '.
@@ -857,15 +892,16 @@ final class PhabricatorMetaMTAMail
       $body = $raw_body;
     }
 
-    $max = PhabricatorEnv::getEnvConfig('metamta.email-body-limit');
+    $body_limit = PhabricatorEnv::getEnvConfig('metamta.email-body-limit');
-    if (strlen($body) > $max) {
+    if (strlen($body) > $body_limit) {
       $body = id(new PhutilUTF8StringTruncator())
-        ->setMaximumBytes($max)
+        ->setMaximumBytes($body_limit)
         ->truncateString($body);
       $body .= "\n";
-      $body .= pht('(This email was truncated at %d bytes.)', $max);
+      $body .= pht('(This email was truncated at %d bytes.)', $body_limit);
     }
     $mailer->setBody($body);
+    $body_limit -= strlen($body);
 
     // If we sent a different message body than we were asked to, record
     // what we actually sent to make debugging and diagnostics easier.
@@ -879,8 +915,17 @@ final class PhabricatorMetaMTAMail
       $send_html = $this->shouldSendHTML($preferences);
     }
 
-    if ($send_html && isset($params['html-body'])) {
+    if ($send_html) {
-      $mailer->setHTMLBody($params['html-body']);
+      $html_body = idx($params, 'html-body');
+      if (strlen($html_body)) {
+        // NOTE: We just drop the entire HTML body if it won't fit. Safely
+        // truncating HTML is hard, and we already have the text body to fall
+        // back to.
+        if (strlen($html_body) <= $body_limit) {
+          $mailer->setHTMLBody($html_body);
+          $body_limit -= strlen($html_body);
+        }
+      }
     }
 
     // Pass the headers to the mailer, then save the state so we can show

src/applications/metamta/storage/__tests__/PhabricatorMetaMTAMailTestCase.php

@@ -331,4 +331,84 @@ final class PhabricatorMetaMTAMailTestCase extends PhabricatorTestCase {
     $this->assertEqual(null, $mail->getMailerKey());
   }
 
+  public function testMailSizeLimits() {
+    $env = PhabricatorEnv::beginScopedEnv();
+    $env->overrideEnvConfig('metamta.email-body-limit', 1024 * 512);
+
+    $user = $this->generateNewTestUser();
+    $phid = $user->getPHID();
+
+    $string_1kb = str_repeat('x', 1024);
+    $html_1kb = str_repeat('y', 1024);
+    $string_1mb = str_repeat('x', 1024 * 1024);
+    $html_1mb = str_repeat('y', 1024 * 1024);
+
+    // First, send a mail with a small text body and a small HTML body to make
+    // sure the basics work properly.
+    $mail = id(new PhabricatorMetaMTAMail())
+      ->addTos(array($phid))
+      ->setBody($string_1kb)
+      ->setHTMLBody($html_1kb);
+
+    $mailer = new PhabricatorMailImplementationTestAdapter();
+    $mail->sendWithMailers(array($mailer));
+    $this->assertEqual(
+      PhabricatorMailOutboundStatus::STATUS_SENT,
+      $mail->getStatus());
+
+    $text_body = $mailer->getBody();
+    $html_body = $mailer->getHTMLBody();
+
+    $this->assertEqual($string_1kb, $text_body);
+    $this->assertEqual($html_1kb, $html_body);
+
+
+    // Now, send a mail with a large text body and a large HTML body. We expect
+    // the text body to be truncated and the HTML body to be dropped.
+    $mail = id(new PhabricatorMetaMTAMail())
+      ->addTos(array($phid))
+      ->setBody($string_1mb)
+      ->setHTMLBody($html_1mb);
+
+    $mailer = new PhabricatorMailImplementationTestAdapter();
+    $mail->sendWithMailers(array($mailer));
+    $this->assertEqual(
+      PhabricatorMailOutboundStatus::STATUS_SENT,
+      $mail->getStatus());
+
+    $text_body = $mailer->getBody();
+    $html_body = $mailer->getHTMLBody();
+
+    // We expect the body was truncated, because it exceeded the body limit.
+    $this->assertTrue(
+      (strlen($text_body) < strlen($string_1mb)),
+      pht('Text Body Truncated'));
+
+    // We expect the HTML body was dropped completely after the text body was
+    // truncated.
+    $this->assertTrue(
+      !strlen($html_body),
+      pht('HTML Body Removed'));
+
+
+    // Next send a mail with a small text body and a large HTML body. We expect
+    // the text body to be intact and the HTML body to be dropped.
+    $mail = id(new PhabricatorMetaMTAMail())
+      ->addTos(array($phid))
+      ->setBody($string_1kb)
+      ->setHTMLBody($html_1mb);
+
+    $mailer = new PhabricatorMailImplementationTestAdapter();
+    $mail->sendWithMailers(array($mailer));
+    $this->assertEqual(
+      PhabricatorMailOutboundStatus::STATUS_SENT,
+      $mail->getStatus());
+
+    $text_body = $mailer->getBody();
+    $html_body = $mailer->getHTMLBody();
+
+    $this->assertEqual($string_1kb, $text_body);
+    $this->assertTrue(!strlen($html_body));
+  }
+
 }

src/applications/project/__tests__/PhabricatorProjectCoreTestCase.php

@@ -1177,6 +1177,100 @@ final class PhabricatorProjectCoreTestCase extends PhabricatorTestCase {
     $this->assertTrue($can_edit);
   }
 
+  public function testProjectPolicyRules() {
+    $author = $this->generateNewTestUser();
+
+    $proj_a = PhabricatorProject::initializeNewProject($author)
+      ->setName('Policy A')
+      ->save();
+    $proj_b = PhabricatorProject::initializeNewProject($author)
+      ->setName('Policy B')
+      ->save();
+
+    $user_none = $this->generateNewTestUser();
+    $user_any = $this->generateNewTestUser();
+    $user_all = $this->generateNewTestUser();
+
+    $this->joinProject($proj_a, $user_any);
+    $this->joinProject($proj_a, $user_all);
+    $this->joinProject($proj_b, $user_all);
+
+    $any_policy = id(new PhabricatorPolicy())
+      ->setRules(
+        array(
+          array(
+            'action' => PhabricatorPolicy::ACTION_ALLOW,
+            'rule' => 'PhabricatorProjectsPolicyRule',
+            'value' => array(
+              $proj_a->getPHID(),
+              $proj_b->getPHID(),
+            ),
+          ),
+        ))
+      ->save();
+
+    $all_policy = id(new PhabricatorPolicy())
+      ->setRules(
+        array(
+          array(
+            'action' => PhabricatorPolicy::ACTION_ALLOW,
+            'rule' => 'PhabricatorProjectsAllPolicyRule',
+            'value' => array(
+              $proj_a->getPHID(),
+              $proj_b->getPHID(),
+            ),
+          ),
+        ))
+      ->save();
+
+    $any_task = ManiphestTask::initializeNewTask($author)
+      ->setViewPolicy($any_policy->getPHID())
+      ->save();
+
+    $all_task = ManiphestTask::initializeNewTask($author)
+      ->setViewPolicy($all_policy->getPHID())
+      ->save();
+
+    $map = array(
+      array(
+        pht('Project policy rule; user in no projects'),
+        $user_none,
+        false,
+        false,
+      ),
+      array(
+        pht('Project policy rule; user in some projects'),
+        $user_any,
+        true,
+        false,
+      ),
+      array(
+        pht('Project policy rule; user in all projects'),
+        $user_all,
+        true,
+        true,
+      ),
+    );
+
+    foreach ($map as $test_case) {
+      list($label, $user, $expect_any, $expect_all) = $test_case;
+
+      $can_any = PhabricatorPolicyFilter::hasCapability(
+        $user,
+        $any_task,
+        PhabricatorPolicyCapability::CAN_VIEW);
+
+      $can_all = PhabricatorPolicyFilter::hasCapability(
+        $user,
+        $all_task,
+        PhabricatorPolicyCapability::CAN_VIEW);
+
+      $this->assertEqual($expect_any, $can_any, pht('%s / Any', $label));
+      $this->assertEqual($expect_all, $can_all, pht('%s / All', $label));
+    }
+  }
+
+
   private function moveToColumn(
     PhabricatorUser $viewer,
     PhabricatorProject $board,

src/applications/project/policyrule/PhabricatorProjectsAllPolicyRule.php

@@ -0,0 +1,29 @@
+<?php
+
+final class PhabricatorProjectsAllPolicyRule
+  extends PhabricatorProjectsBasePolicyRule {
+
+  public function getRuleDescription() {
+    return pht('members of all projects');
+  }
+
+  public function applyRule(
+    PhabricatorUser $viewer,
+    $value,
+    PhabricatorPolicyInterface $object) {
+
+    $memberships = $this->getMemberships($viewer->getPHID());
+    foreach ($value as $project_phid) {
+      if (empty($memberships[$project_phid])) {
+        return false;
+      }
+    }
+
+    return true;
+  }
+
+  public function getRuleOrder() {
+    return 205;
+  }
+
+}

src/applications/project/policyrule/PhabricatorProjectsBasePolicyRule.php

@@ -0,0 +1,64 @@
+<?php
+
+abstract class PhabricatorProjectsBasePolicyRule
+  extends PhabricatorPolicyRule {
+
+  private $memberships = array();
+
+  protected function getMemberships($viewer_phid) {
+    return idx($this->memberships, $viewer_phid, array());
+  }
+
+  public function willApplyRules(
+    PhabricatorUser $viewer,
+    array $values,
+    array $objects) {
+
+    $values = array_unique(array_filter(array_mergev($values)));
+    if (!$values) {
+      return;
+    }
+
+    $projects = id(new PhabricatorProjectQuery())
+      ->setViewer(PhabricatorUser::getOmnipotentUser())
+      ->withMemberPHIDs(array($viewer->getPHID()))
+      ->withPHIDs($values)
+      ->execute();
+    foreach ($projects as $project) {
+      $this->memberships[$viewer->getPHID()][$project->getPHID()] = true;
+    }
+  }
+
+  public function getValueControlType() {
+    return self::CONTROL_TYPE_TOKENIZER;
+  }
+
+  public function getValueControlTemplate() {
+    $datasource = id(new PhabricatorProjectDatasource())
+      ->setParameters(
+        array(
+          'policy' => 1,
+        ));
+
+    return $this->getDatasourceTemplate($datasource);
+  }
+
+  public function getValueForStorage($value) {
+    PhutilTypeSpec::newFromString('list<string>')->check($value);
+    return array_values($value);
+  }
+
+  public function getValueForDisplay(PhabricatorUser $viewer, $value) {
+    $handles = id(new PhabricatorHandleQuery())
+      ->setViewer($viewer)
+      ->withPHIDs($value)
+      ->execute();
+
+    return mpull($handles, 'getFullName', 'getPHID');
+  }
+
+  public function ruleHasEffect($value) {
+    return (bool)$value;
+  }
+
+}

src/applications/project/policyrule/PhabricatorProjectsPolicyRule.php

@@ -1,32 +1,10 @@
 <?php
 
 final class PhabricatorProjectsPolicyRule
-  extends PhabricatorPolicyRule {
+  extends PhabricatorProjectsBasePolicyRule {
-
-  private $memberships = array();
 
   public function getRuleDescription() {
-    return pht('members of projects');
+    return pht('members of any project');
-  }
-
-  public function willApplyRules(
-    PhabricatorUser $viewer,
-    array $values,
-    array $objects) {
-
-    $values = array_unique(array_filter(array_mergev($values)));
-    if (!$values) {
-      return;
-    }
-
-    $projects = id(new PhabricatorProjectQuery())
-      ->setViewer(PhabricatorUser::getOmnipotentUser())
-      ->withMemberPHIDs(array($viewer->getPHID()))
-      ->withPHIDs($values)
-      ->execute();
-    foreach ($projects as $project) {
-      $this->memberships[$viewer->getPHID()][$project->getPHID()] = true;
-    }
   }
 
   public function applyRule(
@@ -34,8 +12,9 @@ final class PhabricatorProjectsPolicyRule
     $value,
     PhabricatorPolicyInterface $object) {
 
+    $memberships = $this->getMemberships($viewer->getPHID());
     foreach ($value as $project_phid) {
-      if (isset($this->memberships[$viewer->getPHID()][$project_phid])) {
+      if (isset($memberships[$project_phid])) {
         return true;
       }
     }
@@ -43,40 +22,8 @@ final class PhabricatorProjectsPolicyRule
     return false;
   }
 
-  public function getValueControlType() {
-    return self::CONTROL_TYPE_TOKENIZER;
-  }
-
-  public function getValueControlTemplate() {
-    $datasource = id(new PhabricatorProjectDatasource())
-      ->setParameters(
-        array(
-          'policy' => 1,
-        ));
-
-    return $this->getDatasourceTemplate($datasource);
-  }
-
   public function getRuleOrder() {
     return 200;
   }
 
-  public function getValueForStorage($value) {
-    PhutilTypeSpec::newFromString('list<string>')->check($value);
-    return array_values($value);
-  }
-
-  public function getValueForDisplay(PhabricatorUser $viewer, $value) {
-    $handles = id(new PhabricatorHandleQuery())
-      ->setViewer($viewer)
-      ->withPHIDs($value)
-      ->execute();
-
-    return mpull($handles, 'getFullName', 'getPHID');
-  }
-
-  public function ruleHasEffect($value) {
-    return (bool)$value;
-  }
-
 }

src/applications/system/application/PhabricatorSystemApplication.php

@@ -26,6 +26,7 @@ final class PhabricatorSystemApplication extends PhabricatorApplication {
       '/readonly/' => array(
         '(?P<reason>[^/]+)/' => 'PhabricatorSystemReadOnlyController',
       ),
+      '/object/(?P<name>[^/]+)/' => 'PhabricatorSystemObjectController',
     );
   }
 

src/applications/system/controller/PhabricatorSystemObjectController.php

@@ -0,0 +1,39 @@
+<?php
+
+final class PhabricatorSystemObjectController
+  extends PhabricatorController {
+
+  public function shouldAllowPublic() {
+    return true;
+  }
+
+  public function handleRequest(AphrontRequest $request) {
+    $viewer = $this->getViewer();
+    $name = $request->getURIData('name');
+
+    $object = id(new PhabricatorObjectQuery())
+      ->setViewer($viewer)
+      ->withNames(array($name))
+      ->executeOne();
+    if (!$object) {
+      return new Aphront404Response();
+    }
+
+    $phid = $object->getPHID();
+    $handles = $viewer->loadHandles(array($phid));
+    $handle = $handles[$phid];
+
+    $object_uri = $handle->getURI();
+    if (!strlen($object_uri)) {
+      return $this->newDialog()
+        ->setTitle(pht('No Object URI'))
+        ->appendParagraph(
+          pht(
+            'Object "%s" exists, but does not have a URI to redirect to.',
+            $name))
+        ->addCancelButton('/', pht('Done'));
+    }
+
+    return id(new AphrontRedirectResponse())->setURI($object_uri);
+  }
+}

src/applications/transactions/engineextension/PhabricatorSubtypeEditEngineExtension.php

@@ -47,6 +47,11 @@ final class PhabricatorSubtypeEditEngineExtension
       ->setValue($object->getEditEngineSubtype())
       ->setOptions($options);
 
+    // If subtypes are configured, enable changing them from the bulk editor.
+    if (count($map) > 1) {
+      $subtype_field->setBulkEditLabel(pht('Change subtype to'));
+    }
+
     return array(
       $subtype_field,
     );