Repository: laravel/framework (php)

Issue:
HTTP client open streams causing "too many open files" error
<!-- DO NOT THROW THIS AWAY -->
<!-- Fill out the FULL versions with patch versions -->

- Laravel Version: 8.58.0
- PHP Version: 8.0.1
- Database Driver & Version: n/a

### Description:

Laravel 8.37 introduced an issue where standard HTTP GET requests using the Laravel HTTP client would leave the request stream open until the script completed. Given sufficient requests, the script will eventually fail with a "too many open files" error.

### Steps To Reproduce:

Run multiple HTTP::get() requests.
```
for($i=0; $i<10; $i++){
    $response = Http::get('https://google.com');
    sleep(5);
}
```
While the script is running, use `lsof` on the command line to watch the open files count grow.

`lsof -u forge | grep "ESTABLISHED"` (tested on a forge server under the forge user)

```
php      56418 forge    7u     IPv4             194346      0t0        TCP http-debug:50600->lga25s74-in-f14.1e100.net:https (ESTABLISHED)
php      56418 forge    8u     IPv4             194359      0t0        TCP http-debug:45022->lga25s71-in-f4.1e100.net:https (ESTABLISHED)
php      56418 forge   11u     IPv4             194838      0t0        TCP http-debug:50604->lga25s74-in-f14.1e100.net:https (ESTABLISHED)
php      56418 forge   12u     IPv4             194849      0t0        TCP http-debug:45026->lga25s71-in-f4.1e100.net:https (ESTABLISHED)
php      56418 forge   15u     IPv4             195291      0t0        TCP http-debug:50608->lga25s74-in-f14.1e100.net:https (ESTABLISHED)
php      56418 forge   16u     IPv4             195302      0t0        TCP http-debug:45030->lga25s71-in-f4.1e100.net:https (ESTABLISHED)
php      56418 forge   19u     IPv4             195463      0t0        TCP http-debug:50612->lga25s74-in-f14.1e100.net:https (ESTABLISHED)
php      56418 forge   20u     IPv4             195474      0t0        TCP http-debug:45034->lga25s71-in-f4.1e100.net:https (ESTABLISHED)
php      56418 forge   23u     IPv4             195488      0t0        TCP http-debug:50616->lga25s74-in-f14.1e100.net:https (ESTABLISHED)
php      56418 forge   24u     IPv4             195499      0t0        TCP http-debug:45038->lga25s71-in-f4.1e100.net:https (ESTABLISHED)
php      56418 forge   27u     IPv4             195514      0t0        TCP http-debug:50620->lga25s74-in-f14.1e100.net:https (ESTABLISHED)
php      56418 forge   28u     IPv4             195525      0t0        TCP http-debug:45042->lga25s71-in-f4.1e100.net:https (ESTABLISHED)
php      56418 forge   31u     IPv4             195539      0t0        TCP http-debug:50624->lga25s74-in-f14.1e100.net:https (ESTABLISHED)
php      56418 forge   32u     IPv4             195550      0t0        TCP http-debug:45046->lga25s71-in-f4.1e100.net:https (ESTABLISHED)
php      56418 forge   35u     IPv4             195563      0t0        TCP http-debug:50628->lga25s74-in-f14.1e100.net:https (ESTABLISHED)
php      56418 forge   36u     IPv4             195574      0t0        TCP http-debug:45050->lga25s71-in-f4.1e100.net:https (ESTABLISHED)
php      56418 forge   39u     IPv4             195588      0t0        TCP http-debug:50632->lga25s74-in-f14.1e100.net:https (ESTABLISHED)
php      56418 forge   40u     IPv4             195599      0t0        TCP http-debug:45054->lga25s71-in-f4.1e100.net:https (ESTABLISHED)
php      56418 forge   43u     IPv4             195612      0t0        TCP http-debug:50636->lga25s74-in-f14.1e100.net:https (ESTABLISHED)
php      56418 forge   44u     IPv4             195623      0t0        TCP http-debug:45058->lga25s71-in-f4.1e100.net:https (ESTABLISHED)
```

<!-- If possible, please provide a GitHub repository to demonstrate your issue -->
<!-- laravel new bug-report --github="--public" -->


Pull request:
[8.x] Return a new or existing guzzle client based on context
## Issue
This PR resolves a breaking change in backwards compatibility between Laravel Framework 8.36.2 and 8.37. Laravel 8.37 introduced support for concurrent asynchronous requests in the HTTP client #36948. As part of that change, a protected $client property was introduced in the PendingRequest class which stores a reference to the Guzzle client in order to reuse it for asynchronous request pooling. An unintended side effect of this update occurs when making synchronous calls through the HTTP client. The reused client appears to hold references to every request made in a script, keeping all streams (temporary files) open until the script completes. This results in a "too many open files" error when sufficient HTTP requests are made in a process #38641. In our case that number was ~300 running in a queued job.

## Proposed Solution
This update restores the previous functionality of returning a new Guzzle client for each standard HTTP request. In the updated buildClient method, the PendingRequest will check if either a Guzzle client has been set using the setClient method, or if the call is asynchronous ($this->async), if so, it will continue to use the $this->client property. If not, it will return a new client as it had previously. The change supports the new pooling feature without breaking backwards compatibility.

## Further Development
It does seem the appearance of this issue hints at a further refactoring that may be necessary at some point. While the async calls don't seem to create the same open files issue, manually setting a Guzzle client using the setClient method may still be affected by the underlying problem depending on configuration of that client. I suspect the source of the problem may be that open references to Guzzle Request objects in the reused Guzzle Client are preventing them from closing their streams and being garbage collected as the requests in the recreated client are when it goes out of scope, but I am uncertain.

Patch:
diff --git a/src/Illuminate/Http/Client/PendingRequest.php b/src/Illuminate/Http/Client/PendingRequest.php
index d5426fb09c..40a3a87557 100644
--- a/src/Illuminate/Http/Client/PendingRequest.php
+++ b/src/Illuminate/Http/Client/PendingRequest.php
@@ -798,6 +798,28 @@ protected function populateResponse(Response $response)
      * @return \GuzzleHttp\Client
      */
     public function buildClient()
+    {
+        return $this->requestsReusableClient()
+               ? $this->getReusableClient()
+               : $this->createClient($this->buildHandlerStack());
+    }
+
+    /**
+     * Determine if a reusable client is required.
+     *
+     * @return bool
+     */
+    protected function requestsReusableClient()
+    {
+        return ! is_null($this->client) || $this->async;
+    }
+
+    /**
+     * Retrieve a reusable Guzzle client.
+     *
+     * @return \GuzzleHttp\Client
+     */
+    protected function getReusableClient()
     {
         return $this->client = $this->client ?: $this->createClient($this->buildHandlerStack());
     }
diff --git a/src/Illuminate/Http/Client/Request.php b/src/Illuminate/Http/Client/Request.php
index f8699b5ac4..0e493f1fa9 100644
--- a/src/Illuminate/Http/Client/Request.php
+++ b/src/Illuminate/Http/Client/Request.php
@@ -120,9 +120,7 @@ public function header($key)
      */
     public function headers()
     {
-        return collect($this->request->getHeaders())->mapWithKeys(function ($values, $header) {
-            return [$header => $values];
-        })->all();
+        return $this->request->getHeaders();
     }
 
     /**
diff --git a/src/Illuminate/Http/Client/Response.php b/src/Illuminate/Http/Client/Response.php
index dc25cb830a..70672f3ea7 100644
--- a/src/Illuminate/Http/Client/Response.php
+++ b/src/Illuminate/Http/Client/Response.php
@@ -107,9 +107,7 @@ public function header(string $header)
      */
     public function headers()
     {
-        return collect($this->response->getHeaders())->mapWithKeys(function ($v, $k) {
-            return [$k => $v];
-        })->all();
+        return $this->response->getHeaders();
     }
 
     /**


Test patch:
diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml
index a47297b0a7..61b9f3f509 100644
--- a/.github/workflows/tests.yml
+++ b/.github/workflows/tests.yml
@@ -91,10 +91,10 @@ jobs:
       matrix:
         php: ['7.3', '7.4', '8.0']
         stability: [prefer-lowest, prefer-stable]
-        include:
-          - php: '8.1'
-            flags: "--ignore-platform-req=php"
-            stability: prefer-stable
+        # include:
+        #   - php: '8.1'
+        #     flags: "--ignore-platform-req=php"
+        #     stability: prefer-stable
 
     name: PHP ${{ matrix.php }} - ${{ matrix.stability }} - Windows
 
diff --git a/tests/Encryption/EncrypterTest.php b/tests/Encryption/EncrypterTest.php
index e439c6786b..aee0872c8a 100755
--- a/tests/Encryption/EncrypterTest.php
+++ b/tests/Encryption/EncrypterTest.php
@@ -102,7 +102,7 @@ public function testThatAnAeadTagCantBeModified()
         $this->expectException(DecryptException::class);
         $this->expectExceptionMessage('Could not decrypt the data.');
 
-        $data->tag = 'A'.substr($data->tag, 1, 23);
+        $data->tag[0] = $data->tag[0] === 'A' ? 'B' : 'A';
         $encrypted = base64_encode(json_encode($data));
         $e->decrypt($encrypted);
     }