I originally posted this issue under Path Redirect at http://drupal.org/node/218277 but then I realized it is probably an issue with the pathauto code that integrates into path redirect.

I have pathauto 2.0 running with the path redirect module. On pathauto settings, under "Update action", I have "Create a new alias. Redirect from old alias." which is AWESOME, but I just discovered a bug that I think is caused by this. I think it is caused by changing the title of the article and then later reverting to the original title (or one that would result in the same URL)

The error messages I get back from the browsers are:
-----------------------
Firefox:
The page isn't redirecting properly
Firefox has detected that the server is redirecting the request for this address in a way that will never complete.
* This problem can sometimes be caused by disabling or refusing to accept cookies.

Safari:
Safari can’t open the page.
Too many redirects occurred trying to open “http://www.example.org/news/charity-skydive-breath-life-sponsorship”. This might occur if you open a page that is redirected to open another page which then is redirected to open the original page.
-------------------
The Safari message was more useful in this case, and when I went to the URL redirects admin page, I noticed that there were several aliases pointing to node/471 - apparently the url was changed several times. One of the redirects was the same as the current URL being requested, so I guess it sends the redirecting into a loop and causes this problem.

I deleted the redirect that had the same URL as the current page alias and the problem was fixed. :)

I guess what needs to happen is that before changing the alias to the page, we need to check if there is already a redirect with the same alias and delete the alias if it exists, since the page no longer needs to redirect from that url.

Comments

OpenChimp’s picture

Title: Old redirects can't be the same and current node alias » problems caused by conflicts between aliases and redirects

I was thinking a bit more about this and perhaps the solution needs a bit more than just deleting the old redirect that will cause the problem. That is fine in the case described above in which the old redirect points to the same node that the new alias is being created for. But, we also need to check the node id to which the redirect is pointing. If it is the same as the node id we are working on then, as I said, deleting it should be fine, but if someone is creating a new node or modifying a different one, then we need to resolve the conflict in a different way.

Option 1: Treat the situation the same way that duplicate aliases are treated. An integer is appended to the alias to distinguish it from the other node with the same alias. This would just require a lookup in the redirect table in addition to the alias table.
Option 2: Delete the conflicting redirect. This seems risky from an SEO perspective since the redirect may be important.
Option 3: Prompt the user to figure out the appropriate action. Since the older conflicting alias is really just a redirect (which might not even be useful in some cases), then the admin might prefer to delete the old redirect instead of appending a number to the new pages url. They may also wish to change the title of the article (or whatever field is used to generate the url)

Perhaps this is just over-analyzing what is bound to be a rare circumstance. What do you think?

I just ran some tests to see how the module currently behaves. Here are the steps I took:

STEP1 create node (nid-1) with title abc
- creates alias with path /content/abc :: FINE
STEP2 create node (nid-2) with title abc
- creates alias with path /content/abc-0 :: FINE
STEP3 edit node (nid-1) and change title to abcd
- deletes alias /content/abc :: FINE
- creates alias /content/abcd :: FINE
- creates redirect pointing /content/abc to node/1 :: FINE
STEP4 create node (nid-3) with title abc
- creates alias with path /content/abc :: OOPS

After creating nid-3 (STEP4), I can't access it. The redirect (STEP3) takes precedence. So now both node/3 and content/abc take the user to content/abcd (nid-1)

As stated above, this can be resolved by either deleting the redirect that is in conflict with the alias or by using a method like in STEP2 in which the system recognizes that the path alias already exists and creates a variant by adding the integer to the url. I don't really know which is preferable.

greggles’s picture

I followed your steps and tried to repeat this problem and couldn't, which is curious.

Then I upgraded to the latest versions of path redirect and agree that I experience the problem.

I agree with the options you provide and the analysis of them. I don't like the idea of interrupting the normal flow to ask the user what to do because that could be annoying to them. I think that option 1 makes the most sense and that the best solution will be to do that and, if "pathauto verbose" is on, to let the user know that this is what happened and why. By warning users (if verbose is on) then we kind of do a hybrid of Option 1 and Option 3. If the user says "hey, this isn't what I want" when they see the warning then they can go and manually fix it. The warning could even include links to the "edit" pages for the related redirects and aliases.

Does that seem reasonable?

greggles’s picture

Status: Active » Needs review
StatusFileSize
new1.73 KB

So, I provided a patch over at http://drupal.org/node/218277#comment-729758 which addresses the endless redirects inside of path redirect itself. But we still have a separate problem based on your comment #2.

So...I created a patch which checks both the url_alias and, if the table exists, the path_redirect table to determine whether or not an alias exists.

If you can please test both of these I'll commit them and we'll be ready for a Pathauto5.x-2.1!

greggles’s picture

Title: problems caused by conflicts between aliases and redirects » ensure aliases are unique among existing aliases and redirects

more specific title

greggles’s picture

Status: Needs review » Fixed

Ok - fixed for 5.x-2 and HEAD.

OpenChimp’s picture

Sorry for the absenteeism on my part ... I'm still developing good drupal habits.

The solutions you came up with sound great. Using the verbose setting to determine whether to display the message makes perfect sense.

Thanks.

greggles’s picture

In the end I decided not to warn the user. I see this as an extension of determining that aliases are unique and we currently don't warn them about it but just append an index until it works.

Do you still think that we need to warn the user?

Also, can you please test out the patch for path redirect and comment on the issue whether it works or not? I'd like to get it committed to path redirect and having reviews from other people will help.

Thanks.

Anonymous’s picture

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for two weeks with no activity.