WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 3 years ago

#18157 closed defect (bug) (fixed)

Warning message "Undefined property: WP_Http_Curl::$headers" on action=do-plugin-upgrade

Reported by: chrisjean Owned by: dd32
Milestone: 3.3 Priority: normal
Severity: normal Version:
Component: HTTP API Keywords: has-patch commit
Focuses: Cc:

Description

After updating trunk today, I upgraded the plugins on my test site and saw the following warning message when I went to the plugins page:

PHP Notice:  Undefined property: WP_Http_Curl::$headers in wp-includes/class-http.php on line 1107, referer: wp-admin/update-core.php?action=do-plugin-upgrade

The warning went away when I refreshed the page, so this is not a persistent condition.

It looks like the following is happening:

  1. An instance of the WP_Http_Curl has its request method called.
  2. The instance's headers instance variable is unset as part of the request method's cleanup.
  3. The instance's request method is called again, this time with a blocking arg of not true.
  4. Since blocking is not true, the WP_Http_Curl::stream_handlers instance method is not added as the CURLOPT_HEADERFUNCTION.
  5. Since stream_handlers does not fire, the WP_Http_Curl::headers instance variable isn't populated as the instance variable is still unset and could only be set/updated by the WP_Http_Curl::stream_handlers method.

I would think that a line as simple as the following at the top of the request method could solve the problem:

$this->headers = ''

However, since I'm not fully aware of how everything needs to function, I suppose that this could cause issues.

If this could work, I wonder if it would be better to simply set the variable to an empty string rather than unsetting it as I can also see a scenario where a warning would be created on subsequent requests regardless of the value of the blocking arg as the stream_headers method uses concat to set the headers variable which will cause a warning if the variable isn't set (which it won't be after each run).

This approach is reflected in my attached patch. I'd definitely like it to be reviewed (preferably by dd32 as it looks like he wrote the code) before any consideration for being added.

Attachments (1)

undefined-property-wp_http_curl-headers.diff (447 bytes) - added by chrisbliss18 3 years ago.

Download all attachments as: .zip

Change History (4)

comment:1 nacin3 years ago

Been meaning to create a ticket for this as well.

comment:2 dd323 years ago

  • Keywords commit added; needs-testing removed
  • Milestone changed from Awaiting Review to 3.3
  • Owner set to dd32
  • Status changed from new to accepted

Patch looks like the right thing to me

comment:3 ryan3 years ago

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

In [18457]:

Empty headers instance var instead of unsetting to avoid warning. Props chrisbliss18. fixes #18157

Note: See TracTickets for help on using tickets.