Make WordPress Core

Opened 7 months ago

Closed 7 months ago

Last modified 7 months ago

#59336 closed defect (bug) (fixed)

Update edit term link generation while generating row action links

Reported by: thelovekesh's profile thelovekesh Owned by: audrasjb's profile audrasjb
Milestone: 6.4 Priority: normal
Severity: normal Version: 6.3.1
Component: Taxonomy Keywords: has-patch php81 has-unit-tests commit
Focuses: administration, php-compatibility Cc:

Description

Due to the last parameter being passed as null in add_query_arg(), row action links generated by WP_Terms_List_Table::handle_row_actions() with PHP 8.2+ cause deprecation warnings.

[29-Aug-2023 18:22:43 UTC] PHP Deprecated:  strstr(): Passing null to parameter #1 ($haystack) of type string is deprecated in /wp-includes/functions.php on line 1144
[29-Aug-2023 18:22:43 UTC] PHP Deprecated:  stripos(): Passing null to parameter #1 ($haystack) of type string is deprecated in /wp-includes/functions.php on line 1151
[29-Aug-2023 18:22:43 UTC] PHP Deprecated:  stripos(): Passing null to parameter #1 ($haystack) of type string is deprecated in /wp-includes/functions.php on line 1154
[29-Aug-2023 18:22:43 UTC] PHP Deprecated:  str_contains(): Passing null to parameter #1 ($haystack) of type string is deprecated in /wp-includes/functions.php on line 1161
[29-Aug-2023 18:22:43 UTC] PHP Deprecated:  str_contains(): Passing null to parameter #1 ($haystack) of type string is deprecated in /wp-includes/functions.php on line 1164

This occurs when a user lacks the capability to edit_term and get_edit_term_link() returns null.

Change History (12)

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


7 months ago
#1

Restrict edit term link generation if the user lacks the edit_term cap.

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

#2 @thelovekesh
7 months ago

  • Type changed from enhancement to defect (bug)

#3 @jrf
7 months ago

  • Keywords needs-unit-tests php81 added

I have reviewed the PR and would very much like to see this change safeguarded with a test.

@thelovekesh commented on PR #5198:


7 months ago
#4

@jrfnl Since the above patch only eliminates a variable assignment and uses the expression directly, I'm not sure if it actually necessitates a test case. The function add_query_arg(), becomes the cause of the issue when passing the URL as null in it and it should be handled separately IMO.

Should I add a test case to Tests_Functions::test_add_query_arg() to assert deprecations? But in my opinion, this is outside the purview of this PR. Please point me in the right direction.

@jrf commented on PR #5198:


7 months ago
#5

@jrfnl Since the above patch only eliminates a variable assignment and uses the expression directly, I'm not sure if it actually necessitates a test case. The function add_query_arg(), becomes the cause of the issue when passing the URL as null in it and it should be handled separately IMO.

Sorry, but no. The add_query_arg() function is doing things 100% correctly. This function is the one _doing it wrong_ as it is passing data which is not within the accepted data types for the add_query_arg() function.

So, it is this function which needs a test proving the PHP 8.1 issue and safeguarding the fix.

@jrf commented on PR #5198:


7 months ago
#6

@jrfnl Since the above patch only eliminates a variable assignment and uses the expression directly, I'm not sure if it actually necessitates a test case. The function add_query_arg(), becomes the cause of the issue when passing the URL as null in it and it should be handled separately IMO.

Sorry, but no. The add_query_arg() function is doing things 100% correctly. This function is the one _doing it wrong_ as it is passing data which is not within the accepted data types for the add_query_arg() function.

So, it is this function which needs a test proving the PHP 8.1 issue and safeguarding the fix.

#7 @jrf
7 months ago

  • Milestone changed from Awaiting Review to 6.4

#8 @jrf
7 months ago

I've re-reviewed the PR after the update by @thelovekesh. Aside from some small updates to the tests which are needed, this is looking good and should be ready to go soon.

#9 @jrf
7 months ago

  • Keywords has-unit-tests commit added; needs-unit-tests removed

The requested updates have been made. This patch, including the associated tests, are good to go as far as I'm concerned.

Adding the commit keyword. 👍🏻

#10 @audrasjb
7 months ago

  • Owner set to audrasjb
  • Status changed from new to reviewing

Self assigning for final review and commit.

#11 @audrasjb
7 months ago

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

In 56631:

Taxonomy: Restrict term edit link generation in WP_Terms_List_Table::handle_row_actions().

This changeset restricts edit term link generation if the user lacks the edit_term cap in order to prevent PHP 8.1+ deprecations shown when a user lacks this
capability and get_edit_term_link() returns null.

Props thelovekesh, jrf.
Fixes #59336.

Note: See TracTickets for help on using tickets.