WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#45278 closed defect (bug) (duplicate)

Validate transient name lengths

Reported by: dartiss Owned by:
Milestone: Priority: normal
Severity: normal Version: 5.1
Component: Options, Meta APIs Keywords:
Focuses: Cc:

Description

I would like to re-open discussion on #15058. 3 years have passed and I believe I have a stronger argument for reviewing this issue again.

The issue... transient name lengths are not checked and, if too long, may be truncated

When last discussed, from what I can tell, no changes were made other than, via another ticket and for another reason, the field that the transient name is stored in was expanded from 64 characters to 256 characters. However, if the transient name is still too long for the field then it will "silently fail".

The reason why I believe that this is now worthy of further discussion is that one particular point was missed - the two records created for an expiring transient are of different lengths. The prefix for these 2 records are _transient_ and _transient_timeout_. This means that the name for the timeout record is 8 characters less than the primary record. In this case, it's possible for the timeout record to be truncated and for the primary record not to be - the result is a potentially devastating situation where a non-expiring transient is created.

It's unlikely, though, with 256 characters to play with. But not impossible. I would like to see something implemented to protect against this - ideally against any truncation happening but, at minimum, to protect again the above scenario occurring.

set_transient already returns a boolean response to indicate success of the transient being stored - a check of length against the minimum before truncation occurs and returning a false response would, to me, seem to be the most suitable solution. At worse case, if the user code isn't checking for this response, it will expect a transient to be stored and, when it's not there, will generate a new one - at least the data is fresh and I'd like to think this is the better result.

Change History (4)

#1 @swissspidy
3 years ago

  • Keywords close added

#15058 is still open. Unless I‘m missing something, I don‘t see a reason why we can‘t keep discussion in one place.

Last edited 3 years ago by swissspidy (previous) (diff)

#2 @dartiss
3 years ago

  • Resolution set to invalid
  • Status changed from new to closed

Yeah, you're right @swissspidy - my bad. I've not had enough coffee.

I'll close this up and, as you say, transfer the discussion back to that ticket.

#3 @swissspidy
3 years ago

  • Milestone Awaiting Review deleted
  • Resolution changed from invalid to duplicate

Duplicate of #15058.

#4 @SergeyBiryukov
3 years ago

  • Component changed from General to Options, Meta APIs
  • Keywords needs-patch close removed
Note: See TracTickets for help on using tickets.