Skip to content

Commit

Permalink
Added JSON error check while decoding socketId as per review comment
Browse files Browse the repository at this point in the history
  • Loading branch information
sacOO7 committed Sep 24, 2024
1 parent 43a6e89 commit baf6820
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 5 deletions.
14 changes: 10 additions & 4 deletions src/Utils.php
Original file line number Diff line number Diff line change
Expand Up @@ -79,14 +79,20 @@ public static function base64url_decode($data) {
."'connectionKey' and 'clientId' as keys. 'clientId' is null if connection is not identified";

/**
* @return object
* @throws AblyException
*/
public static function decodeSocketId($socketId) {
public static function decodeSocketId($socketId): ?object
{
$socketIdObject = null;
if ($socketId) {
$socketIdObject = json_decode(self::base64url_decode($socketId));
if (!$socketIdObject) {
throw new AblyException("SocketId decoding failed, ".self::SOCKET_ID_ERROR);
$socketIdJsonString = self::base64url_decode($socketId);
if (!$socketIdJsonString) {
throw new AblyException("Base64 decoding failed, ".self::SOCKET_ID_ERROR);
}
$socketIdObject = json_decode($socketIdJsonString);
if (json_last_error() !== JSON_ERROR_NONE) {
throw new AblyException("JSON decoding failed: " . json_last_error_msg() . ", " . self::SOCKET_ID_ERROR);
}
if (!isset($socketIdObject->connectionKey)) {
throw new AblyException("ConnectionKey is not set, ".self::SOCKET_ID_ERROR);
Expand Down
2 changes: 1 addition & 1 deletion tests/UtilsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ public function testDecodeSocketId() {
public function testExceptionOnDecodingInvalidSocketId()
{
self::expectException(AblyException::class);
self::expectExceptionMessage("SocketId decoding failed, ".Utils::SOCKET_ID_ERROR);
self::expectExceptionMessage("Base64 decoding failed, ".Utils::SOCKET_ID_ERROR);
Utils::decodeSocketId("invalid_socket_id");
}

Expand Down

0 comments on commit baf6820

Please sign in to comment.