Make WordPress Core

Opened 19 years ago

Closed 18 years ago

Last modified 18 years ago

#3039 closed defect (bug) (fixed)

<?php wp_link_pages(); ?> related bug in all versions of WP

Reported by: resist's profile resist Owned by: markjaquith's profile markjaquith
Milestone: Priority: normal
Severity: minor Version: 2.1
Component: General Keywords: has-patch 2nd-opinion
Focuses: Cc:

Description

Even if <?php wp_link_pages(); ?> is not included in a theme, there is a bug in all versions of WP. Adding of a random (page) number to any of your articles, say you have mysite.info/2006/01/21/my-stupid-post/ so add any number, more than 1, thus it looks like mysite.info/2006/01/21/my-stupid-post/5 - the result is an ugly article page without text body.

Attachments (1)

wp_link_pages_fix.diff (573 bytes) - added by markjaquith 19 years ago.
fix for /trunk/

Download all attachments as: .zip

Change History (17)

#1 @markjaquith
19 years ago

  • Owner changed from anonymous to markjaquith
  • Severity changed from major to minor
  • Status changed from new to assigned

Why would anyone be going to this URL for an unpaged entry?

#2 @resist
19 years ago

Any constructive solution to a bug?

People will do that if you have multi-page article and they navigate by page number at the end of the url just to see what happens if they enter a nonexistent page.

If you dont want to help out with fixing it, please re-assign it to another person. Thanks

#3 @markjaquith
19 years ago

I was prompting you (the reporter) to elaborate on why you think this is a bug and under which plausible circumstances the bug could be presented (which you answered in your second sentence). I assign bugs to myself when I am interested in helping solve them, or when I want more information from the reporter before proceeding.

I'm working on a patch now. The solution may not be optimal, because the way it currently works, I have no way of finding out how many pages exist within the entry until I attempt to echo out the content of the entry (in get_the_content() ).

@markjaquith
19 years ago

fix for /trunk/

#4 @markjaquith
19 years ago

  • Keywords has-patch 2nd-opinion added
  • Version changed from 2.0.4 to 2.1

Patch does the following:

  • Checks to see if the requested $page is greater than the number of total pages
  • If it is, $page is reduced to equal the total number of pages (i.e. the highest numbered page).

thus, for a post with 4 pages, requesting page 5 or any page above 4 will give you page 4.

#5 @ryan
19 years ago

Wondering if we should 404 or redirect to keep the links canonical. Oh well, this way is simple. :-)

#6 @ryan
19 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

(In [4106]) Don't page off the end. Props Mark J. fixes #3039

#7 @RuddO
18 years ago

The patch should issue a 301 permanent redirect to the highest page number, otherwise a malicious person could exploit this by creating a page in their web host that points to a post in your blog, adding /1, /2 /.... 1000. Then google would get angry at your blog and say "tons of duplicate content" in SERPs.

#8 @RuddO
18 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

#9 @markjaquith
18 years ago

RuddO,

I considered that, but it's too late at that point to do a 301... headers already sent.

Also, this vulnerability already exists in that you can specify multiple URLs for a post that'll lead to the same content, but have a different URL.

#10 @RuddO
18 years ago

To steal one of Colbert's famous lines, "I feel you, man". But shouldn't the check to see if that's a valid post page number be done earlier? I mean, at post retrieval time, when the $posts array is being composed (I think) it's not too late to emit headers, then a small check like the one you've written could be implemented, something like if (is_Single or is_page) then (if page number over bounds) then do_redirect

#11 @RuddO
18 years ago

OTOH, if what I'm advocating is tchnically impossible, the fix should include post text that gives out some sort of consolation error message "Click here to read the full story" with a hyperlink to the last page of the post.

#12 @resist
18 years ago

so what's the last word? Should people be using 4106 or the one in the attachments? BTW thx for taking care of this problem!

#13 @ryan
18 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

(In [4228]) Don't page off the end. Props Mark J. fixes #3039

#14 @foolswisdom
18 years ago

  • Milestone set to 2.0.5

#15 @foolswisdom
18 years ago

This was checked in Trunk in [4106].

#16 @(none)
18 years ago

  • Milestone 2.0.5 deleted

Milestone 2.0.5 deleted

Note: See TracTickets for help on using tickets.