Make WordPress Core

Opened 4 months ago

Last modified 8 weeks ago

#58986 reopened defect (bug)

TypeError: Unsupported operand types: string * int *

Reported by: nurielmeni's profile nurielmeni Owned by:
Milestone: 6.5 Priority: normal
Severity: critical Version: 6.2.2
Component: Date/Time Keywords: has-patch has-unit-tests needs-testing dev-feedback
Focuses: rest-api, php-compatibility Cc:

Description (last modified by sabernhardt)

Path: /wp-includes/rest-api/endpoints/class-wp-rest-posts-controller.php
File: class-wp-rest-posts-controller.php
Line: 1833

Expression Error: get_option('gmt_offset') * HOUR_IN_SECONDS
Rais Exception: TypeError: Unsupported operand types: string * int

Suggested Fix: intval(get_option('gmt_offset')) * HOUR_IN_SECONDS

Thanks

Attachments (3)

58986.patch (900 bytes) - added by nihar007 4 months ago.
Patch added. I added intval function
ticket-patch-58986.diff (821 bytes) - added by mhshohel 4 months ago.
Implemented a patch to perform a type cast on 'gmt_offset'. Additionally, incorporated a standard practice by introducing a default value of 0.
58986.diff (8.7 KB) - added by rleeson 3 months ago.
Add common retrieval for GMT offset as a float with default of 0.

Download all attachments as: .zip

Change History (16)

#1 @sabernhardt
4 months ago

  • Component changed from REST API to Date/Time
  • Description modified (diff)
  • Focuses rest-api added
  • Keywords needs-patch added

Hi and thanks for the ticket!

WP_REST_Posts_Controller::prepare_item_for_response() probably needs the same change as the current_time() function in #57035. Note: the offset is not always an integer (#57998).

#2 @SergeyBiryukov
4 months ago

  • Milestone changed from Awaiting Review to 6.4

@nihar007
4 months ago

Patch added. I added intval function

@mhshohel
4 months ago

Implemented a patch to perform a type cast on 'gmt_offset'. Additionally, incorporated a standard practice by introducing a default value of 0.

#3 @mhshohel
4 months ago

  • Keywords has-patch added; needs-patch removed

Resolved the problem of unsupported operand types: string * int *. Additionally, adhered to the standard practice by setting 0 as the default value.

#4 @rleeson
3 months ago

  • Keywords needs-patch added; has-patch removed
  • Resolution set to invalid
  • Status changed from new to closed

GMT offsets can be fractional, as noted in this ticket 57035 comment so intval or casting to an int cause regression for those fractional time zones.

#5 @sabernhardt
3 months ago

  • Resolution invalid deleted
  • Status changed from closed to reopened

Reopening, as this would need a (new) patch that considers fractional offsets.

This ticket was mentioned in PR #5092 on WordPress/wordpress-develop by Zebedeu.


3 months ago
#6

  • Keywords has-patch added; needs-patch removed

Previously, using the function or casting to to convert the GMT offset to a fractional decimal number could cause regression problems for fractional timezones. Now, we use the function to correctly convert the GMT offset to a decimal number. This resolves the issue of incorrect conversion of fractional timezones. fixes:

Trac ticket: https://core.trac.wordpress.org/ticket/58986

props marcio-zebedeu

@rleeson
3 months ago

Add common retrieval for GMT offset as a float with default of 0.

#7 @rleeson
3 months ago

  • Keywords has-unit-tests added
  • New method added current_gmt_offset which reads the offset and type casts to float, with a default of 0.
  • Replaced all Core instances of get_option( 'gmt_offset' ) with the function to guarantee a float in every instance, avoiding type cast issues/errors.
  • Added Unit Tests to the date and datetime groups
  • Also addresses ticket #57035.

This patch is here.

This ticket was mentioned in PR #5094 on WordPress/wordpress-develop by @rleeson.


3 months ago
#8

Edge cases of a missing/invalid GMT offset can cause type casting error. Additionally, Core contained multiple direct calls to retrieve this option with an inconsistent usage and an apparent misconception in some resolutions that the offset was an integer, where it is actually a float/double.

To address the issue, I added current_gtm_offset to ensure a valid float, with default offset of 0, and further replaced all instances of the direct data retrieval with this function as a proxy.

Trac tickets: Ticket #58986 and Ticket # 57035

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


2 months ago

#10 @oglekler
2 months ago

  • Keywords needs-testing dev-feedback added

This ticket was discussed during a bug scrub.

It needs testing and dev review. Please check that Unit tests are covering all possible cases and address both declared tickets.

Add props: @mukesh27

Last edited 2 months ago by oglekler (previous) (diff)

This ticket was mentioned in Slack in #core-test by oglekler. View the logs.


2 months ago

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


8 weeks ago

#13 @nicolefurlan
8 weeks ago

  • Milestone changed from 6.4 to 6.5

This ticket was discussed in today's bug scrub.

We decided to move this (and #57035) to 6.5. Per @hellofromTonya:

I'm concerned about both of these tickets going in this late in the release due to the lack of test coverage and testing. Why? Casting with inval() for the one and (float) for the other may have side effects. Thinking it needs a contextual dive into how the code got to the current state to avoid potentially introducing a regression.

@SergeyBiryukov or @rarst if you have time to review and have confidence in this fix, don't hesitate to pull it back into the 6.4 milestone.

Props to @hellofromTonya :)

Note: See TracTickets for help on using tickets.