Skip to content

Ensure numeric type of iat and nbf parameters#453

Closed
joshuaruesweg wants to merge 4 commits into
googleapis:mainfrom
joshuaruesweg:patch-2
Closed

Ensure numeric type of iat and nbf parameters#453
joshuaruesweg wants to merge 4 commits into
googleapis:mainfrom
joshuaruesweg:patch-2

Conversation

@joshuaruesweg
Copy link
Copy Markdown

The two parameters must be numeric according to RFC. With newer PHP versions, if a token has expired, the exception will additionally throw an own exception if the parameter is not numeric (and the date function expects zero or an integer as parameter).

@google-cla
Copy link
Copy Markdown

google-cla Bot commented Aug 24, 2022

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@joshuaruesweg joshuaruesweg marked this pull request as draft August 24, 2022 19:33
@joshuaruesweg joshuaruesweg force-pushed the patch-2 branch 3 times, most recently from ac79986 to 71b2329 Compare September 8, 2022 18:29
@joshuaruesweg joshuaruesweg marked this pull request as ready for review September 8, 2022 18:30
Comment thread src/JWT.php Outdated
Copy link
Copy Markdown
Collaborator

@bshaffer bshaffer left a comment

Choose a reason for hiding this comment

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

Since floor can cast from a string to an int, but is_int is strict, this would be a breaking change. Before this change, the library silently accepts numeric strings, but after this change it would throw exceptions:

$payload = [
    'message' => 'abc',
    'iat' => (string) time(),
    'exp' => (string) (time() + 20), // time in the future
    'nbf' => (string) (time() - 20)
];

// these work without throwing exceptions
$encoded = JWT::encode($payload, $this->hmacKey->getKeyMaterial(), 'HS256');
$decoded = JWT::decode($encoded, $this->hmacKey);

Therefore, we cannot merge this without breaking changes, and so we'd need to create a new major version before doing so

@bshaffer bshaffer added next major version do not merge Indicates a pull request not ready for merge, due to either quality or timing. labels May 11, 2026
@bshaffer
Copy link
Copy Markdown
Collaborator

Closing as this is handled correctly in #634

The floor function throws an error already, so there's no need to add it here... although if you'd rather have an exception thrown instead of a PHP error, you can resubmit this PR using is_numeric instead of is_int.

@bshaffer bshaffer closed this May 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do not merge Indicates a pull request not ready for merge, due to either quality or timing. next major version

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants