Make WordPress Core

Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#48046 closed defect (bug) (fixed)

Add array cast before count() in class-wp-xml-server.php

Reported by: bitcomplex's profile bitcomplex Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: 5.3 Priority: normal
Severity: normal Version:
Component: XML-RPC Keywords: has-patch
Focuses: docs Cc:

Description

<?php
/**
         * Checks if the method received at least the minimum number of arguments.
         *
         * @since 3.4.0
         *
         * @param string|array $args Sanitize single string or array of strings.
         * @param int $count         Minimum number of arguments.
         * @return bool if `$args` contains at least $count arguments.
         */
        protected function minimum_args( $args, $count ) {
                if ( count( $args ) < $count ) {
                        $this->error = new IXR_Error( 400, __( 'Insufficient arguments passed to this XML-RPC method.' ) );
                        return false;
                }
                return true;
        }

There should be an array-cast of $args prior to the call to count(). If $args is a string you'll get Warning: count(): Parameter must be an array or an object that implements Countable if running php 7.3 ...

Attachments (1)

48046.patch (594 bytes) - added by dkarfa 5 years ago.

Download all attachments as: .zip

Change History (9)

#1 follow-up: @SergeyBiryukov
5 years ago

  • Component changed from General to XML-RPC
  • Focuses docs added
  • Milestone changed from Awaiting Review to 5.3

Hi @bitcomplex, welcome to WordPress Trac! Thanks for the report.

It looks like the function documentation is incorrect, it should always receive an array. I haven't found any instances in core where it receives a string. This appears to be the case since the introduction in [20636].

Have you run into an issue where it receives a string? I think we should just correct the documentation here.

Last edited 5 years ago by SergeyBiryukov (previous) (diff)

#2 in reply to: ↑ 1 @bitcomplex
5 years ago

Replying to SergeyBiryukov:

Hi @bitcomplex, welcome to WordPress Trac! Thanks for the report.

It looks like the function documentation is incorrect, it should always receive an array. I haven't found any instances in core where it receives a string. This appears to be the case since the introduction in [20636].

Have you run into an issue where it receives a string? I think we should just correct the documenation here.

Yes, we've run into it. Screenshot from Sentry:
https://drive.google.com/file/d/1zGzjj3VDCxIoTi5RyEoCUjaICyaGxzld/view?usp=sharing

Thanks for your quick reply!

@dkarfa
5 years ago

#3 @SergeyBiryukov
5 years ago

  • Keywords has-patch added; needs-patch removed
  • Owner set to SergeyBiryukov
  • Status changed from new to reviewing

#4 @SergeyBiryukov
5 years ago

For reference, the current documentation for $args appears to be a copy/paste from the ::escape() method.

#5 @dkarfa
5 years ago

I believe the Docs is updated already.

        /**
	 * Checks if the method received at least the minimum number of arguments.
	 *
	 * @since 3.4.0
	 *
	 * @param string|array $args Sanitize single string or array of strings.
	 * @param int $count         Minimum number of arguments.
	 * @return bool if `$args` contains at least $count arguments.
	 */
	protected function minimum_args( $args, $count ) {
		if ( count( (array)$args ) < $count ) {
			$this->error = new IXR_Error( 400, __( 'Insufficient arguments passed to this XML-RPC method.' ) );
			return false;
		}

		return true;
	}

#6 @SergeyBiryukov
5 years ago

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

In 46148:

XML-RPC: Avoid a PHP warning in wp_xmlrpc_server::minimum_args() if $args is not an array.

Correct the documentation to clarify that array is the only acceptable type for $args.

Props bitcomplex, dkarfa.
Fixes #48046.

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


5 years ago

#8 @SergeyBiryukov
5 years ago

#48073 was marked as a duplicate.

Note: See TracTickets for help on using tickets.