WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 5 years ago

Last modified 4 years ago

#30910 closed defect (bug) (fixed)

future post permalink can be revealed when blog article requested by url like ...?p={post_id}

Reported by: e.mazovetskiy Owned by: boonebgorges
Milestone: 4.2 Priority: normal
Severity: normal Version: 4.2
Component: Posts, Post Types Keywords: has-patch
Focuses: Cc:
PR Number:

Description

If your permalink is composed of article's title, it can be easily revealed by brute force on post id, giving a clue to the content of your future post.

Error 404 is rendered in this case but location is being changed from ?p=NNNN, to the permalink of future post, which is completely undesirable sometimes.

There is work around this with filter on 'redirect_canonical', but I think this should be the part of core.

Attachments (4)

30910.patch (547 bytes) - added by e.mazovetskiy 5 years ago.
suggested patch
link-template.php.patch (2.1 KB) - added by CalEvans 5 years ago.
link-template.php.patch
postStatus.php.patch (2.2 KB) - added by CalEvans 5 years ago.
tests/phpunit/tests/query/postStatus.php patch.
30910.diff (677 bytes) - added by DrewAPicture 5 years ago.

Download all attachments as: .zip

Change History (19)

@e.mazovetskiy
5 years ago

suggested patch

#1 @SergeyBiryukov
5 years ago

  • Component changed from Canonical to Posts, Post Types
  • Keywords has-patch added
  • Milestone changed from Awaiting Review to 4.2

#2 follow-up: @SergeyBiryukov
5 years ago

  • Keywords needs-unit-tests added

#3 in reply to: ↑ 2 ; follow-up: @CalEvans
5 years ago

Replying to SergeyBiryukov:

I do not believe - but I would be happy to be corrected - that the patch supplied will solve the problem.

I was able to recreate the problem, but not yet in a unit test. I can recreate it with wget. I believe the problem lies not in get_post_permalink() but in get_permalink().

Again, I could be wrong. I'm posting to get an opinion before I attempt to solve the wrong problem. :)

Cheers1
=C=

#4 in reply to: ↑ 3 @e.mazovetskiy
5 years ago

I did some additional tests. So, get_permalink() works for native post type and get_post_permalink() works for custom post type. So both functions should be patched.

Replying to CalEvans:

Replying to SergeyBiryukov:

I do not believe - but I would be happy to be corrected - that the patch supplied will solve the problem.

I was able to recreate the problem, but not yet in a unit test. I can recreate it with wget. I believe the problem lies not in get_post_permalink() but in get_permalink().

Again, I could be wrong. I'm posting to get an opinion before I attempt to solve the wrong problem. :)

Cheers1
=C=

#5 @CalEvans
5 years ago

Ok, I've now patched both functions. Working on a unit test.

Cheers!
=C=

@CalEvans
5 years ago

link-template.php.patch

@CalEvans
5 years ago

tests/phpunit/tests/query/postStatus.php patch.

#6 @CalEvans
5 years ago

  • Keywords needs-unit-tests removed

2 new assertions added, one for each condition.

Cheers1
=C=

#7 @boonebgorges
5 years ago

  • Owner set to boonebgorges
  • Resolution set to fixed
  • Status changed from new to closed

In 31114:

In get_permalink(), don't resolve to pretty permalink if post has 'future' status.

We already do this for other non-public statuses, to prevent leaking non-public
information about unpublished posts.

Props e.mazovetskiy, CalEvans.
Fixes #30910.

@DrewAPicture
5 years ago

#8 @DrewAPicture
5 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Looks like [31114] inadvertently introduced a regression in terms of how the sample permalink markup is displayed in edit-form-advanced. I actually noticed that while posting on the make/core P2, the sample permalink was no longer visible or editable for scheduled posts in trunk:

http://f.cl.ly/items/2E2H0n0L241I1Y3E1K10/Screen%20Shot%202015-02-01%20at%204.02.01%20PM.png

Turns out, we missed adding the 'future' post status to the exception list in get_sample_permalink(). This is covered in 30910.diff.

Without adding 'future' to the exception list, when get_permalink() is called with the $leavename parameter set to true, the permalink fragment that gets passed through doesn't have the %postname% or %pagename% tag included, which ultimately results in it failing checks for a sample permalink in the UI.

Interesting bug to track down.

Last edited 5 years ago by DrewAPicture (previous) (diff)

#9 @boonebgorges
5 years ago

Thanks for digging this up, DrewAPicture. I've verified the issue and managed to write a unit test to demonstrate the bug and your fix (it was not straightforward to do).

#10 @boonebgorges
5 years ago

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

In 31323:

In get_sample_permalink(), override 'future' status before generating permalink.

In [31114], get_permalink() was modified to prevent pretty permalinks from
being generated for posts with the 'future' post status. This inadvertently
broke the pretty permalink preview for scheduled posts. The fix is to include
the 'future' status in the list of statuses that get_sample_permalink() fakes
as 'publish' before it fetches a permalink.

Props DrewAPicture.
Fixes #30910.

#11 follow-up: @kevkoeh
5 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

This fix creates a disruption in the workflow of authors who use the 'View Post' button on drafts to also get a copy of the full pretty permalink, for use in social media tools and other planning.

Copying that permalink from the edit screen is frequently sub-optimal.

https://cldup.com/zDa7KNkAp1.png

Ideally, the 'View Post' button would still take you to the pretty permalink, but the numeric ?p=123 url would not redirect to the pretty version until the post is published publicly. Is that possible?

#12 in reply to: ↑ 11 @boonebgorges
5 years ago

Replying to kevkoeh:

Ideally, the 'View Post' button would still take you to the pretty permalink, but the numeric ?p=123 url would not redirect to the pretty version until the post is published publicly. Is that possible?

Ah, I hadn't thought about that. Yes: we should be able to use the pretty permalink for the href of the 'View Post' button. That URL will properly route to the scheduled post for users who have the capability to read it.

#13 @boonebgorges
5 years ago

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

In 32002:

After [31114] and [31323], 'View Post' generated in get_sample_permalink_html() should go to pretty permalink.

get_permalink() will return a non-pretty permalink for future posts, which
breaks some user workflows that expect View Post to lead to a page with the
pretty permalink.

Fixes #30910.

#14 @johnbillion
4 years ago

r32002 caused a regression. See #32954.

This ticket was mentioned in Slack in #core by sergey. View the logs.


4 years ago

Note: See TracTickets for help on using tickets.