From af5001db642d2546bcc8e5af79f0d0c6a79eb75d Mon Sep 17 00:00:00 2001 From: Chad Little Date: Fri, 24 Jun 2016 14:09:49 -0700 Subject: [PATCH] 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 --- .../20160623.phame.blog.fulldomain.1.sql | 2 + .../20160623.phame.blog.fulldomain.2.sql | 3 ++ .../20160623.phame.blog.fulldomain.3.sql | 3 ++ .../blog/PhameBlogManageController.php | 9 ++--- .../phame/editor/PhameBlogEditEngine.php | 14 +++---- .../phame/editor/PhameBlogEditor.php | 29 +++++++++----- src/applications/phame/storage/PhameBlog.php | 38 +++++++++---------- .../phame/storage/PhameBlogTransaction.php | 14 +++---- 8 files changed, 64 insertions(+), 48 deletions(-) create mode 100644 resources/sql/autopatches/20160623.phame.blog.fulldomain.1.sql create mode 100644 resources/sql/autopatches/20160623.phame.blog.fulldomain.2.sql create mode 100644 resources/sql/autopatches/20160623.phame.blog.fulldomain.3.sql diff --git a/resources/sql/autopatches/20160623.phame.blog.fulldomain.1.sql b/resources/sql/autopatches/20160623.phame.blog.fulldomain.1.sql new file mode 100644 index 0000000000..96fc3b27ba --- /dev/null +++ b/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}; diff --git a/resources/sql/autopatches/20160623.phame.blog.fulldomain.2.sql b/resources/sql/autopatches/20160623.phame.blog.fulldomain.2.sql new file mode 100644 index 0000000000..a323333c85 --- /dev/null +++ b/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; diff --git a/resources/sql/autopatches/20160623.phame.blog.fulldomain.3.sql b/resources/sql/autopatches/20160623.phame.blog.fulldomain.3.sql new file mode 100644 index 0000000000..05f6009de1 --- /dev/null +++ b/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'; diff --git a/src/applications/phame/controller/blog/PhameBlogManageController.php b/src/applications/phame/controller/blog/PhameBlogManageController.php index a87b02070d..4085d3ef6b 100644 --- a/src/applications/phame/controller/blog/PhameBlogManageController.php +++ b/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(); - if (!$domain) { - $domain = phutil_tag('em', array(), pht('No external domain')); + $full_domain = $blog->getDomainFullURI(); + if (!$full_domain) { + $full_domain = phutil_tag('em', array(), pht('No external domain')); } - - $properties->addProperty(pht('Domain'), $domain); + $properties->addProperty(pht('Full Domain'), $full_domain); $parent_site = $blog->getParentSite(); if (!$parent_site) { diff --git a/src/applications/phame/editor/PhameBlogEditEngine.php b/src/applications/phame/editor/PhameBlogEditEngine.php index 70d878e15a..5f96309e8b 100644 --- a/src/applications/phame/editor/PhameBlogEditEngine.php +++ b/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') - ->setLabel(pht('Custom Domain')) - ->setDescription(pht('Blog domain name.')) - ->setConduitDescription(pht('Change the blog domain.')) - ->setConduitTypeDescription(pht('New blog domain.')) - ->setValue($object->getDomain()) - ->setTransactionType(PhameBlogTransaction::TYPE_DOMAIN), + ->setKey('domainFullURI') + ->setLabel(pht('Full Domain URI')) + ->setDescription(pht('Blog full domain URI.')) + ->setConduitDescription(pht('Change the blog full domain URI.')) + ->setConduitTypeDescription(pht('New blog full domain URI.')) + ->setValue($object->getDomainFullURI()) + ->setTransactionType(PhameBlogTransaction::TYPE_FULLDOMAIN), id(new PhabricatorTextEditField()) ->setKey('parentSite') ->setLabel(pht('Parent Site')) diff --git a/src/applications/phame/editor/PhameBlogEditor.php b/src/applications/phame/editor/PhameBlogEditor.php index d2a9fcbc1f..197387985b 100644 --- a/src/applications/phame/editor/PhameBlogEditor.php +++ b/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: - return $object->getDomain(); + case PhameBlogTransaction::TYPE_FULLDOMAIN: + 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: - return $object->setDomain($xaction->getNewValue()); + case PhameBlogTransaction::TYPE_FULLDOMAIN: + $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( diff --git a/src/applications/phame/storage/PhameBlog.php b/src/applications/phame/storage/PhameBlog.php index 4f092cd3f1..6d9c51071a 100644 --- a/src/applications/phame/storage/PhameBlog.php +++ b/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) { - $example_domain = 'blog.example.com'; + public function validateCustomDomain($domain_full_uri) { + $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); - if ($uri->getProtocol()) { + $uri = new PhutilURI($domain_full_uri); + $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 bare domain name (for example, "%s").', + 'The custom domain should include a valid protocol in the URI '. + '(for example, "%s"). Valid protocols are "http" or "https".', $example_domain), - ); + ); } - if ($uri->getPort()) { - 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) { + if (strlen($path) && $path != '/') { 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; } diff --git a/src/applications/phame/storage/PhameBlogTransaction.php b/src/applications/phame/storage/PhameBlogTransaction.php index 91c6552666..2d74ca5cc6 100644 --- a/src/applications/phame/storage/PhameBlogTransaction.php +++ b/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;