Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[1.x] Sends content length to address OpenSSL issue #167

Merged
merged 2 commits into from
Apr 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/Protocols/Pusher/Http/Controllers/ChannelController.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@

use Laravel\Reverb\Protocols\Pusher\MetricsHandler;
use Laravel\Reverb\Servers\Reverb\Http\Connection;
use Laravel\Reverb\Servers\Reverb\Http\Response;
use Psr\Http\Message\RequestInterface;
use React\Promise\PromiseInterface;
use Symfony\Component\HttpFoundation\JsonResponse;

class ChannelController extends Controller
{
Expand All @@ -20,6 +20,6 @@ public function __invoke(RequestInterface $request, Connection $connection, stri
return app(MetricsHandler::class)->gather($this->application, 'channel', [
'channel' => $channel,
'info' => ($info = $this->query['info']) ? $info.',occupied' : 'occupied',
])->then(fn ($channel) => new JsonResponse((object) $channel));
])->then(fn ($channel) => new Response((object) $channel));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,9 @@
use Laravel\Reverb\Protocols\Pusher\Concerns\InteractsWithChannelInformation;
use Laravel\Reverb\Protocols\Pusher\MetricsHandler;
use Laravel\Reverb\Servers\Reverb\Http\Connection;
use Laravel\Reverb\Servers\Reverb\Http\Response;
use Psr\Http\Message\RequestInterface;
use React\Promise\PromiseInterface;
use Symfony\Component\HttpFoundation\JsonResponse;
use Symfony\Component\HttpFoundation\Response;

class ChannelUsersController extends Controller
{
Expand All @@ -24,15 +23,15 @@ public function __invoke(RequestInterface $request, Connection $connection, stri
$channel = $this->channels->find($channel);

if (! $channel) {
return new JsonResponse((object) [], 404);
return new Response((object) [], 404);
}

if (! $this->isPresenceChannel($channel)) {
return new JsonResponse((object) [], 400);
return new Response((object) [], 400);
}

return app(MetricsHandler::class)
->gather($this->application, 'channel_users', ['channel' => $channel->name()])
->then(fn ($connections) => new JsonResponse(['users' => $connections]));
->then(fn ($connections) => new Response(['users' => $connections]));
}
}
4 changes: 2 additions & 2 deletions src/Protocols/Pusher/Http/Controllers/ChannelsController.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@

use Laravel\Reverb\Protocols\Pusher\MetricsHandler;
use Laravel\Reverb\Servers\Reverb\Http\Connection;
use Laravel\Reverb\Servers\Reverb\Http\Response;
use Psr\Http\Message\RequestInterface;
use React\Promise\PromiseInterface;
use Symfony\Component\HttpFoundation\JsonResponse;

class ChannelsController extends Controller
{
Expand All @@ -20,6 +20,6 @@ public function __invoke(RequestInterface $request, Connection $connection, stri
return app(MetricsHandler::class)->gather($this->application, 'channels', [
'filter' => $this->query['filter_by_prefix'] ?? null,
'info' => $this->query['info'] ?? null,
])->then(fn ($channels) => new JsonResponse(['channels' => array_map(fn ($item) => (object) $item, $channels)]));
])->then(fn ($channels) => new Response(['channels' => array_map(fn ($item) => (object) $item, $channels)]));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@

use Laravel\Reverb\Protocols\Pusher\MetricsHandler;
use Laravel\Reverb\Servers\Reverb\Http\Connection;
use Laravel\Reverb\Servers\Reverb\Http\Response;
use Psr\Http\Message\RequestInterface;
use React\Promise\PromiseInterface;
use Symfony\Component\HttpFoundation\JsonResponse;

class ConnectionsController extends Controller
{
Expand All @@ -18,6 +18,6 @@ public function __invoke(RequestInterface $request, Connection $connection, stri
$this->verify($request, $connection, $appId);

return app(MetricsHandler::class)->gather($this->application, 'connections')
->then(fn ($connections) => new JsonResponse(['connections' => count($connections)]));
->then(fn ($connections) => new Response(['connections' => count($connections)]));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,9 @@
use Laravel\Reverb\Protocols\Pusher\EventDispatcher;
use Laravel\Reverb\Protocols\Pusher\MetricsHandler;
use Laravel\Reverb\Servers\Reverb\Http\Connection;
use Laravel\Reverb\Servers\Reverb\Http\Response;
use Psr\Http\Message\RequestInterface;
use React\Promise\PromiseInterface;
use Symfony\Component\HttpFoundation\JsonResponse;
use Symfony\Component\HttpFoundation\Response;

use function React\Promise\all;

Expand All @@ -31,7 +30,7 @@ public function __invoke(RequestInterface $request, Connection $connection, stri
$validator = $this->validator($payload);

if ($validator->fails()) {
return new JsonResponse($validator->errors(), 422);
return new Response($validator->errors(), 422);
}

$items = collect($payload['batch']);
Expand All @@ -56,11 +55,11 @@ public function __invoke(RequestInterface $request, Connection $connection, stri

if ($items->contains(fn ($item) => ! empty($item))) {
return all($items)->then(function ($items) {
return new JsonResponse(['batch' => array_map(fn ($item) => (object) $item, $items)]);
return new Response(['batch' => array_map(fn ($item) => (object) $item, $items)]);
});
}

return new JsonResponse(['batch' => (object) []]);
return new Response(['batch' => (object) []]);
}

/**
Expand Down
9 changes: 4 additions & 5 deletions src/Protocols/Pusher/Http/Controllers/EventsController.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,9 @@
use Laravel\Reverb\Protocols\Pusher\EventDispatcher;
use Laravel\Reverb\Protocols\Pusher\MetricsHandler;
use Laravel\Reverb\Servers\Reverb\Http\Connection;
use Laravel\Reverb\Servers\Reverb\Http\Response;
use Psr\Http\Message\RequestInterface;
use React\Promise\PromiseInterface;
use Symfony\Component\HttpFoundation\JsonResponse;
use Symfony\Component\HttpFoundation\Response;

class EventsController extends Controller
{
Expand All @@ -30,7 +29,7 @@ public function __invoke(RequestInterface $request, Connection $connection, stri
$validator = $this->validator($payload);

if ($validator->fails()) {
return new JsonResponse($validator->errors(), 422);
return new Response($validator->errors(), 422);
}

$channels = Arr::wrap($payload['channels'] ?? $payload['channel'] ?? []);
Expand All @@ -51,10 +50,10 @@ public function __invoke(RequestInterface $request, Connection $connection, stri
if (isset($payload['info'])) {
return app(MetricsHandler::class)
->gather($this->application, 'channels', ['info' => $payload['info'], 'channels' => $channels])
->then(fn ($channels) => new JsonResponse(['channels' => array_map(fn ($channel) => (object) $channel, $channels)]));
->then(fn ($channels) => new Response(['channels' => array_map(fn ($channel) => (object) $channel, $channels)]));
}

return new JsonResponse((object) []);
return new Response((object) []);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,9 @@
use Laravel\Reverb\ServerProviderManager;
use Laravel\Reverb\Servers\Reverb\Contracts\PubSubProvider;
use Laravel\Reverb\Servers\Reverb\Http\Connection;
use Laravel\Reverb\Servers\Reverb\Http\Response;
use Psr\Http\Message\RequestInterface;
use React\Promise\PromiseInterface;
use Symfony\Component\HttpFoundation\JsonResponse;
use Symfony\Component\HttpFoundation\Response;

class UsersTerminateController extends Controller
{
Expand All @@ -24,7 +23,7 @@ public function __invoke(RequestInterface $request, Connection $connection, stri
'type' => 'terminate',
'application' => serialize($this->application),
'payload' => ['user_id' => $userId],
])->then(fn () => new JsonResponse((object) []));
])->then(fn () => new Response((object) []));
}

$connections = collect($this->channels->connections());
Expand All @@ -35,6 +34,6 @@ public function __invoke(RequestInterface $request, Connection $connection, stri
}
});

return new JsonResponse((object) []);
return new Response((object) []);
}
}
18 changes: 18 additions & 0 deletions src/Servers/Reverb/Http/Response.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
<?php

namespace Laravel\Reverb\Servers\Reverb\Http;

use Symfony\Component\HttpFoundation\JsonResponse;

class Response extends JsonResponse
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to name this specifically ReverbResponse so it's more clear in error logs when this response is logged?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Possibly though that doesn't match the naming of anything else in that directory.

I'm waiting on a test to verify this actually resolves the issue anyway so will wait until then before making any further changes.

{
/**
* Create a new Http response instance.
*/
public function __construct(mixed $data = null, int $status = 200, array $headers = [], bool $json = false)
{
parent::__construct($data, $status, $headers, $json);

$this->headers->set('Content-Length', strlen($this->content));
}
}
20 changes: 20 additions & 0 deletions tests/Feature/Protocols/Pusher/Reverb/ChannelControllerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,15 @@
expect($response->getBody()->getContents())->toBe('{"occupied":true,"subscription_count":1}');
});

it('can send the content-length header', function () {
subscribe('test-channel-one');
subscribe('test-channel-one');

$response = await($this->signedRequest('channels/test-channel-one?info=user_count,subscription_count,cache'));

expect($response->getHeader('Content-Length'))->toBe(['40']);
});

it('can gather data for a single channel', function () {
$this->usingRedis();

Expand Down Expand Up @@ -121,3 +130,14 @@
expect($response->getStatusCode())->toBe(200);
expect($response->getBody()->getContents())->toBe('{"occupied":true,"subscription_count":1}');
});

it('can send the content-length header when gathering results', function () {
$this->usingRedis();

subscribe('test-channel-one');
subscribe('test-channel-one');

$response = await($this->signedRequest('channels/test-channel-one?info=user_count,subscription_count,cache'));

expect($response->getHeader('Content-Length'))->toBe(['40']);
});
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,19 @@
await($this->signedRequest('channels/presence-test-channel/users'));
})->throws(ResponseException::class);

it('can send the content-length header', function () {
$channel = app(ChannelManager::class)
->for(app()->make(ApplicationProvider::class)->findByKey('reverb-key'))
->findOrCreate('presence-test-channel');
$channel->subscribe($connection = new FakeConnection('test-connection-one'), validAuth($connection->id(), 'presence-test-channel', $data = json_encode(['user_id' => 1, 'user_info' => ['name' => 'Taylor']])), $data);
$channel->subscribe($connection = new FakeConnection('test-connection-two'), validAuth($connection->id(), 'presence-test-channel', $data = json_encode(['user_id' => 2, 'user_info' => ['name' => 'Joe']])), $data);
$channel->subscribe($connection = new FakeConnection('test-connection-three'), validAuth($connection->id(), 'presence-test-channel', $data = json_encode(['user_id' => 3, 'user_info' => ['name' => 'Jess']])), $data);

$response = await($this->signedRequest('channels/presence-test-channel/users'));

expect($response->getHeader('Content-Length'))->toBe(['38']);
});

it('gathers the user data', function () {
$this->usingRedis();

Expand Down Expand Up @@ -92,3 +105,18 @@
expect($response->getStatusCode())->toBe(200);
expect($response->getBody()->getContents())->toBe('{"users":[{"id":2},{"id":3}]}');
});

it('can send the content-length header when gathering results', function () {
$this->usingRedis();

$channel = app(ChannelManager::class)
->for(app()->make(ApplicationProvider::class)->findByKey('reverb-key'))
->findOrCreate('presence-test-channel');
$channel->subscribe($connection = new FakeConnection('test-connection-one'), validAuth($connection->id(), 'presence-test-channel', $data = json_encode(['user_id' => 1, 'user_info' => ['name' => 'Taylor']])), $data);
$channel->subscribe($connection = new FakeConnection('test-connection-two'), validAuth($connection->id(), 'presence-test-channel', $data = json_encode(['user_id' => 2, 'user_info' => ['name' => 'Joe']])), $data);
$channel->subscribe($connection = new FakeConnection('test-connection-three'), validAuth($connection->id(), 'presence-test-channel', $data = json_encode(['user_id' => 3, 'user_info' => ['name' => 'Jess']])), $data);

$response = await($this->signedRequest('channels/presence-test-channel/users'));

expect($response->getHeader('Content-Length'))->toBe(['38']);
});
20 changes: 20 additions & 0 deletions tests/Feature/Protocols/Pusher/Reverb/ChannelsControllerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,15 @@
expect($response->getBody()->getContents())->toBe('{"channels":{"test-channel-two":{}}}');
});

it('can send the content-length header', function () {
subscribe('test-channel-one');
subscribe('presence-test-channel-two');

$response = await($this->signedRequest('channels?info=user_count'));

expect($response->getHeader('Content-Length'))->toBe(['81']);
});

it('can gather all channel information', function () {
$this->usingRedis();

Expand Down Expand Up @@ -102,3 +111,14 @@
expect($response->getStatusCode())->toBe(200);
expect($response->getBody()->getContents())->toBe('{"channels":{"test-channel-two":{}}}');
});

it('can send the content-length header when gathering results', function () {
$this->usingRedis();

subscribe('test-channel-one');
subscribe('presence-test-channel-two');

$response = await($this->signedRequest('channels?info=user_count'));

expect($response->getHeader('Content-Length'))->toBe(['81']);
});
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,15 @@
expect($response->getBody()->getContents())->toBe('{"connections":1}');
});

it('can send the content-length header', function () {
subscribe('test-channel-one');
subscribe('presence-test-channel-two');

$response = await($this->signedRequest('connections'));

expect($response->getHeader('Content-Length'))->toBe(['17']);
});

it('can gather a connection count', function () {
$this->usingRedis();

Expand All @@ -51,3 +60,14 @@
expect($response->getStatusCode())->toBe(200);
expect($response->getBody()->getContents())->toBe('{"connections":1}');
});

it('can send the content-length header when gathering results', function () {
$this->usingRedis();

subscribe('test-channel-one');
subscribe('presence-test-channel-two');

$response = await($this->signedRequest('connections'));

expect($response->getHeader('Content-Length'))->toBe(['17']);
});
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,18 @@
expect($response->getBody()->getContents())->toBe('{"batch":[{"user_count":1},{}]}');
});

it('can send the content-length header', function () {
$response = await($this->signedPostRequest('batch_events', ['batch' => [
[
'name' => 'NewEvent',
'channel' => 'test-channel',
'data' => json_encode(['some' => 'data']),
],
]]));

expect($response->getHeader('Content-Length'))->toBe(['12']);
});

it('can receive an event batch trigger with multiple events and gather info for each', function () {
$this->usingRedis();

Expand Down Expand Up @@ -145,3 +157,17 @@

expect($response->getStatusCode())->toBe(500);
})->throws(ResponseException::class, exceptionCode: 500);

it('can send the content-length header when gathering results', function () {
$this->usingRedis();

$response = await($this->signedPostRequest('batch_events', ['batch' => [
[
'name' => 'NewEvent',
'channel' => 'test-channel',
'data' => json_encode(['some' => 'data']),
],
]]));

expect($response->getHeader('Content-Length'))->toBe(['12']);
});
10 changes: 10 additions & 0 deletions tests/Feature/Protocols/Pusher/Reverb/EventsControllerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -197,3 +197,13 @@
it('fails when app cannot be found', function () {
await($this->signedPostRequest('events', appId: 'invalid-app-id'));
})->throws(ResponseException::class, exceptionCode: 404);

it('can send the content-length header', function () {
$response = await($this->signedPostRequest('events', [
'name' => 'NewEvent',
'channel' => 'test-channel',
'data' => json_encode(['some' => 'data']),
]));

expect($response->getHeader('Content-Length'))->toBe(['2']);
});
Loading