A recent core change requires paths to start with a leading slash.

See https://www.drupal.org/node/2516824

CommentFileSizeAuthor
#13 interdiff.txt2.15 KBjhedstrom
#13 leading-slash-2529996-13.patch6.32 KBjhedstrom
#9 leading-slash-2529996-09.patch5.74 KBjhedstrom
#8 2529996.slash-prefix.8.patch1.71 KBAnonymous (not verified)
#6 2529996.slash-prefix.6.patch1.65 KBAnonymous (not verified)
slash-prefix.patch1.31 KBAnonymous (not verified)
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Anonymous’s picture

Status: Active » Needs review
Anonymous’s picture

Status: Needs review » Needs work

retesting

Anonymous’s picture

Status: Needs work » Needs review
Berdir’s picture

Thanks for the patch.

+++ b/src/EventSubscriber/GlobalredirectSubscriber.php
@@ -127,7 +127,7 @@ class GlobalredirectSubscriber implements EventSubscriberInterface {
 
     $path_info = ltrim($event->getRequest()->getPathInfo(), '/');
     if (substr($path_info, -1, 1) === '/') {
-      $path_info = trim($path_info, '/');
+      $path_info = "/" . rtrim($path_info, '/');
       try {
         $path_info = $this->aliasManager->getPathByAlias($path_info);

@@ -149,7 +149,7 @@ class GlobalredirectSubscriber implements EventSubscriberInterface {
     $request = $event->getRequest();
-    $path = trim($request->getPathInfo(), '/');
+    $path = "/" . trim($request->getPathInfo(), '/');

Might be possible to just no longer trim() the path that we have there?

Berdir’s picture

Status: Needs review » Needs work
Anonymous’s picture

Sure, I only did it to validate the format, but I guess it's safe to assume that the format is correct as it's not the module's job to valid it...

Added new patch...

Anonymous’s picture

Status: Needs work » Needs review
Anonymous’s picture

Whoops, seems you need to check the path isn't '/' when you de-slash a URI, to stop redirects on the homepage.

jhedstrom’s picture

FileSize
5.74 KB

This should get tests passing.

jhedstrom’s picture

Sorry for the array syntax changes, they make it look a little more complex of a change.

Berdir’s picture

I guess we''ll have to wait on #2532740: Fix incorrect config schema to land but this looks good to me.

Berdir’s picture

Status: Needs review » Needs work

I've been testing this, globalredirectDeslash() fails on the frontpage because it results in an empty URL.

jhedstrom’s picture

Status: Needs work » Needs review
FileSize
6.32 KB
2.15 KB

This adds a test and resolves the issue for deslashing the front page.

vimokkhadipa’s picture

I apply #13 patch. Work fine!

Berdir’s picture

Status: Needs review » Closed (won't fix)