Fix /tag/aa%20bb project URIs

Summary:
Ref T9551. To set things up:

  - Name a project `aa bb`. This will have the tag `aa_bb`.
  - Try to visit `/tag/aa%20bb`.

Here's what happens now:

  - You get an Aphront redirect error as it tries to add the trailing `/`. Add `phutil_escape_uri()` so that works again.
  - Then, you 404, even though this tag is reasonably equivalent to the real project tag and could be redirected. Add a fallback to lookup, resolve, and redirect if we can find a hit for the tag.

This also fixes stuff like `/tag/AA_BB/`.

Test Plan: Visited URIs like `/tag/aa%20bb`, `/tag/aa%20bb/`, `/tag/Aa_bB/`, etc. None of them worked before and now they all do.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9551

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

src/aphront/configuration/AphrontApplicationConfiguration.php

@@ -372,6 +372,13 @@ abstract class AphrontApplicationConfiguration extends Phobject {
       $result = $this->routePath($maps, $path.'/');
       if ($result) {
         $slash_uri = $request->getRequestURI()->setPath($path.'/');
+
+        // We need to restore URI encoding because the webserver has
+        // interpreted it. For example, this allows us to redirect a path
+        // like `/tag/aa%20bb` to `/tag/aa%20bb/`, which may eventually be
+        // resolved meaningfully by an application.
+        $slash_uri = phutil_escape_uri($slash_uri);
+
         $external = strlen($request->getRequestURI()->getDomain());
         return $this->buildRedirectController($slash_uri, $external);
       }

src/applications/project/controller/PhabricatorProjectViewController.php

@@ -26,9 +26,16 @@ final class PhabricatorProjectViewController
     }
     $project = $query->executeOne();
     if (!$project) {
-      return new Aphront404Response();
+
+      // If this request corresponds to a project but just doesn't have the
+      // slug quite right, redirect to the proper URI.
+      $uri = $this->getNormalizedURI($slug);
+      if ($uri !== null) {
+        return id(new AphrontRedirectResponse())->setURI($uri);
       }
 
+      return new Aphront404Response();
+    }
 
     $columns = id(new PhabricatorProjectColumnQuery())
       ->setViewer($viewer)
@@ -53,4 +60,31 @@ final class PhabricatorProjectViewController
     return $this->delegateToController($controller_object);
   }
 
+  private function getNormalizedURI($slug) {
+    if (!strlen($slug)) {
+      return null;
+    }
+
+    $normal = PhabricatorSlug::normalizeProjectSlug($slug);
+    if ($normal === $slug) {
+      return null;
+    }
+
+    $viewer = $this->getViewer();
+
+    // Do execute() instead of executeOne() here so we canonicalize before
+    // raising a policy exception. This is a little more polished than letting
+    // the user hit the error on any variant of the slug.
+
+    $projects = id(new PhabricatorProjectQuery())
+      ->setViewer($viewer)
+      ->withSlugs(array($normal))
+      ->execute();
+    if (!$projects) {
+      return null;
+    }
+
+    return "/tag/{$normal}/";
+  }
+
 }

src/applications/project/editor/PhabricatorProjectTransactionEditor.php

@@ -498,9 +498,7 @@ final class PhabricatorProjectTransactionEditor
     PhabricatorLiskDAO $object,
     $name) {
 
-    $object = (clone $object);
+    $slug = PhabricatorSlug::normalizeProjectSlug($name);
-    $object->setPhrictionSlug($name);
-    $slug = $object->getPrimarySlug();
 
     $slug_object = id(new PhabricatorProjectSlug())->loadOneWhere(
       'slug = %s',

src/infrastructure/util/PhabricatorSlug.php

@@ -2,6 +2,12 @@
 
 final class PhabricatorSlug extends Phobject {
 
+  public static function normalizeProjectSlug($slug) {
+    $slug = str_replace('/', ' ', $slug);
+    $slug = self::normalize($slug);
+    return rtrim($slug, '/');
+  }
+
   public static function normalize($slug) {
     $slug = preg_replace('@/+@', '/', $slug);
     $slug = trim($slug, '/');