WordPress.org
Get WordPress

Make WordPress Core

Opened 9 years ago

Closed 5 years ago

#36416 closed enhancement (fixed)

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

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

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 9 years ago.
patch with unit tests
36416.diff​ (2.8 KB) - added by kirasong 5 years ago.
Refresh of trimed.patch

Download all attachments as: .zip

Change History (10)

@pbearne
9 years ago

patch with unit tests

#1 @pbearne
9 years ago

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

#2 @DrewAPicture
9 years ago

  • Version changed from trunk to 2.0

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

@kirasong
5 years ago

Refresh of trimed.patch

#4 @kirasong
5 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
5 years ago

  • Milestone set to 5.5
  • Owner set to SergeyBiryukov
  • Status changed from new to reviewing

#6 @SergeyBiryukov
5 years ago

In 47452:

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
5 years ago

In 47453:

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.

See #36416.

#8 @SergeyBiryukov
5 years ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 47454:

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.