Allow PhameBlog to take a full URI instead of just a domain name

Summary: Ref T9897. This moves "Domain" to "DomainFullURI" to allow setting of https or for some reason, a port. I guess.

Test Plan: Try to break by setting a path, or fake protocol. Set to http, or https, see correct redirects. Verify domain still gets written.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T9897

Differential Revision: https://secure.phabricator.com/D16173

resources/sql/autopatches/20160623.phame.blog.fulldomain.1.sql

@@ -0,0 +1,2 @@
+ALTER TABLE {$NAMESPACE}_phame.phame_blog
+  ADD domainFullURI VARCHAR(128) COLLATE {$COLLATE_TEXT};

resources/sql/autopatches/20160623.phame.blog.fulldomain.2.sql

@@ -0,0 +1,3 @@
+UPDATE {$NAMESPACE}_phame.phame_blog
+  SET domainFullURI = CONCAT('http://', domain, '/')
+  WHERE domain IS NOT NULL;

resources/sql/autopatches/20160623.phame.blog.fulldomain.3.sql

@@ -0,0 +1,3 @@
+UPDATE {$NAMESPACE}_phame.phame_blogtransaction
+  SET transactionType = 'phame.blog.full.domain'
+  WHERE transactionType = 'phame.blog.domain';

src/applications/phame/controller/blog/PhameBlogManageController.php

@@ -97,12 +97,11 @@ final class PhameBlogManageController extends PhameBlogController {
     $properties = id(new PHUIPropertyListView())
       ->setUser($viewer);
 
-    $domain = $blog->getDomain();
+    $full_domain = $blog->getDomainFullURI();
-    if (!$domain) {
+    if (!$full_domain) {
-      $domain = phutil_tag('em', array(), pht('No external domain'));
+      $full_domain = phutil_tag('em', array(), pht('No external domain'));
     }
-
+    $properties->addProperty(pht('Full Domain'), $full_domain);
-    $properties->addProperty(pht('Domain'), $domain);
 
     $parent_site = $blog->getParentSite();
     if (!$parent_site) {

src/applications/phame/editor/PhameBlogEditEngine.php

@@ -94,13 +94,13 @@ final class PhameBlogEditEngine
         ->setTransactionType(PhameBlogTransaction::TYPE_DESCRIPTION)
         ->setValue($object->getDescription()),
       id(new PhabricatorTextEditField())
-        ->setKey('domain')
+        ->setKey('domainFullURI')
-        ->setLabel(pht('Custom Domain'))
+        ->setLabel(pht('Full Domain URI'))
-        ->setDescription(pht('Blog domain name.'))
+        ->setDescription(pht('Blog full domain URI.'))
-        ->setConduitDescription(pht('Change the blog domain.'))
+        ->setConduitDescription(pht('Change the blog full domain URI.'))
-        ->setConduitTypeDescription(pht('New blog domain.'))
+        ->setConduitTypeDescription(pht('New blog full domain URI.'))
-        ->setValue($object->getDomain())
+        ->setValue($object->getDomainFullURI())
-        ->setTransactionType(PhameBlogTransaction::TYPE_DOMAIN),
+        ->setTransactionType(PhameBlogTransaction::TYPE_FULLDOMAIN),
       id(new PhabricatorTextEditField())
         ->setKey('parentSite')
         ->setLabel(pht('Parent Site'))

src/applications/phame/editor/PhameBlogEditor.php

@@ -17,7 +17,7 @@ final class PhameBlogEditor
     $types[] = PhameBlogTransaction::TYPE_NAME;
     $types[] = PhameBlogTransaction::TYPE_SUBTITLE;
     $types[] = PhameBlogTransaction::TYPE_DESCRIPTION;
-    $types[] = PhameBlogTransaction::TYPE_DOMAIN;
+    $types[] = PhameBlogTransaction::TYPE_FULLDOMAIN;
     $types[] = PhameBlogTransaction::TYPE_PARENTSITE;
     $types[] = PhameBlogTransaction::TYPE_PARENTDOMAIN;
     $types[] = PhameBlogTransaction::TYPE_STATUS;
@@ -38,8 +38,8 @@ final class PhameBlogEditor
         return $object->getSubtitle();
       case PhameBlogTransaction::TYPE_DESCRIPTION:
         return $object->getDescription();
-      case PhameBlogTransaction::TYPE_DOMAIN:
+      case PhameBlogTransaction::TYPE_FULLDOMAIN:
-        return $object->getDomain();
+        return $object->getDomainFullURI();
       case PhameBlogTransaction::TYPE_PARENTSITE:
         return $object->getParentSite();
       case PhameBlogTransaction::TYPE_PARENTDOMAIN:
@@ -61,7 +61,7 @@ final class PhameBlogEditor
       case PhameBlogTransaction::TYPE_PARENTSITE:
       case PhameBlogTransaction::TYPE_PARENTDOMAIN:
         return $xaction->getNewValue();
-      case PhameBlogTransaction::TYPE_DOMAIN:
+      case PhameBlogTransaction::TYPE_FULLDOMAIN:
         $domain = $xaction->getNewValue();
         if (!strlen($xaction->getNewValue())) {
           return null;
@@ -81,8 +81,17 @@ final class PhameBlogEditor
         return $object->setSubtitle($xaction->getNewValue());
       case PhameBlogTransaction::TYPE_DESCRIPTION:
         return $object->setDescription($xaction->getNewValue());
-      case PhameBlogTransaction::TYPE_DOMAIN:
+      case PhameBlogTransaction::TYPE_FULLDOMAIN:
-        return $object->setDomain($xaction->getNewValue());
+        $new_value = $xaction->getNewValue();
+        if (strlen($new_value)) {
+          $uri = new PhutilURI($new_value);
+          $domain = $uri->getDomain();
+          $object->setDomain($domain);
+        } else {
+          $object->setDomain(null);
+        }
+        $object->setDomainFullURI($new_value);
+        return;
       case PhameBlogTransaction::TYPE_STATUS:
         return $object->setStatus($xaction->getNewValue());
       case PhameBlogTransaction::TYPE_PARENTSITE:
@@ -102,7 +111,7 @@ final class PhameBlogEditor
       case PhameBlogTransaction::TYPE_NAME:
       case PhameBlogTransaction::TYPE_SUBTITLE:
       case PhameBlogTransaction::TYPE_DESCRIPTION:
-      case PhameBlogTransaction::TYPE_DOMAIN:
+      case PhameBlogTransaction::TYPE_FULLDOMAIN:
       case PhameBlogTransaction::TYPE_PARENTSITE:
       case PhameBlogTransaction::TYPE_PARENTDOMAIN:
       case PhameBlogTransaction::TYPE_STATUS:
@@ -156,7 +165,7 @@ final class PhameBlogEditor
           $errors[] = $error;
         }
         break;
-      case PhameBlogTransaction::TYPE_DOMAIN:
+      case PhameBlogTransaction::TYPE_FULLDOMAIN:
         if (!$xactions) {
           continue;
         }
@@ -185,9 +194,11 @@ final class PhameBlogEditor
             nonempty(last($xactions), null));
           $errors[] = $error;
         }
+        $domain = new PhutilURI($custom_domain);
+        $domain = $domain->getDomain();
         $duplicate_blog = id(new PhameBlogQuery())
           ->setViewer(PhabricatorUser::getOmnipotentUser())
-          ->withDomain($custom_domain)
+          ->withDomain($domain)
           ->executeOne();
         if ($duplicate_blog && $duplicate_blog->getID() != $object->getID()) {
           $error = new PhabricatorApplicationTransactionValidationError(

src/applications/phame/storage/PhameBlog.php

@@ -18,6 +18,7 @@ final class PhameBlog extends PhameDAO
   protected $subtitle;
   protected $description;
   protected $domain;
+  protected $domainFullURI;
   protected $parentSite;
   protected $parentDomain;
   protected $configData;
@@ -46,6 +47,7 @@ final class PhameBlog extends PhameDAO
         'subtitle' => 'text64',
         'description' => 'text',
         'domain' => 'text128?',
+        'domainFullURI' => 'text128?',
         'parentSite' => 'text128',
         'parentDomain' => 'text128',
         'status' => 'text32',
@@ -112,34 +114,29 @@ final class PhameBlog extends PhameDAO
    *
    * @return string
    */
-  public function validateCustomDomain($custom_domain) {
+  public function validateCustomDomain($domain_full_uri) {
-    $example_domain = 'blog.example.com';
+    $example_domain = 'http://blog.example.com/';
     $label = pht('Invalid');
 
     // note this "uri" should be pretty busted given the desired input
     // so just use it to test if there's a protocol specified
-    $uri = new PhutilURI($custom_domain);
+    $uri = new PhutilURI($domain_full_uri);
-    if ($uri->getProtocol()) {
+    $domain = $uri->getDomain();
+    $protocol = $uri->getProtocol();
+    $path = $uri->getPath();
+    $supported_protocols = array('http', 'https');
+
+    if (!in_array($protocol, $supported_protocols)) {
       return array(
         $label,
         pht(
-          'The custom domain should not include a protocol. Just provide '.
+          'The custom domain should include a valid protocol in the URI '.
-          'the bare domain name (for example, "%s").',
+          '(for example, "%s"). Valid protocols are "http" or "https".',
           $example_domain),
-      );
+        );
     }
 
-    if ($uri->getPort()) {
+    if (strlen($path) && $path != '/') {
-      return array(
-        $label,
-        pht(
-          'The custom domain should not include a port number. Just provide '.
-          'the bare domain name (for example, "%s").',
-          $example_domain),
-      );
-    }
-
-    if (strpos($custom_domain, '/') !== false) {
       return array(
         $label,
         pht(
@@ -150,7 +147,7 @@ final class PhameBlog extends PhameDAO
         );
     }
 
-    if (strpos($custom_domain, '.') === false) {
+    if (strpos($domain, '.') === false) {
       return array(
         $label,
         pht(
@@ -191,7 +188,8 @@ final class PhameBlog extends PhameDAO
   }
 
   public function getExternalLiveURI() {
-    $uri = new PhutilURI('http://'.$this->getDomain().'/');
+    $uri = new PhutilURI($this->getDomainFullURI());
+    PhabricatorEnv::requireValidRemoteURIForLink($uri);
     return (string)$uri;
   }
 

src/applications/phame/storage/PhameBlogTransaction.php

@@ -6,7 +6,7 @@ final class PhameBlogTransaction
   const TYPE_NAME        = 'phame.blog.name';
   const TYPE_SUBTITLE    = 'phame.blog.subtitle';
   const TYPE_DESCRIPTION = 'phame.blog.description';
-  const TYPE_DOMAIN      = 'phame.blog.domain';
+  const TYPE_FULLDOMAIN  = 'phame.blog.full.domain';
   const TYPE_STATUS      = 'phame.blog.status';
   const TYPE_PARENTSITE  = 'phame.blog.parent.site';
   const TYPE_PARENTDOMAIN = 'phame.blog.parent.domain';
@@ -46,7 +46,7 @@ final class PhameBlogTransaction
         }
         break;
       case self::TYPE_DESCRIPTION:
-      case self::TYPE_DOMAIN:
+      case self::TYPE_FULLDOMAIN:
         return 'fa-pencil';
       case self::TYPE_STATUS:
         if ($new == PhameBlog::STATUS_ARCHIVED) {
@@ -85,7 +85,7 @@ final class PhameBlogTransaction
       case self::TYPE_NAME:
       case self::TYPE_SUBTITLE:
       case self::TYPE_DESCRIPTION:
-      case self::TYPE_DOMAIN:
+      case self::TYPE_FULLDOMAIN:
       case self::TYPE_PARENTSITE:
       case self::TYPE_PARENTDOMAIN:
         $tags[] = self::MAILTAG_DETAILS;
@@ -140,9 +140,9 @@ final class PhameBlogTransaction
           '%s updated the blog\'s description.',
           $this->renderHandleLink($author_phid));
         break;
-      case self::TYPE_DOMAIN:
+      case self::TYPE_FULLDOMAIN:
         return pht(
-          '%s updated the blog\'s domain to "%s".',
+          '%s updated the blog\'s full domain to "%s".',
           $this->renderHandleLink($author_phid),
           $new);
         break;
@@ -230,9 +230,9 @@ final class PhameBlogTransaction
           $this->renderHandleLink($author_phid),
           $this->renderHandleLink($object_phid));
         break;
-      case self::TYPE_DOMAIN:
+      case self::TYPE_FULLDOMAIN:
         return pht(
-          '%s updated the domain for %s.',
+          '%s updated the full domain for %s.',
           $this->renderHandleLink($author_phid),
           $this->renderHandleLink($object_phid));
         break;