PathAuto creates a url alias that already exists in redirects. For example:
Create a piece of content with a title of Foo Bar. This generates a url alias of /foo-bar for node 1.
Next change the title of node 1 from Foo Bar to Foo Bar Renamed. Changes the url alias to /foo-bar-renamed and adds a redirect from /foo-bar to /foo-bar-renamed for node 1.
Next create a new piece of content with the title of Foo Bar again. This would be node 2. Upon saving, it creates a url alias of /foo-bar however, this then redirects to node 1 because the /foo-bar exists in the redirect table.
This has created havoc for some of our users because we have public advisories that are standardized and allows follow a similar pattern for publication.

Thanks,
Ron

CommentFileSizeAuthor
#12 pathauto-avoid-alias-that-is-redirected-2170259-12.patch667 bytestorotil
#8 pathauto-avoid-alias-that-is-redirected-2170259-8.patch659 bytestrrroy
#4 pathauto-avoid-alias-that-is-redirected-2170259-4.patch1.21 KBserundeputy
#2 pathauto-avoid-alias-that-is-redirected-2170259-2.patch1.21 KBserundeputy
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

WVxNitemare’s picture

WVxNitemare CreditAttribution: WVxNitemare commented

I was looking at the code for the function _pathauto_alias_exists and after browsing some of the issues here already perhaps the fix would be:

function _pathauto_alias_exists($alias, $source, $language = LANGUAGE_NONE) {
  $pid = db_query_range("SELECT pid FROM {url_alias} WHERE source <> :source AND alias = :alias AND language IN (:language, :language_none) ORDER BY language DESC, pid DESC", 0, 1, array(
    ':source' => $source,
    ':alias' => $alias,
    ':language' => $language,
    ':language_none' => LANGUAGE_NONE,
  ))->fetchField();

  //return !empty($pid);
  if (module_exists('redirect')){
	  $rid = redirect_load_by_source($alias, $language);
  }
  return !empty($pdi) || !empty($rid);
}

I've never suggested a fix before so not sure exactly how to do it... but this seems to work.

serundeputy’s picture

serundeputy CreditAttribution: serundeputy commented
Status: Active » Needs review
Issue tags: +pathauto, +301 redirect
FileSize
pathauto-avoid-alias-that-is-redirected-2170259-2.patch1.21 KB

rolling patch into pathauto.module 

function pathauto_is_alias_reserved

check if there is a redirect.

Status: Needs review » Needs work

The last submitted patch, 2: pathauto-avoid-alias-that-is-redirected-2170259-2.patch, failed testing.

serundeputy’s picture

serundeputy CreditAttribution: serundeputy commented
Version: 7.x-1.2 » 7.x-1.x-dev
FileSize
pathauto-avoid-alias-that-is-redirected-2170259-4.patch1.21 KB

re-rolling patch failed simple test

serundeputy’s picture

serundeputy CreditAttribution: serundeputy commented
Status: Needs work » Needs review

putting in 'Needs review' to run simple tests on re-roll.

serundeputy’s picture

serundeputy CreditAttribution: serundeputy commented
Assigned: Unassigned » serundeputy
trrroy’s picture

trrroy CreditAttribution: trrroy commented

This patch worked for me.

As a side note, it does only work on the Pathauto generated urls (which is correct for this particular issue/module). I was still having a problem if I unchecked 'Generate Auto url alias' then entered my own alias. If there was an existing redirect, it still created the alias which caused a bad redirect to another node. I applied the patch for the Redirect module from #242 on Fix and prevent circular redirects which solved that problem.

trrroy’s picture

trrroy CreditAttribution: trrroy commented
FileSize
pathauto-avoid-alias-that-is-redirected-2170259-8.patch659 bytes

After a little more testing I found that the if/else causes only the redirect to be checked. This prevents an existing alias from being checked. The attached patch removes the alias check from the 'else' clause.

serundeputy’s picture

serundeputy CreditAttribution: serundeputy commented
Assigned: serundeputy » Unassigned

setting to unassigned to indicate free for review

Leeteq’s picture

Leeteq CreditAttribution: Leeteq commented
Related issues: +#1796596: Fix and prevent circular redirects

This must be (re-)checked with the just released 1.3 version of Pathauto plus the "new" RC3 release of Redirect which fixed #1796596: Fix and prevent circular redirects.

If this is still a problem, maybe this issue should be added to as a child issue to #2548927: Plan for Pathauto 7.x-1.4 release?

torotil’s picture

torotil CreditAttribution: torotil at more onion commented
Issue tags: -

This is still a problem. The patch could be refined by:

  1. Implementing it as a hook_pathauto_is_alias_reserved() implementation on behalf of the redirect module.
  2. Dealing with language neutral redirects
  3. Not declaring the alias as reserved if the alias source is the same as the redirect target (ie. one and the same content can take its old name again).
torotil’s picture

torotil CreditAttribution: torotil at more onion commented
FileSize
pathauto-avoid-alias-that-is-redirected-2170259-12.patch667 bytes

Here is a patch implementing my own suggestions.

Language neutral redirects are already taken care of as redirect_load_by_source() already loads language neutral redirects as well.

jjtoyas’s picture

jjtoyas CreditAttribution: jjtoyas at Agiledrop - Your Trusted Drupal Teammates commented

For me, everything works as expected.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented

I don't maintain pathauto 7.x,. but FWIW, this behavior is basically by design.

Use case: You have node 1 with Alias A and node 2 with alias B. You want to change node 2 to alias A, so you first edit node 1, change it to C, then redirect creates a A => C redirect. Now you edit B and change it to A, then you want the redirect to be replaced with the new alias and not create A-0.

It's a hook, if you want to change that behavior then you implement that hook in your own module.

torotil’s picture

torotil CreditAttribution: torotil at more onion commented

@Berdir: I had similar doubts when first came across this, but after analysing edge cases I’ve come to the conclusion that this is indeed a bug that warrants fixing. Take a look at the following example:

  1. We have two (or more) nodes which have a title similar enough to get the same pathauto path: node/1 (A) & node/2 (A-0), but with different content.
  2. Links to node/1 are shared using the alias (eg. sent out by email).
  3. The title of node/1 is changed (B, A→B). The links now rely on the redirect to work.
  4. Now we load and save node/2 (A, removing the redirect A→B, add A-0 → A).
  5. Result: Without ever consciously editing any path or redirect the links to node/1 are broken and point to node/2 instead.
Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented

Valid use case, as I said, I won't be committing any 7.x patches anyway, but for an 8.x issue, I would at least expect it to be configurable, the behavior on existing sites should not change. And I think pathauto should not implement hooks for redirect, either it should be added to redirect.module or pathauto should check for the redirect module being enabled elsewhere. If redirect ever adds such a hook then you get fatal errors.

torotil’s picture

torotil CreditAttribution: torotil at more onion commented

If redirect ever adds such a hook then you get fatal errors.

I guess other maintainers are having similar views on committing changes to 7.x. So the likelihood of the redirect module adding this hook is negligible.

I won't be committing any 7.x patches anyway, but for an 8.x issue

Leaving this to someone else then. I don’t expect to become a D8+ developer anytime soon.