Hardcoded value of $chunk_size variable in class-IXR.php in wp-includes

[ankit.gade@…]

Variable $chunk_size is used to restrict RAM usage to parse xml-rpc response message if the message is very large.
This variable value has been hard coded to 262144 (256 Kb). A filter should be provided for setting value of this variable.

It has following advantages.

1. If message is large around 4-5 Mb, then getResponse() method of IXR_Client returns null, we can avoid this situation by setting $chunk_size to higher value.

2. If server configuration is well versed, for example: If we have 32 GB RAM on server then allotting 5-8 Mb to $chunk_size should not be an issue and for the same filter can be used.

[ankit.gade@…]

Filter added for setting value of $chunk_size

[georgestephanis]

If a filter is added, it would need to have function_exists() conditionals wrapped around it.

An example of how that's been done in the one other place that it's happened can be seen here:

https://core.trac.wordpress.org/browser/tags/4.0/src/wp-includes/class-IXR.php#L230

[ankit.gade@…]

function_exists() check before applying filter.

[ankit.gade@…]

Thank you. New diff file has been added.

Replying to georgestephanis:

If a filter is added, it would need to have function_exists() conditionals wrapped around it.

An example of how that's been done in the one other place that it's happened can be seen here:

https://core.trac.wordpress.org/browser/tags/4.0/src/wp-includes/class-IXR.php#L230

[ankit.gade@…]

Will anybody notice this bug ? It's an useful thing to do because it creates critical issues during XML-RPC response parsing due to harcoded value.

[georgestephanis]

Please don't set the priority to critical, that's normally reserved for much more 'things are boing boom' issues. I've yet to see an example of things actually breaking due to this hardcoded chunk size.

Also, you just submitted this yesterday. Relax! The soonest it would get added in is potentially 4.1, which won't have a freeze for over a month yet.

There will be a trac scrub on Friday, as noted here - ​https://make.wordpress.org/core/2014/10/14/weekly-bug-scrubs-are-back/ - I'd suggest popping in to IRC then to speak to folks about this in #wordpress-dev.

[westi]

Replying to ankit.gade@…:

Variable $chunk_size is used to restrict RAM usage to parse xml-rpc response message if the message is very large.
This variable value has been hard coded to 262144 (256 Kb). A filter should be provided for setting value of this variable.

It has following advantages.

1. If message is large around 4-5 Mb, then getResponse() method of IXR_Client returns null, we can avoid this situation by setting $chunk_size to higher value.

2. If server configuration is well versed, for example: If we have 32 GB RAM on server then allotting 5-8 Mb to $chunk_size should not be an issue and for the same filter can be used.

Hey, Ankit Gade can you explain these bugs in more detail with some examples.

I'm not sure how change the chunk_size should affect the maximum response size that can be parsed.

It would be good to have some test date / a unit test which shows how this bug can be reproduced.

[ankit.gade@…]

Replying to westi:

Replying to ankit.gade@…:

Variable $chunk_size is used to restrict RAM usage to parse xml-rpc response message if the message is very large.
This variable value has been hard coded to 262144 (256 Kb). A filter should be provided for setting value of this variable.

It has following advantages.

1. If message is large around 4-5 Mb, then getResponse() method of IXR_Client returns null, we can avoid this situation by setting $chunk_size to higher value.

2. If server configuration is well versed, for example: If we have 32 GB RAM on server then allotting 5-8 Mb to $chunk_size should not be an issue and for the same filter can be used.

Hey, Ankit Gade can you explain these bugs in more detail with some examples.

I'm not sure how change the chunk_size should affect the maximum response size that can be parsed.

It would be good to have some test date / a unit test which shows how this bug can be reproduced.

Hello Peter,

Let me give you the scenario on which I am working currently and having such issue. Assume that we have two sites A and B, and I am currently working on site A.

1. I am getting list of users present on site B using xml-rpc, the response message which I am getting is contains array of objects of users present on site B and the response message size is around 3 Mb. When I want to get that response in a variable using getresponse method of IXR_Client class, I am getting null. ( FYI, I have large value set for memory_limit in php.ini )

2. Moreover, we have this piece of code in parse method of class IXR_Message

           do {
             if (strlen($this->message) <= $chunk_size) {
                 $final = true;
             }
             $part = substr($this->message, 0, $chunk_size);
             $this->message = substr($this->message, $chunk_size);
             if (!xml_parse($this->_parser, $part, $final)) {
                 return false;
             }
             if ($final) {
                 break;
             }
         } while (true);
   

If we have somewhat bigger $chunk_size set by filter ( In case we have good server resources ) then we can save the time we are iterating in do-while loop, since each time it will call functions it needs to to put arguments on stack etc.

So, in this case we can parse the message in single iteration.

Let me know if I am clear and in case you have any further query.

Thanks

[wonderboymusic]

In 35095:

XML-RPC: Add a filter, xmlrpc_memory_limit, to allow the value of $xmlrpc_memory_limit to be increased.

Props ankit.gade.
Fixes #29957.

[dd32]

This change feels confusing referencing memory in the way that it does, it's also not really a memory limit as the code now reflects.

I'd probably suggest something odd and vague would be a better option here, xmlrpc_chunk_parsing_size or similar.

[ankit.gade@…]

Changed filter name from xmlrpc_memory_limit to xmlrpc_chunk_parsing_size

[ankit.gade@…]

Replying to dd32:

This change feels confusing referencing memory in the way that it does, it's also not really a memory limit as the code now reflects.

I'd probably suggest something odd and vague would be a better option here, xmlrpc_chunk_parsing_size or similar.

Word Memory has been used because $chunk_size is basically used to deal with efficient RAM usage, but I also agree with you that it may lead to confusion.

I have changed the name of filter and added revised patch.

[wonderboymusic]

In 35279:

XML-RPC: after [35095], change the name of the xmlrpc_memory_limit filter to xmlrpc_chunk_parsing_size.

Props ankit.gade.
Fixes #29957.

[dd32]

In 35364:

Update variable naming after [35279].

See #29957.