Skip to content

Commit 612dbc7

Browse files
FelixJacobiFelixJacobi
committed
Fixed cookie extraction in CurlTransport in PHP 8 (fixes #76)
Moved getHeadersAndContentFromCurlHandle to CurlTransport as it not usuable and testable as standalone util
1 parent 59d78f7 commit 612dbc7

11 files changed

Lines changed: 223 additions & 73 deletions

File tree

‎.gitignore‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,10 @@ composer.phar
1010
.php_cs.cache
1111
cghooks.lock
1212

13+
# PHPUnit reports (for convenience when working locally)
14+
reports
15+
16+
# PHPUnit result cache
1317
.phpunit.result.cache
1418

1519
# Local environment variables

‎src/Http/Transport/Bridge/PsrHttpClient/PsrHttpClientTransport.php‎

Lines changed: 3 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222

2323
use BigBlueButton\Exceptions\NetworkException;
2424
use BigBlueButton\Exceptions\RuntimeException;
25-
use BigBlueButton\Http\SetCookie;
25+
use BigBlueButton\Http\Transport\Cookie;
2626
use BigBlueButton\Http\Transport\TransportInterface;
2727
use BigBlueButton\Http\Transport\TransportRequest;
2828
use BigBlueButton\Http\Transport\TransportResponse;
@@ -133,23 +133,8 @@ public function request(TransportRequest $request): TransportResponse
133133
throw new NetworkException('Bad response.', $psrResponse->getStatusCode());
134134
}
135135

136-
$jsessionCookie = null;
137-
138-
foreach ($psrResponse->getHeader('Set-Cookie') as $headerValue) {
139-
$cookie = SetCookie::fromString($headerValue);
140-
141-
if ($cookie->getName() === 'JSESSIONID') {
142-
$value = $cookie->getValue();
143-
144-
if ('' === $value) {
145-
break;
146-
}
147-
148-
$jsessionCookie = $value;
149-
150-
break;
151-
}
152-
}
136+
$headerValues = $psrResponse->getHeader('Set-Cookie');
137+
$jsessionCookie = Cookie::extractJsessionId($headerValues);
153138

154139
return new TransportResponse($psrResponse->getBody()->getContents(), $jsessionCookie);
155140
}

‎src/Http/Transport/Bridge/SymfonyHttpClient/SymfonyHttpClientTransport.php‎

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222

2323
use BigBlueButton\Exceptions\NetworkException;
2424
use BigBlueButton\Exceptions\RuntimeException;
25-
use BigBlueButton\Http\SetCookie;
25+
use BigBlueButton\Http\Transport\Cookie;
2626
use BigBlueButton\Http\Transport\TransportInterface;
2727
use BigBlueButton\Http\Transport\TransportRequest;
2828
use BigBlueButton\Http\Transport\TransportResponse;
@@ -153,19 +153,7 @@ private static function extractJsessionCookie(ResponseInterface $symfonyResponse
153153
$responseHeaders = $symfonyResponse->getHeaders();
154154

155155
if (isset($responseHeaders['set-cookie'])) {
156-
foreach ($responseHeaders['set-cookie'] as $headerValue) {
157-
$cookie = SetCookie::fromString($headerValue);
158-
159-
if ($cookie->getName() === 'JSESSIONID') {
160-
$value = $cookie->getValue();
161-
162-
if ('' === $value) {
163-
return null;
164-
}
165-
166-
return $value;
167-
}
168-
}
156+
return Cookie::extractJsessionId($responseHeaders['set-cookie']);
169157
}
170158

171159
return null;

‎src/Http/Transport/Cookie.php‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* This file is part of littleredbutton/bigbluebutton-api-php.
7+
*
8+
* littleredbutton/bigbluebutton-api-php is free software: you can redistribute it and/or modify
9+
* it under the terms of the GNU Lesser General Public License as published by
10+
* the Free Software Foundation, either version 3 of the License, or
11+
* (at your option) any later version.
12+
*
13+
* littleredbutton/bigbluebutton-api-php is distributed in the hope that it will be useful,
14+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
15+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
16+
* GNU Lesser General Public License for more details.
17+
*
18+
* You should have received a copy of the GNU Lesser General Public License
19+
* along with littleredbutton/bigbluebutton-api-php. If not, see <http://www.gnu.org/licenses/>.
20+
*/
21+
namespace BigBlueButton\Http\Transport;
22+
23+
use BigBlueButton\Http\SetCookie;
24+
25+
/**
26+
* Cookie extraction utils
27+
*
28+
* @internal
29+
*/
30+
final class Cookie
31+
{
32+
/**
33+
* @param string[] $headerValues
34+
* @return string|null
35+
*/
36+
public static function extractJsessionId(array $headerValues): ?string
37+
{
38+
foreach ($headerValues as $headerValue) {
39+
$cookie = SetCookie::fromString($headerValue);
40+
41+
if ($cookie->getName() === 'JSESSIONID') {
42+
$value = $cookie->getValue();
43+
44+
if ('' === $value) {
45+
return null;
46+
}
47+
48+
return $value;
49+
}
50+
}
51+
52+
return null;
53+
}
54+
}

‎src/Http/Transport/CurlTransport.php‎

Lines changed: 64 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -102,17 +102,7 @@ public function request(TransportRequest $request): TransportResponse
102102
}
103103
// @codeCoverageIgnoreEnd
104104

105-
// Needed to store the JSESSIONID
106-
$cookieFile = @tmpfile(); // Silenced as tmpfile can throw notices like "creating file in system temporary directory".
107-
// @codeCoverageIgnoreStart
108-
if (false === $cookieFile) {
109-
throw new RuntimeException('Could not create temporary file for cookies.');
110-
}
111-
// @codeCoverageIgnoreEnd
112-
$cookieFilePath = stream_get_meta_data($cookieFile)['uri'];
113-
114105
$options = self::mergeCurlOptions(
115-
self::buildCookieOptions($cookieFilePath),
116106
self::buildUrlOptions($request),
117107
self::buildPostOptions($request),
118108
self::INTERNAL_CURL_OPTIONS,
@@ -123,12 +113,7 @@ public function request(TransportRequest $request): TransportResponse
123113
curl_setopt($ch, $option, $value);
124114
}
125115

126-
$data = curl_exec($ch);
127-
// @codeCoverageIgnoreStart
128-
if ($data === false) {
129-
throw new NetworkException('Error during curl_exec. Error: ' . curl_error($ch));
130-
}
131-
// @codeCoverageIgnoreEnd
116+
[$headers, $data] = self::getHeadersAndContentFromCurlHandle($ch);
132117

133118
$httpCode = curl_getinfo($ch, CURLINFO_HTTP_CODE);
134119
if ($httpCode < 200 || $httpCode >= 300) {
@@ -137,29 +122,14 @@ public function request(TransportRequest $request): TransportResponse
137122

138123
curl_close($ch);
139124

140-
$cookies = file_get_contents($cookieFilePath);
141125
$sessionId = null;
142-
143-
if (strpos($cookies, 'JSESSIONID') !== false) {
144-
preg_match('/(?:JSESSIONID\s*)(?<JSESSIONID>.*)/', $cookies, $output);
145-
$sessionId = $output['JSESSIONID'];
126+
if (isset($headers['set-cookie'])) {
127+
$sessionId = Cookie::extractJsessionId($headers['set-cookie']);
146128
}
147129

148130
return new TransportResponse($data, $sessionId);
149131
}
150132

151-
/**
152-
* @param string $cookieFilePath
153-
* @return mixed[]
154-
*/
155-
private static function buildCookieOptions(string $cookieFilePath): array
156-
{
157-
return [
158-
CURLOPT_COOKIEFILE => $cookieFilePath,
159-
CURLOPT_COOKIEJAR => $cookieFilePath,
160-
];
161-
}
162-
163133
/**
164134
* @param TransportRequest $request
165135
* @return mixed[]
@@ -169,7 +139,6 @@ private static function buildPostOptions(TransportRequest $request): array
169139
$options = [];
170140

171141
if ('' !== $payload = $request->getPayload()) {
172-
$options[CURLOPT_HEADER] = 0;
173142
$options[CURLOPT_CUSTOMREQUEST] = 'POST';
174143
$options[CURLOPT_POST] = 1;
175144
$options[CURLOPT_POSTFIELDS] = $payload;
@@ -222,4 +191,65 @@ private static function mergeCurlOptions(array ...$options): array
222191

223192
return ArrayHelper::mergeRecursive(true, $headerOptions, ...$options);
224193
}
194+
195+
/**
196+
* A raw response as returned from cURL will contain the headers followed by "\r\n\r\n" and the content.
197+
*
198+
* @param \CurlHandle|resource $curlHandle
199+
* @return array{0: string, 1: string[]} First key headers, second key is content
200+
* @throws NetworkException
201+
* @link https://stackoverflow.com/questions/10589889/returning-header-as-array-using-curl
202+
*/
203+
private static function getHeadersAndContentFromCurlHandle($curlHandle): array
204+
{
205+
/** @noinspection PhpElementIsNotAvailableInCurrentPhpVersionInspection */
206+
// @codeCoverageIgnoreStart
207+
if (\PHP_VERSION_ID >= 80000 && !$curlHandle instanceof \CurlHandle) {
208+
/** @noinspection PhpElementIsNotAvailableInCurrentPhpVersionInspection */
209+
throw new \InvalidArgumentException(sprintf('$curlHandle must be "%s". "%s" given.', \CurlHandle::class, get_debug_type($curlHandle)));
210+
} elseif (\PHP_VERSION_ID < 80000 && !is_resource($curlHandle)) {
211+
throw new \InvalidArgumentException(sprintf('$curlHandle must be resource. "%s" given.', is_object($curlHandle) ? get_class($curlHandle) : gettype($curlHandle)));
212+
}
213+
// @codeCoverageIgnoreEnd
214+
215+
$headers = [];
216+
217+
curl_setopt($curlHandle, CURLOPT_HEADER, 1);
218+
$responseContent = curl_exec($curlHandle);
219+
220+
// @codeCoverageIgnoreStart
221+
if (false === $responseContent) {
222+
throw new NetworkException('Error during curl_exec. Error: ' . curl_error($curlHandle));
223+
}
224+
// @codeCoverageIgnoreEnd
225+
226+
$headerSize = curl_getinfo($curlHandle, CURLINFO_HEADER_SIZE);
227+
$headerContent = substr($responseContent, 0, $headerSize);
228+
229+
// Split the string on every "double" new line.
230+
$headerParts = explode("\r\n\r\n", $headerContent, 2); // only split once to mitigate scrapping content if it contains newlines with carriage return
231+
232+
// Loop of response headers. The "count() -1" is to avoid an empty row for the extra line break before the body of the response.
233+
for ($index = 0; $index < count($headerParts) - 1; $index++) {
234+
foreach (explode("\r\n", $headerParts[$index]) as $i => $line) {
235+
if (0 === $i) {
236+
// HTTP code
237+
continue;
238+
}
239+
240+
$splitHeader = explode(': ', $line, 2);
241+
// @codeCoverageIgnoreStart
242+
if (!isset($splitHeader[0], $splitHeader[1])) {
243+
throw new \InvalidArgumentException(sprintf('Header value "%s" is invalid. Expected format is "Header-Name: value".', $line));
244+
}
245+
// @codeCoverageIgnoreEnd
246+
247+
[$header, $value] = $splitHeader;
248+
// Use lower case to have an always predictable result
249+
$headers[strtolower($header)][] = $value;
250+
}
251+
}
252+
253+
return [$headers, substr($responseContent, $headerSize)];
254+
}
225255
}

‎src/Http/Transport/Header.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ public static function mergeCurlHeaders(array ...$headers) : array
4848
}
4949

5050
$splitHeader = explode(': ', $header, 2);
51-
if (!isset($splitHeader[0]) || !isset($splitHeader[1])) {
51+
if (!isset($splitHeader[0], $splitHeader[1])) {
5252
throw new \InvalidArgumentException(sprintf('Header value "%s" is invalid. Expected format is "Header-Name: value".', $header));
5353
}
5454

‎tests/integration/Http/Transport/CurlTransportTest.php‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525

2626
/**
2727
* @covers \BigBlueButton\Http\Transport\CurlTransport
28+
* @uses \BigBlueButton\Http\Transport\Cookie
2829
* @uses \BigBlueButton\Http\Transport\TransportRequest
2930
* @uses \BigBlueButton\Http\Transport\TransportResponse
3031
* @uses \BigBlueButton\Util\ArrayHelper
@@ -121,10 +122,6 @@ public function testRequestWithoutPayload(): void
121122

122123
public function testRequestWithCookie(): void
123124
{
124-
if (\PHP_VERSION_ID >= 80000) {
125-
self::markTestSkipped('Broken on PHP 8. To be fixed in https://github.com/littleredbutton/bigbluebutton-api-php/pull/78.');
126-
}
127-
128125
$request = new TransportRequest('http://localhost:8057/cookie.php', '', 'application/xml');
129126
$transport = new CurlTransport();
130127

@@ -159,4 +156,14 @@ public function testRequestWithDuplicatedHeader(): void
159156
$this->assertSame('FOO', $dump['input'], 'input echo is correct');
160157
$this->assertSame('3', $dump['vars']['HTTP_CONTENT_LENGTH'], 'Content-Length echo is correct');
161158
}
159+
160+
public function testRequestWithDoubleNewLine(): void
161+
{
162+
$request = new TransportRequest('http://localhost:8057/double-newline.php', '', 'application/xml');
163+
$transport = new CurlTransport();
164+
165+
$response = $transport->request($request);
166+
167+
$this->assertSame("Foo\r\n\r\nBar\r\n", $response->getBody(), 'body is correct');
168+
}
162169
}

‎tests/integration/Http/Transport/Fixtures/web/double-newline.php‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* This file is part of littleredbutton/bigbluebutton-api-php.
7+
*
8+
* littleredbutton/bigbluebutton-api-php is free software: you can redistribute it and/or modify
9+
* it under the terms of the GNU Lesser General Public License as published by
10+
* the Free Software Foundation, either version 3 of the License, or
11+
* (at your option) any later version.
12+
*
13+
* littleredbutton/bigbluebutton-api-php is distributed in the hope that it will be useful,
14+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
15+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
16+
* GNU Lesser General Public License for more details.
17+
*
18+
* You should have received a copy of the GNU Lesser General Public License
19+
* along with littleredbutton/bigbluebutton-api-php. If not, see <http://www.gnu.org/licenses/>.
20+
*/
21+
if ('cli-server' !== \PHP_SAPI) {
22+
// safe guard against unwanted execution
23+
throw new \Exception("You cannot run this script directly, it's a fixture for TestHttpServer.");
24+
}
25+
26+
echo "Foo\r\n\r\n";
27+
echo "Bar\r\n";

‎tests/unit/Http/Transport/Bridge/PsrHttpClient/PsrHttpClientTransportTest.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@
3535

3636
/**
3737
* @covers \BigBlueButton\Http\Transport\Bridge\PsrHttpClient\PsrHttpClientTransport
38-
* @uses \BigBlueButton\Http\SetCookie
38+
* @uses \BigBlueButton\Http\Transport\Cookie
3939
* @uses \BigBlueButton\Http\Transport\TransportRequest
4040
* @uses \BigBlueButton\Http\Transport\TransportResponse
4141
*/

‎tests/unit/Http/Transport/Bridge/SymfonyHttpClient/SymfonyHttpClientTransportTest.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@
3535

3636
/**
3737
* @covers \BigBlueButton\Http\Transport\Bridge\SymfonyHttpClient\SymfonyHttpClientTransport
38-
* @uses \BigBlueButton\Http\SetCookie
38+
* @uses \BigBlueButton\Http\Transport\Cookie
3939
* @uses \BigBlueButton\Http\Transport\TransportRequest
4040
* @uses \BigBlueButton\Http\Transport\TransportResponse
4141
* @uses \BigBlueButton\Util\ArrayHelper

0 commit comments

Comments
 (0)