Make WordPress Core

Opened 10 years ago

Closed 10 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 10 years ago.
29294.2.patch (2.4 KB) - added by azaozz 10 years ago.

Download all attachments as: .zip

Change History (8)

#1 @azaozz
10 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
10 years ago

#2 @azaozz
10 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 the cookie to expire in 1 day.
Last edited 10 years ago by azaozz (previous) (diff)

#3 @mdawaffe
10 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
10 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
10 years ago

#5 @azaozz
10 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 10 years ago by azaozz (previous) (diff)

#6 @azaozz
10 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.