WordPress.org
Get WordPress

Make WordPress Core

Opened 15 years ago

Last modified 11 days ago

#16612 new enhancement

WordPress should return nocache headers for requests with comment cookies

Reported by: barry's profile barry Owned by:
Milestone: Priority: normal
Severity: normal Version:
Component: Comments Keywords: has-patch 2nd-opinion reporter-feedback
Focuses: Cc:

Description (last modified by barry)

Most themes, when displaying the comment form, change the HTML to pre-fill username, email address, and website when comment cookies are received in the HTTP request. Since the response does not have explicit nocache headers, per RFC2616 (​http://www.ietf.org/rfc/rfc2616.txt) intermediate caches can use heuristics to determine the cache TTL for the response. Since there is 0 freshness data in the response, it is not really possible to perform good heuristics, but in practice, caches will assign a default TTL to this type of response. The result is that private information input by user A when submitting a comment can be returned to user B when making a request for the same URL.

To protect ourselves against this, we should call nocache_headers() when comment cookies are sent and the comment form is being displayed. Alternatively, we can send nocache headers for all requests with comment cookies regardless of the comment form being displayed or not (probably easier and maybe safer).

​http://humboldtherald.wordpress.com/2011/01/27/gremlins/ is a story likely caused by an aggressive cache and the lack of nocache headers.

Attachments (3)

comment_cookies_nocache.diff​ (1.2 KB) - added by westi 15 years ago.
The fix :)
16612.diff​ (1.3 KB) - added by thomaswm 10 years ago.
Refreshed patch
16612.3.diff​ (1.3 KB) - added by rachelbaker 9 years ago.
Renames the function to wp_check_comment_cookies

Download all attachments as: .zip

Change History (17)

#1 @barry
15 years ago

  • Description modified (diff)

@westi
15 years ago

The fix :)

#2 @nacin
15 years ago

Or, per a discussion, "Vary: Cookie" could be sent.

#3 @sorich87
14 years ago

  • Type changed from defect (bug) to enhancement

#4 @nacin
12 years ago

  • Component changed from General to Bootstrap/Load

#5 @chriscct7
10 years ago

  • Keywords has-patch needs-refresh added

@thomaswm
10 years ago

Refreshed patch

#6 @thomaswm
10 years ago

  • Keywords needs-testing added; needs-refresh removed

Added refreshed version of the patch as 16612.diff​.

#7 @DrewAPicture
10 years ago

#35266 was marked as a duplicate.

#8 @DrewAPicture
10 years ago

  • Component changed from Bootstrap/Load to Comments

#9 @rachelbaker
10 years ago

  • Keywords 4.7-early added

This ticket was mentioned in ​Slack in #core by helen. ​View the logs.
9 years ago

#11 @helen
9 years ago

Was Vary: Cookie ever discussed? If going with the existing patch, it needs a different function name.

@rachelbaker: Still want to do this 4.7-early?

@rachelbaker
9 years ago

Renames the function to wp_check_comment_cookies

#12 @rachelbaker
9 years ago

  • Keywords 2nd-opinion added; 4.7-early removed
  • Milestone changed from Awaiting Review to Future Release

@helen I am not aware of any discussion or plans to implement Vary: Cookie headers. I defer to @wonderboymusic or @markjaquith for a more informed response.

#13 @rmccue
9 years ago

Related: #38060.

#14 @huzaifaalmesbah
11 days ago

  • Keywords reporter-feedback added; needs-testing removed

Tested against trunk (7.0-alpha-61215-src).

Attempted to apply the latest patch (16612.3.diff) using grunt patch:16612, but it does not apply cleanly.

Conflicts:

  • src/wp-includes/class-wp.php (.rej created)

Because the patch no longer applies, testing is not currently possible.

As this patch is ~9 years old, requesting reporter/component maintainer feedback on whether this is still need.

Note: See TracTickets for help on using tickets.