WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#9559 closed defect (bug) (fixed)

Site search results can include passworded posts

Reported by: coffee2code Owned by: ShaneF
Milestone: 2.8 Priority: normal
Severity: normal Version: 2.7.1
Component: General Keywords: has-patch
Focuses: Cc:

Description

By default, WordPress's built-in search feature searches the contents of passworded posts. If the content of a passworded post
matches the search criteria, WordPress will include that post in the listing of search results. While it is true that the
post contents will not be displayed to the visitor (unless they know and have entered the password for the post), the fact that
the otherwise protected post appeared in the search results allows for the visitor to search-bomb your site in an effort to deduce
some of the content of the password-protected post.

Of course, external search (as done from Google) would never include the passworded post contents since that content is not available to external search engines.

I have released a plugin that addresses the issue and prevents passworded posts from being included in search results, but this may be something we may want to consider for core.

The attached patch prevents passworded posts from being included in search results on the front-end of the site (i.e. by visitors). It does not add the constraint on searches performed within the admin.

The drawback, of course, is that a visitor couldn't legitimately perform a search and find a passworded post that they may have the password for. Hence a privacy vs. usability issue, and I vote that privacy prevails. (I've seen the search-bomb happen, so it's a real concern.)

Attachments (1)

bug9559.omit-passworded-posts-from-search.diff (648 bytes) - added by coffee2code 5 years ago.
Aforementioned patch

Download all attachments as: .zip

Change History (12)

coffee2code5 years ago

Aforementioned patch

comment:1 Elpie5 years ago

+1 for this. Until a site is search-bombed most users would be unaware that their password-protected posts are appearing in search result listings.
The change works and it would be great to see this make it into 2.8 final.

comment:2 ShaneF5 years ago

  • Owner changed from anonymous to ShaneF
  • Status changed from new to assigned

I'll test it later.

comment:3 follow-up: ryan5 years ago

There's no index on post_password, so this will likely seriously degrade query performance. I believe that's why it is not done now. Maybe this is acceptable if done only for search requests. Also, maybe we change from !is_admin to is_user_logged_in() so logged in users can still search from the front page.

comment:4 CiaranG5 years ago

  • Cc ciaran@… added

comment:6 in reply to: ↑ 3 Denis-de-Bernardy5 years ago

Replying to ryan:

There's no index on post_password, so this will likely seriously degrade query performance.

No so. Or rather, only if there are multitudes of posts that have a password. Else, it'll just filter out a result here and there.

Also note that, if the vast majority of posts happen to have no passwords, and you look for posts without one as the only criteria in your query, the optimizer will simply ignore that index: A seq scan, on which you filter out those that have a faster, will generally be faster.

In fact, the likelihood is that you can even toss the following in, without any noticeable degradation of speed:

AND ( post_password = '' OR post_author = $user_id )

The optimizer would then filter the results returned by the index used based on that criteria, like this:

nested loop on index
  for each row
    check the above
    break at $limit rows

comment:7 ryan5 years ago

Right you are. I just EXPLAINed and compared the two.

comment:8 ryan5 years ago

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

(In [11106]) Don't show password protected posts in searches for non-logged in users. Props coffee2code. fixes #9559

comment:9 ryan5 years ago

I changed to use !is_user_logged_in(). If there are problems with that, shout.

comment:10 Denis-de-Bernardy5 years ago

For sure, you might have users of membership plugins that will complain about that decision, but it can be filtered out by the plugins themselves.

comment:11 coffee2code5 years ago

Looks good to me.

Note: See TracTickets for help on using tickets.