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

[BUG]: Unable to use JWT Parser when getJsonRawBody is called #16373

Closed
dz3n opened this issue Jul 8, 2023 · 2 comments · Fixed by #16380
Closed

[BUG]: Unable to use JWT Parser when getJsonRawBody is called #16373

dz3n opened this issue Jul 8, 2023 · 2 comments · Fixed by #16380
Labels
5.0 The issues we want to solve in the 5.0 release bug A bug report status: medium Medium

Comments

@dz3n
Copy link

dz3n commented Jul 8, 2023

Describe the bug
If getJsonRawBody is called, JWT parser throws an error instead of the parsed JWT

To Reproduce

use Phalcon\Encryption\Security\JWT\Signer\Hmac;
use Phalcon\Encryption\Security\JWT\Token\Parser;
...

$request = Di::getDefault()->get('request');

/**
 * If this line is removed the code works fine
 */
$payload = $request->getJsonRawBody(true);


$jwt = 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJ0ZXN0IjoiMSIsImlhdCI6MTY4ODgzNDY2MywiZXhwIjoxNjg4ODM4MjYzfQ.i0Cwm4uHqAwyZ_gWZpDA3sUdha56HxngkvFE2Mz5tsM';

$parser = new Parser();
$signer = new Hmac();

$tokenObject = $parser->parse($jwt);
$data = $tokenObject->getClaims()->getPayload();

var_dump($data);

If $payload = $request->getJsonRawBody(true); exists I get json_decode error: Syntax error instead of the parsed JWT

Details

  • Phalcon version: 5.0.4
  • PHP Version: PHP 8.1.7
  • Operating System:
  • Installation type: Ubuntu + Docker
  • Server: Nginx
@dz3n dz3n added bug A bug report status: unverified Unverified labels Jul 8, 2023
@sinbadxiii
Copy link
Contributor

sinbadxiii commented Jul 14, 2023

The problem really exists
There is no json_decode error checking in the Request

    public function getJsonRawBody(bool associative = false) -> <\stdClass> | array | bool
    {
        var rawBody;

        let rawBody = this->getRawBody();

        if typeof rawBody != "string" {
            return false;
        }

        return json_decode(rawBody, associative);
    }

We can do validation and then an error will be shown for empty request and all projects will fail :)

    public function getJsonRawBody(bool associative = false) -> <\stdClass> | array | bool
    {
        var rawBody;

        let rawBody = this->getRawBody();

        if typeof rawBody != "string" {
            return false;
        }

        var decoded;

        let decoded = json_decode(rawBody, associative);

        if unlikely JSON_ERROR_NONE !== json_last_error() {
            throw new \InvalidArgumentException(
                "json_decode error: " . json_last_error_msg()
            );
        }

        return decoded;
    }

or this problem can be solved if we just will return stdClass if rawBody == ""

    public function getJsonRawBody(bool associative = false) -> <\stdClass> | array | bool
    {
        var rawBody;

        let rawBody = this->getRawBody();

        if typeof rawBody != "string" {
            return false;
        }

        if rawBody == ""
        {
            return new \stdClass();  //or let rawBody == "{}"; as variant to work associative array
        }

        return json_decode(rawBody, associative);
    }

Guys what do you think? what's better?

@sinbadxiii sinbadxiii mentioned this issue Jul 25, 2023
5 tasks
@niden niden added status: medium Medium 5.0 The issues we want to solve in the 5.0 release and removed status: unverified Unverified labels Jul 25, 2023
sinbadxiii added a commit to sinbadxiii/cphalcon that referenced this issue Jul 26, 2023
sinbadxiii added a commit to sinbadxiii/cphalcon that referenced this issue Jul 26, 2023
@niden niden linked a pull request Jul 26, 2023 that will close this issue
5 tasks
@niden
Copy link
Sponsor Member

niden commented Jul 26, 2023

Resolved in #16380

@niden niden closed this as completed Jul 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
5.0 The issues we want to solve in the 5.0 release bug A bug report status: medium Medium
Projects
Status: Released
Development

Successfully merging a pull request may close this issue.

3 participants