We have an issue which i'm sure is a rather small use case but annoying none the less. I believe it to be an issue with subpathauto_url_inbound_alter() not checking if the current path has a redirect set if someone is using the redirect module.

Our specific use case:
Term 1 alias: foo/bar
Term 2 alias: foo/bar/foo

Changing foo/bar/foo to foo/bar/foo2 creates a redirect on the term from foo/bar/foo to foo/bar/foo2. This is correct. Actually going to foo/bar/foo however, goes to the foo/bar term.

This is because the module is not checking to see if foo/bar/foo is a redirect before doing its thing and is finding a match on foo/bar once the trailing foo has been removed. Due to it finding a match, it never does the redirect.

Putting a check in subpathauto_url_inbound_alter() would be simple enough but would like to know what your opinion is on this is.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jarrodirwin’s picture

Status: Active » Needs review
FileSize
691 bytes

As we need a fix for this in our system asap, I have added a simple patch that checks if the redirect module is enabled then checks for a redirect before looking for a match on other parts of the URL.

The more I think about this the more the issue may be with how/when the redirect module kicks in and does the redirect on their end.

Anyway, patch attached.

Status: Needs review » Needs work

The last submitted patch, check_for_redirect_from_redirect_module-1-1850832.patch, failed testing.

Pls’s picture

Status: Needs work » Needs review

Patch works for me so marking needs review.

deanflory’s picture

Issue summary: View changes

This issue is 3.5 years old and the latest patch now fails testing. Is this still an issue?

klidifia’s picture

Yes it's still an issue and the patch does not deal with it adequately, for example:

/some/path-with/a-redirect/ that redirects somewhere else, e.g. /some/path-with/a-redirect-done/
User accesses path: /some/path-with/a-redirect/foo/bar

Ideally what would happen is the module would pick up that the initial redirect and the end URI would be:

/some/path-with/a-redirect-done/foo/bar

pub497’s picture

This is still an issue with the latest version, instead of taking to page not found it will just take you to the parent page.

ie /node/parent/childnotfound will take you to node/parent, which is pretty bad and misleading

frrrt’s picture

I just fixed this issue for our site and attached a patch with our solution. I'm new to uploading patches so I'd appreciate any help! :-)

It should do exactly what klidifia and nitesoul were mentioning:
If there exist a redirect /some/path-with/a-redirect/ to /some/path-with/a-redirect-done/
then /some/path-with/a-redirect/foo/bar will be redirected to /some/path-with/a-redirect-done/foo/bar.

nord102’s picture

After applying the patch in #7, it did not fix my issue. I was experiencing the same issue as #6

Essentially the issue was happening when the moduel was assigning the $sourcein subpathauto_lookup_subpath. It was confirming that an alias for the $base_path existed and then was appending the $path_suffix to $source_based_path which would result in something like:

//The url is somewebsite.com/some/valid/path/invalid_subpath
$base_path = some/valid/path;
$source_based_path = node/12345;
$path_suffix = invalid_subpath;

$source = node/12345/invalid_subpath;

which would just return the node (node/12345, in the example) and ignore the appended $path_suffix.

Therefore in the attached patch, I changed how $source was being defined by using the $base_path instead of the $source_based_path and then checked that with drupal_valid_path() to see if it was valid.

Status: Needs review » Needs work

The last submitted patch, 8: subpathauto-subpathauto_lookup_subpath-1850832-8.patch, failed testing.

nord102’s picture

My mistake, this patch should work. (forgot to apply the previous patch first)

lolandese’s picture

Issue summary: View changes
mauritsl’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work
nord102’s picture

Re-uploading patch from #8 and #10. See patch description in #8

nord102’s picture

Status: Needs work » Needs review
lolandese’s picture

Category: Support request » Bug report
Status: Needs review » Reviewed & tested by the community

The OP describes the issue for taxonomy terms. It should be noted the same happens for nodes. Just to give some steps again to reproduce the issue (before applying the patch):

  • Edit an existing node with some working alias, e.g. old/alias.
  • Change the alias to the alias of another page plus another part. E.g. if you have another page (e.g. node/123) with the alias foo/bar make the alias of our node foo/bar/new.

A redirect should now automatically be created from the old alias to the new alias (old/alias -> foo/bar/new). However before applying the patch it will be node/123/old/alias -> foo/bar/new. After applying the patch repeating the same steps the aliases will now be as expected thus it solves the issue.

Please commit.

lolandese’s picture

Status: Reviewed & tested by the community » Needs work

On second test it seems to disrupt the basic functionality. foo/bar/new/edit now throws a 404.

To see if we can adjust the conditional to only kick-in when needed.

lolandese’s picture

Status: Needs work » Needs review
FileSize
897 bytes

With the Redirect module enabled, the patch:

  • resolves alias/edit to node/[nid]/edit (as expected functionality of Subpathauto)
  • leaves alias/something untouched if defined through hook_menu() by other modules and affected by Subpathauto
  • redirects a changed alias correctly to the new one (as expected functionality of the Redirect module)
  • redirects a manually created alias as defined (as expected functionality of the Redirect module)
lolandese’s picture

Attached patch solves the specific use case described by the OP when changing aliases (automatically generated redirect from old to new). You might still run into issues if manually defining the URLs through admin/config/search/path/add when an alias already exists for part of the newly created redirect (in the field Existing system path).