Make WordPress Core

Opened 7 weeks ago

Last modified 6 weeks ago

#63502 reviewing defect (bug)

HTTP 500 in `wp-json/batch` with specific arguments

Reported by: bor0's profile bor0 Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: 6.9 Priority: normal
Severity: normal Version: 6.8
Component: REST API Keywords: has-patch has-unit-tests has-test-info dev-feedback
Focuses: Cc:

Description

Hitting the endpoint /wp-json/batch/v1 with specific arguments can result in 500.

To reproduce, use this script:

#!/bin/bash
curl -X POST http://localhost:8080/wp-json/batch/v1 \
-H "Content-Type: application/json" \
-u bor0:asdf \
-d '{
  "requests": [
    {
      "method": "POST",
      "path": "http://user@:80"
    }
  ]
}'

Results in [Mon Jan 27 12:23:43.157697 2025] [php:error] [pid 27451] [client ::1:61293] PHP Fatal error: Uncaught Error: Call to undefined method WP_Error::get_method() in /opt/homebrew/var/www/wp-includes/rest-api/class-wp-rest-server.php:1153

Attachments (3)

63502.patch (3.2 KB) - added by bor0 7 weeks ago.
63502.2.patch (4.3 KB) - added by bor0 7 weeks ago.
63502.3.patch (4.5 KB) - added by SirLouen 7 weeks ago.

Download all attachments as: .zip

Change History (12)

@bor0
7 weeks ago

#1 in reply to: ↑ description @bor0
7 weeks ago

  • Component changed from General to REST API

@SergeyBiryukov could use your help on getting this one resolved :)

Thanks!

#2 @bor0
7 weeks ago

  • Keywords has-patch added

#3 @SirLouen
7 weeks ago

  • Keywords needs-testing added

This ticket was mentioned in PR #8850 on WordPress/wordpress-develop by bor0.


7 weeks ago
#4

  • Keywords has-unit-tests added

@bor0
7 weeks ago

#5 @bor0
7 weeks ago

Attached a new patch with a test case.

#6 @SergeyBiryukov
7 weeks ago

  • Milestone changed from Awaiting Review to 6.9
  • Owner set to SergeyBiryukov
  • Status changed from new to reviewing

@SirLouen
7 weeks ago

#7 @SirLouen
7 weeks ago

  • Keywords changes-requested has-test-info added; needs-testing removed

Combined Bug Reproduction and Patch Test Report

Description

🟠 This report validates that the indicated patch works as expected, but a little change is provided

Patch tested: https://github.com/WordPress/wordpress-develop/pull/8850.diff

Environment

  • WordPress: 6.9-alpha-60093-src
  • PHP: 8.2.28
  • Server: nginx/1.27.5
  • Database: mysqli (Server: 8.4.5 / Client: mysqlnd 8.2.28)
  • Browser: Chrome 137.0.0.0
  • OS: Windows 10/11
  • Theme: Twenty Twenty-Five 1.2
  • MU Plugins: None activated
  • Plugins:
    • Test Reports 1.2.0

Test Reproduction

  • Followed instructions in OP
  • 🐞 Server error triggered

Expected Results

  • Server error should be handled

Actual Results

  1. ✅ Issue resolved with patch.
  2. ✅ Unit tests correctly represent the OP specified scenario

Additional Notes.

  • @bor0 I've reviewed the code, and I think that maybe more errors could be represented with a little refactor. If you can this patch and if you feel that looks good, you can integrate it into your PR or add the necessary changes.

Supplemental Artifacts

Result after the patch

{"responses":[{"body":{"code":"parse_path_failed","message":"Could not parse the path.","data":{"status":400}},"status":400,"headers":[]}]}

@bor0 commented on PR #8850:


7 weeks ago
#8

If you can review this patch with proposed changes

That seems to cause some phpunit issues.

bor0:~/dev/wp-svn$ svn revert -R .
Reverted 'src/wp-includes/rest-api/class-wp-rest-server.php'
Reverted 'tests/phpunit/tests/rest-api/rest-server.php'
bor0:~/dev/wp-svn$ vendor/bin/phpunit --filter REST_Server
Installing...
Running as single site... To run multisite, use -c tests/phpunit/multisite.xml
Not running ajax tests. To execute these, use --group ajax.
Not running ms-files tests. To execute these, use --group ms-files.
Not running external-http tests. To execute these, use --group external-http.
PHPUnit 9.6.23 by Sebastian Bergmann and contributors.

Warning:       Your XML configuration validates against a deprecated schema.
Suggestion:    Migrate your XML configuration using "--migrate-configuration"!

...............................................................  63 / 122 ( 51%)
...........................................................     122 / 122 (100%)

Time: 00:03.295, Memory: 215.00 MB

OK (122 tests, 385 assertions)
bor0:~/dev/wp-svn$ patch -p0 < ~/Downloads/63502.3.patch 
patching file 'src/wp-includes/rest-api/class-wp-rest-server.php'
patching file 'tests/phpunit/tests/rest-api/rest-server.php'
bor0:~/dev/wp-svn$ vendor/bin/phpunit --filter REST_Server
Installing...
Running as single site... To run multisite, use -c tests/phpunit/multisite.xml
Not running ajax tests. To execute these, use --group ajax.
Not running ms-files tests. To execute these, use --group ms-files.
Not running external-http tests. To execute these, use --group external-http.
PHPUnit 9.6.23 by Sebastian Bergmann and contributors.

Warning:       Your XML configuration validates against a deprecated schema.
Suggestion:    Migrate your XML configuration using "--migrate-configuration"!

...............................................................  63 / 123 ( 51%)
.....................................EEEEE....E.............    123 / 123 (100%)

Time: 00:03.341, Memory: 215.00 MB

There were 6 errors:

1) Tests_REST_Server::test_batch_v1_opt_in with data set "missing" (null, false)
Error: Call to undefined method WP_REST_Request::get_all_error_data()

/Users/bor0/dev/wp-svn/src/wp-includes/rest-api.php:3410
/Users/bor0/dev/wp-svn/src/wp-includes/rest-api/class-wp-rest-server.php:214
/Users/bor0/dev/wp-svn/src/wp-includes/rest-api/class-wp-rest-server.php:1827
/Users/bor0/dev/wp-svn/src/wp-includes/rest-api/class-wp-rest-server.php:1292
/Users/bor0/dev/wp-svn/src/wp-includes/rest-api/class-wp-rest-server.php:1125
/Users/bor0/dev/wp-svn/tests/phpunit/includes/spy-rest-server.php:71
/Users/bor0/dev/wp-svn/src/wp-includes/rest-api.php:586
/Users/bor0/dev/wp-svn/tests/phpunit/tests/rest-api/rest-server.php:2052

2) Tests_REST_Server::test_batch_v1_opt_in with data set "invalid type" (true, false)
Error: Call to undefined method WP_REST_Request::get_all_error_data()

/Users/bor0/dev/wp-svn/src/wp-includes/rest-api.php:3410
/Users/bor0/dev/wp-svn/src/wp-includes/rest-api/class-wp-rest-server.php:214
/Users/bor0/dev/wp-svn/src/wp-includes/rest-api/class-wp-rest-server.php:1827
/Users/bor0/dev/wp-svn/src/wp-includes/rest-api/class-wp-rest-server.php:1292
/Users/bor0/dev/wp-svn/src/wp-includes/rest-api/class-wp-rest-server.php:1125
/Users/bor0/dev/wp-svn/tests/phpunit/includes/spy-rest-server.php:71
/Users/bor0/dev/wp-svn/src/wp-includes/rest-api.php:586
/Users/bor0/dev/wp-svn/tests/phpunit/tests/rest-api/rest-server.php:2052

3) Tests_REST_Server::test_batch_v1_opt_in with data set "invalid type string" ('v1', false)
Error: Call to undefined method WP_REST_Request::get_all_error_data()

/Users/bor0/dev/wp-svn/src/wp-includes/rest-api.php:3410
/Users/bor0/dev/wp-svn/src/wp-includes/rest-api/class-wp-rest-server.php:214
/Users/bor0/dev/wp-svn/src/wp-includes/rest-api/class-wp-rest-server.php:1827
/Users/bor0/dev/wp-svn/src/wp-includes/rest-api/class-wp-rest-server.php:1292
/Users/bor0/dev/wp-svn/src/wp-includes/rest-api/class-wp-rest-server.php:1125
/Users/bor0/dev/wp-svn/tests/phpunit/includes/spy-rest-server.php:71
/Users/bor0/dev/wp-svn/src/wp-includes/rest-api.php:586
/Users/bor0/dev/wp-svn/tests/phpunit/tests/rest-api/rest-server.php:2052

4) Tests_REST_Server::test_batch_v1_opt_in with data set "wrong version" (array(true), false)
Error: Call to undefined method WP_REST_Request::get_all_error_data()

/Users/bor0/dev/wp-svn/src/wp-includes/rest-api.php:3410
/Users/bor0/dev/wp-svn/src/wp-includes/rest-api/class-wp-rest-server.php:214
/Users/bor0/dev/wp-svn/src/wp-includes/rest-api/class-wp-rest-server.php:1827
/Users/bor0/dev/wp-svn/src/wp-includes/rest-api/class-wp-rest-server.php:1292
/Users/bor0/dev/wp-svn/src/wp-includes/rest-api/class-wp-rest-server.php:1125
/Users/bor0/dev/wp-svn/tests/phpunit/includes/spy-rest-server.php:71
/Users/bor0/dev/wp-svn/src/wp-includes/rest-api.php:586
/Users/bor0/dev/wp-svn/tests/phpunit/tests/rest-api/rest-server.php:2052

5) Tests_REST_Server::test_batch_v1_opt_in with data set "false version" (array(false), false)
Error: Call to undefined method WP_REST_Request::get_all_error_data()

/Users/bor0/dev/wp-svn/src/wp-includes/rest-api.php:3410
/Users/bor0/dev/wp-svn/src/wp-includes/rest-api/class-wp-rest-server.php:214
/Users/bor0/dev/wp-svn/src/wp-includes/rest-api/class-wp-rest-server.php:1827
/Users/bor0/dev/wp-svn/src/wp-includes/rest-api/class-wp-rest-server.php:1292
/Users/bor0/dev/wp-svn/src/wp-includes/rest-api/class-wp-rest-server.php:1125
/Users/bor0/dev/wp-svn/tests/phpunit/includes/spy-rest-server.php:71
/Users/bor0/dev/wp-svn/src/wp-includes/rest-api.php:586
/Users/bor0/dev/wp-svn/tests/phpunit/tests/rest-api/rest-server.php:2052

6) Tests_REST_Server::test_batch_v1_partial_error
Error: Call to undefined method WP_REST_Request::get_all_error_data()

/Users/bor0/dev/wp-svn/src/wp-includes/rest-api.php:3410
/Users/bor0/dev/wp-svn/src/wp-includes/rest-api/class-wp-rest-server.php:214
/Users/bor0/dev/wp-svn/src/wp-includes/rest-api/class-wp-rest-server.php:1827
/Users/bor0/dev/wp-svn/src/wp-includes/rest-api/class-wp-rest-server.php:1292
/Users/bor0/dev/wp-svn/src/wp-includes/rest-api/class-wp-rest-server.php:1125
/Users/bor0/dev/wp-svn/tests/phpunit/includes/spy-rest-server.php:71
/Users/bor0/dev/wp-svn/tests/phpunit/tests/rest-api/rest-server.php:2281

ERRORS!
Tests: 123, Assertions: 371, Errors: 6.
bor0:~/dev/wp-svn$ 

#9 @SirLouen
6 weeks ago

  • Keywords dev-feedback added; changes-requested removed

Reviewed the errors and see where the issue comes. I think we can't extend it, so I think it can be left as it is now.

I would confirm now that this is ready.

Note: See TracTickets for help on using tickets.