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

Constructor of Shopify\Auth\Scopes can't handle null #290

Closed
1 task done
sarim opened this issue Jul 25, 2023 · 3 comments · May be fixed by #299
Closed
1 task done

Constructor of Shopify\Auth\Scopes can't handle null #290

sarim opened this issue Jul 25, 2023 · 3 comments · May be fixed by #299
Labels

Comments

@sarim
Copy link

sarim commented Jul 25, 2023

Issue summary

During Auth Begin, a session is created without any scope / accessToken etc.. set. So these fields are null. In this state if $session->isValid() is called, it results in a TypeError.

Because isValid calls Context::$SCOPES->equals($this->scope).
Here $this->scope is null.
SCOPES->equals calls $scopes = new self($scopes);.

And in the constructor of Scopes

public function __construct($scopes)
{
if (is_string($scopes)) {
$scopesArray = explode(self::SCOPE_DELIMITER, $scopes);
} else {
$scopesArray = $scopes;
}
$scopesArray = array_unique(array_filter(array_map('trim', $scopesArray)));

is_string returns false for null type, but this code assumes that is not string, then must be array and continues. Resulting in array_map call throwing a TypeError because it expects array, but null given.

Expected behavior

What do you think should happen?

No PHP TypeError should happen.

Actual behavior

What actually happens?

TypeError

php > require_once 'vendor/autoload_runtime.php';
php > $s = new \Shopify\Auth\Scopes(null);
PHP Warning:  Uncaught TypeError: array_map(): Argument #2 ($array) must be of type array, null given in /home/******************************/vendor/shopify/shopify-api/src/Auth/Scopes.php:27
Stack trace:
#0 /home/******************************/vendor/shopify/shopify-api/src/Auth/Scopes.php(27): array_map()
#1 php shell code(1): Shopify\Auth\Scopes->__construct()
#2 {main}
  thrown in /home/******************************/vendor/shopify/shopify-api/src/Auth/Scopes.php on line 27

Steps to reproduce the problem

  1. php -a
require_once 'vendor/autoload_runtime.php';
$s = new \Shopify\Auth\Scopes(null);

Reduced test case

The best way to get your bug fixed is to provide a reduced test case.


Checklist

  • I have described this issue in a way that is actionable (if possible)

Possible Solution

    public function __construct($scopes)
    {
        if (is_string($scopes)) {
            $scopesArray = explode(self::SCOPE_DELIMITER, $scopes);
        } else {
            $scopesArray = $scopes ?? [];
        }

Adding ?? [] should fix it.

Or Session->isValid shouldn't call Context::$SCOPES->equals if $this->scope is null.

@github-actions
Copy link

This issue is stale because it has been open for 60 days with no activity. It will be closed if no further action occurs in 14 days.

Copy link

This issue is stale because it has been open for 60 days with no activity. It will be closed if no further action occurs in 14 days.

@github-actions github-actions bot added the Stale label Nov 26, 2023
Copy link

We are closing this issue because it has been inactive for a few months.
This probably means that it is not reproducible or it has been fixed in a newer version.
If it’s an enhancement and hasn’t been taken on since it was submitted, then it seems other issues have taken priority.

If you still encounter this issue with the latest stable version, please reopen using the issue template. You can also contribute directly by submitting a pull request– see the CONTRIBUTING.md file for guidelines

Thank you!

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Dec 10, 2023
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 a pull request may close this issue.

1 participant