Skip to content

Conversation

win0err
Copy link

@win0err win0err commented May 10, 2018

It's a minor clarification of input parameters in JWT::decode method.
Second parameter (key) accepts arrays, strings and resources, but in PHPDoc resource type is not listed.

Let's take a look at an example:

<?php

$pubkey =<<<PUBKEY
-----BEGIN PUBLIC KEY-----
[...]
-----END PUBLIC KEY-----
PUBKEY;

$publicKey = openssl_pkey_get_public(pubkey); // $publicKey is a resource
$decodedToken = JWT::decode($token, $publicKey, ['RS256']);

If I run static analysis with PHPStan, I will get the following error:

Parameter #2 $key of static method Firebase\JWT\JWT::decode() expects array|string, resource given.

Fix Parameter #2 $key of static method Firebase\JWT\JWT::decode() expects array|string, resource given.
@googlebot
Copy link

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

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

1 similar comment
@googlebot
Copy link

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

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

@win0err
Copy link
Author

win0err commented May 10, 2018

I signed it!

@googlebot
Copy link

CLAs look good, thanks!

@esbobkov
Copy link

Anybody, merge please

@win0err
Copy link
Author

win0err commented Feb 14, 2020

I've updated PR and resolved conflicts. @bshaffer, please, take a look

@bshaffer bshaffer merged commit ccc74fb into firebase:master Feb 19, 2020
@bshaffer
Copy link
Collaborator

This is great, thanks! And we got it merged in just under 2 years!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants