file_get_contents should probably clear the global state of last headers
Nobody has claimed this yet.
- Dominant language
- C
- Stars
- 40.4k
- Forks
- 8.1k
- Avg merge
- 2d 13h
- Merged PRs (30d)
- 96
Description
Description
The following code:
<?php
file_get_contents(__FILE__);
var_dump(http_get_last_response_headers());
file_get_contents('https://php.net/');
var_dump(http_get_last_response_headers());
file_get_contents(__FILE__);
var_dump(http_get_last_response_headers());
Resulted in this output:
NULL
array( headers from php.net )
array( headers from php.net )
But I expected this output instead:
NULL
array( headers from php.net )
NULL // as reading a file clearly outputs no headers
IMO this is very surprising, and can lead to bugs (see https://github.com/jsonrainbow/json-schema/pull/843 for example) as people are now nudged to switch to http_get_last_response_headers() by the deprecations.
The problem is that $http_response_headers, while a very ugly API, was a local variable.. and http_get_last_response_headers() while appearing cleaner stores global state and can thus leak data across boundaries if you do not call http_clear_last_response_headers() after fetching the headers.
Thus my suggestion would be that file_get_contents first clears the headers and then populates them if the stream wrapper in use has anything to populate.
/cc @Girgias as I know you were involved in the deprecation. And apologies if this was already discussed somewhere I didn't find.
PHP Version
All versions where `http_get_last_response_headers()` is available
Operating System
No response
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by running the PHP reproduction in the issue and reading the implementations of file_get_contents(), http_get_last_response_headers(), and http_clear_last_response_headers(). Trace how stream wrappers update the stored headers, then add coverage showing that a local-file read clears headers left by an earlier HTTP request; done means the final call returns NULL.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- c, php
- Domain
- backend-api-design
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100