Make WordPress Core

Opened 9 years ago

Closed 9 years ago

#29294 closed defect (bug) (fixed)

There may be lots of 'wp-saving-post-*' left-over/stale cookies

Reported by: azaozz's profile azaozz Owned by: azaozz's profile azaozz
Milestone: 4.0 Priority: normal
Severity: normal Version:
Component: Autosave Keywords:
Focuses: Cc:

Description

Happens when the user is redirected after saving a post. Caused by Chrome and Firefox keeping session (no expiration time) cookies "forever" when the user has selected "Show my windows and tabs from last time" in Firefox or "Continue where you left off" in Chrome.

Attachments (2)

29294.patch (1.9 KB) - added by azaozz 9 years ago.
29294.2.patch (2.4 KB) - added by azaozz 9 years ago.

Download all attachments as: .zip

Change History (8)

#1 @azaozz
9 years ago

According to the specs, cookies set without expiration time or max-age should expire when the browser is closed. This is not the case in newer Firefox and Chrome when the above options are set. There is even a "wontfix" ticket for Chromium.

@azaozz
9 years ago

#2 @azaozz
9 years ago

In 29294.patch:

  • Rename the cookie to wp-saving-post (without post_id) so it gets overwritten, and add the post_id to the value.
  • Set cookie to expire in 1 day.
Version 0, edited 9 years ago by azaozz (next)

#3 @mdawaffe
9 years ago

We discussed this outside of Trac a bit. You mentioned setting max-age as a possible solution, but that setcookie() doesn't expose that functionality.

We could keep the per-post cookie and set the max-age property with a manual call to header( 'Set-Cookie: ...' ).

#4 @azaozz
9 years ago

Yeah, max-age: [seconds]; would work better than expires: [GMT date] for these cookies. However setting a cookie with header( 'Set-Cookie: ...' ) may interfere with using setcookie() (to set other cookies) at least in some PHP versions since 5.2.4.

As these cookies are non-critical, perhaps better not to use that as it may cause bugs that will be very hard to find.

The worst case scenario here (no cookie) means that we will be comparing post title, content and excerpt to the data saved in sessionStorage. In some cases that triggers false positives. A proper cookie also triggers removal of the data from sessionStorage, however (thankfully) it seems sessionStorage is emptied properly on quitting the browser, i.e. it is unaffected by the above browser settings.

@azaozz
9 years ago

#5 @azaozz
9 years ago

29294.2.patch also always compares the post title, content and excerpt when a proper cookie is not set. This prevents another (vary rare) false positive where saving has failed while the user was saving changes to tags, categories, etc.

Last edited 9 years ago by azaozz (previous) (diff)

#6 @azaozz
9 years ago

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

In 29572:

Autosave: prevent setting multiple stale wp-saving-post-* cookies when the browser disregards "session cookies" expiration on quit:

  • Add expiration time of 24 hours for these cookies.
  • Rename them to wp-saving-post (no post_id) so there is never more than one cookie per domain.

Fixes #29294.

Note: See TracTickets for help on using tickets.