Error in slug parsing leads to unlimited URLs for the same article = duplicate content

[thermoman]

an example says more than 1000 words:

right url:

​http://ahmongwoman.wordpress.com/2010/09/02/i-am-not-hmong-and-i-dont-speak-spanish/

wrong urls:

​http://ahmongwoman.wordpress.com/2010/09/02/i-am-not------hmong-and-i-dont-speak-spanish/

​http://ahmongwoman.wordpress.com/2010/09/02/i----am-not-hmong-and-i-dont-speak-spanish/

​http://ahmongwoman.wordpress.com/2010/09/02/i-am-not-----hmong-and-i-------dont-speak---------spanish/

Problem:

Wordpress returns the article with HTTP status code 200 for the wrong URLs.

This is a serious issue for people regarding search engine optimization (duplicate content).

Expected results:

Wordpress returns HTTP Status code 301 with Location-Header and right URL to the client.

[thermoman]

Tested with several wordpress blogs - not only with wordpress.com hosted blogs.

Bug is confirmed to be an issue in 3.0 and 2.5

[kawauso]

Discussed in #wordpress and explained <meta rel="canonical" />.

Behaviour with multiple dashes is inconsistent with behaviour for incomplete URLs.

[scribu]

Shouldn't redirect_canonical() be taking care of this?

[dd32]

Shouldn't redirect_canonical() be taking care of this?

Thats where it should be handled, However, Canonical at present mainly covers redirecting query vars to rewritten url's, along with common duplicate content url's (such as those using %category%).

My patch on #12456 should be able to deal with this case.

[wonderboymusic]

Kudos to all of us for letting these bugs sit so long - the dd32 patch fixes this as well

[markjaquith]

What about HTTP vs HTTPS? Wouldn't this change redirect from HTTPS to HTTP? That would be a breaking change. I think the check has to be more conservative and just make sure the post slug exists in the URL, or do a direct slug comparison.

[wonderboymusic]

Now wrapped in set_url_scheme

[nacin]

Needs unit tests like whoa.

[atimmer]

I have added ​tests.14773.1.diff which tests common wordpress canonical functionality and adds this ticket to it as well, for which the tests fail.

According to the tests it only happens if all the words in the URL are already present, when only 1 word of the page_name is present the URL will be right because the page_name is not complete.

Edit:
Adding to this, the patch ​12456.2.diff fixes only 1 of the testcases.
It fixes: "/this--should-be-resolved-" -> "/this-should-be-resolved/"
It does not fix: "/this----should---be---resolved-" -> "/this-should-be-resolved/"

[duck_]

Replying to atimmer:

I have added ​tests.14773.1.diff which tests common wordpress canonical functionality and adds this ticket to it as well, for which the tests fail.

Thanks for the tests! Not a big deal, but I don't really like the name of the class. Maybe we just need to increase the number of tests in Tests_Canonical which actually already has a test for this ticket.

Edit:
Adding to this, the patch ​12456.2.diff fixes only 1 of the testcases.
It fixes: "/this--should-be-resolved-" -> "/this-should-be-resolved/"
It does not fix: "/this----should---be---resolved-" -> "/this-should-be-resolved/"

This isn't a problem with the patch since these URLs 404 on trunk. It looks like the problem is that the multiple -s are being picked up as octet placeholders by sanitize_title_with_dashes(), i.e. ---be--- becomes %be. Maybe we should open a ticket to make a better placeholder.

[atimmer]

​12456.3.diff refreshes the patch against current trunk and the new repository structure.

[atimmer]

Added ​tests.14773.2.diff

It seemed like the canonical test class already has a test for this ticket, I have added tests to test the dashes at the front and the back of the url. I also refreshed the patch and the patch fixes all 3 test cases.

[nacin]

Hi atimmer, could you combine your tests and the patch together?

[atimmer]

​12456.4.diff combines the unit tests and the fix.

[boonebgorges]

Contains .4.patch plus the extracted existing test. See #30284.

[SergeyBiryukov]

#39833 was marked as a duplicate.

[SergeyBiryukov]

Related: #17653

[ivantipov]

What would be the repercussions of turning off sanitization on query to fix this, as such?

function raw_sanitize_title($title, $raw_title, $context) {
    if ($context === 'query') {
        return $raw_title;
    }

    return $title;
}
add_filter('sanitize_title', 'raw_sanitize_title', 10, 3);

[SergeyBiryukov]

#54471 was marked as a duplicate.

[SergeyBiryukov]

#57177 was marked as a duplicate.

[SergeyBiryukov]

#57177 was marked as a duplicate.

[SergeyBiryukov]

#57980 was marked as a duplicate.