Make WordPress Core

Context Navigation

← Previous Ticket
Next Ticket →

Opened 10 years ago

Closed 6 years ago

#36416 closed enhancement (fixed)

maybe_unserialize returns false if non trimmed string is passed but is_serialized returns true

Reported by:	pbearne	Owned by:	SergeyBiryukov
Priority:	normal	Milestone:	5.5
Component:	General	Version:	2.0
Severity:	normal	Keywords:	has-patch has-unit-tests 2nd-opinion
Cc:		Focuses:

Description

In maybe_unserialize we use is_serialized to protect the call to unserialize but is_serialized trim's the string so fails to protect unserialize call

Added a trim and updated code formatting to fix

Attachments (2)

trimed.patch (3.1 KB ) - added by pbearne 10 years ago.: patch with unit tests
36416.diff (2.8 KB ) - added by kirasong 6 years ago.: Refresh of trimed.patch

Download all attachments as: .zip

Change History (10)

@pbearne
10 years ago

Attachment trimed.patch added

patch with unit tests

#1 @pbearne
10 years ago

Keywords has-patch has-unit-tests dev-feedback added

#2 @DrewAPicture
10 years ago

Version trunk → 2.0

This ticket was mentioned in Slack in #core by noisysocks. View the logs.

6 years ago

@kirasong
6 years ago

Attachment 36416.diff added

Refresh of trimed.patch

#4 @kirasong
6 years ago

Keywords 2nd-opinion added; dev-feedback removed

We chatted about this ticket in [today's Triage](https://wordpress.slack.com/archives/C02RQBWTW/p1583989966303400).

I went ahead and refreshed the current patch in 36416.diff .

Swapping dev-feedback to second-opinion per discussion so this can get attention + review.

#5 @SergeyBiryukov
6 years ago

Milestone → 5.5
Owner set to SergeyBiryukov
Status new → reviewing

#6 @SergeyBiryukov
6 years ago

Tests: Extract is_serialized() test cases into data providers; reuse them for maybe_serialize() and maybe_unserialize() tests.

Props pbearne, mikeschroder, SergeyBiryukov.
See #36416.

#7 @SergeyBiryukov
6 years ago

General: Move maybe_serialize() to a more appropriate place in the file, before maybe_unserialize().

Rename the $original parameter of maybe_unserialize() to $data, for consistency with other serialization functions.

#8 @SergeyBiryukov
6 years ago

Resolution → fixed
Status reviewing → closed

General: Trim the input data in maybe_unserialize(), for consistency with is_serialized().

Props pbearne, mikeschroder.
Fixes #36416.

Note: See TracTickets for help on using tickets.

Download in other formats: