Shopify/shopify-api-php

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

sarim opened this issue · 5 comments

sarim commented

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.

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.

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.