unlcms / project-herbie

Drupal 10 implementation at the University of Nebraska–Lincoln
https://cms.unl.edu
GNU General Public License v2.0
5 stars 6 forks source link

Enforce trailing slashes on URLs and Links #183

Closed ericras closed 4 years ago

ericras commented 4 years ago

Followup of #180

Reason: Web audit needs a trailing slash to process a path as a site root. For example: https://engineering.unl.edu/civil/ will be the homepage for a Civil Eng site. If the URL is merely https://engineering.unl.edu/civil then that is just a page on the https://engineering.unl.edu/ site.

Things needed:

  1. This module rewrites Drupal generated links to add the slash on the end: https://www.drupal.org/project/trailing_slash

  2. The other piece is triggering a redirect when the slash isn't present. In D7 the Global Redirect module (and UNL's patched Redirect module) did this. In D8, we probably just want to do the htaccess method on https://www.drupal.org/project/trailing_slash :

RewriteEngine On
RewriteBase /
RewriteCond %{REQUEST_METHOD} !=post [NC]
RewriteRule ^(.*(?:^|/)[^/\.]+)$ $1/ [L,R=301]
ericras commented 4 years ago

This may be a little more complicated. The "Enforce clean and canonical URLs" setting on /admin/config/search/redirect/settings includes "removing trailing slashes". Redirect probably needs to be patched to split the different functionality (canonical enforcement, trailing slashes, etc) into separate settings.

macburgee1 commented 4 years ago

I did a quick code review on the Trailing Slash module, and I think we should steer clear of it. Handling rewrites with Apache would result in infinite looping (Redirect module adds trailing slash, and then Apache removes it, and then Redirect adds it back, and then Apache removes it again...)

I didn't find a good way to extend the Redirect module, and I'm not optimistic we'd get a patch committed, so by process of elimination, I'm going to propose we use a path_processor_outbound service to add a trailing slash to all URLs.

macburgee1 commented 4 years ago

Good catch. What about this instead?

public function processOutbound($path, &$options = [], Request $request = NULL, BubbleableMetadata $bubbleable_metadata = NULL) {
  $path = rtrim($path, '/');
  // If the path does not end in a file extension, then add a trailing slash.
  if (!pathinfo($path, PATHINFO_EXTENSION)) {
    $path = $path . '/';
  }
  return $path;
}
ericras commented 4 years ago

Awesome. (It's very unlikely we would want a "subsite homepage" to be like engineering.unl.edu/civil.html/ so this will be great.)

macburgee1 commented 4 years ago

Change is committed and pushed.