Completed
Push — master ( ad7c33...f0cfea )
by Steven
03:47
created

ProviderRedirectTrait::getRedirectLimit()   A

Complexity

Conditions 1
Paths 1

Size

Total Lines 4
Code Lines 2

Duplication

Lines 0
Ratio 0 %

Code Coverage

Tests 2
CRAP Score 1

Importance

Changes 1
Bugs 0 Features 0
Metric Value
dl 0
loc 4
c 1
b 0
f 0
ccs 2
cts 2
cp 1
rs 10
cc 1
eloc 2
nc 1
nop 0
crap 1
1
<?php
2
3
namespace Stevenmaguire\OAuth2\Client\Tool;
4
5
use GuzzleHttp\Exception\BadResponseException;
6
use GuzzleHttp\Psr7\Uri;
7
use InvalidArgumentException;
8
use Psr\Http\Message\RequestInterface;
9
use Psr\Http\Message\ResponseInterface;
10
11
trait ProviderRedirectTrait
12
{
13
    /**
14
     * Maximum number of times to follow provider initiated redirects
15
     *
16
     * @var integer
17
     */
18
    protected $redirectLimit = 2;
19
20
    /**
21
     * Retrieves current redirect limit.
22
     *
23
     * @return integer
24
     */
25 4
    public function getRedirectLimit()
26
    {
27 4
        return $this->redirectLimit;
28
    }
29
30
    /**
31
     * Sends a request instance and returns a response instance.
32
     *
33
     * @param  RequestInterface $request
34
     * @return ResponseInterface
35
     */
36 8
    protected function sendRequest(RequestInterface $request)
37
    {
38 8
        $attempts = 0;
39
40
        $requestOptions = [
41
            'allow_redirects' => false
42 8
        ];
43
44 8
        $isRedirect = function (ResponseInterface $response) {
45 6
            $statusCode = $response->getStatusCode();
46
47 6
            return $statusCode > 300 && $statusCode < 400 && $response->hasHeader('Location');
48 8
        };
49
50
        try {
51 8
            while ($attempts < $this->redirectLimit) {
52 8
                $attempts++;
53 8
                $response = $this->getHttpClient()->send($request, $requestOptions);
0 ignored issues
show
Bug introduced by
It seems like getHttpClient() must be provided by classes using this trait. How about adding it as abstract method to this trait?

This check looks for methods that are used by a trait but not required by it.

To illustrate, let’s look at the following code example

trait Idable {
    public function equalIds(Idable $other) {
        return $this->getId() === $other->getId();
    }
}

The trait Idable provides a method equalsId that in turn relies on the method getId(). If this method does not exist on a class mixing in this trait, the method will fail.

Adding the getId() as an abstract method to the trait will make sure it is available.

Loading history...
54 6
                $statusCode = $response->getStatusCode();
0 ignored issues
show
Unused Code introduced by
$statusCode is not used, you could remove the assignment.

This check looks for variable assignements that are either overwritten by other assignments or where the variable is not used subsequently.

$myVar = 'Value';
$higher = false;

if (rand(1, 6) > 3) {
    $higher = true;
} else {
    $higher = false;
}

Both the $myVar assignment in line 1 and the $higher assignment in line 2 are dead. The first because $myVar is never used and the second because $higher is always overwritten for every possible time line.

Loading history...
55
56 6
                if ($isRedirect($response)) {
57 2
                    $redirectUrl = new Uri($response->getHeader('Location')[0]);
58 2
                    $request = $request->withUri($redirectUrl);
59 2
                } else {
60 4
                    break;
61
                }
62 2
            }
63 8
        } catch (BadResponseException $e) {
64 2
            $response = $e->getResponse();
65
        }
66
67 8
        return $response;
0 ignored issues
show
Bug introduced by
The variable $response does not seem to be defined for all execution paths leading up to this point.

If you define a variable conditionally, it can happen that it is not defined for all execution paths.

Let’s take a look at an example:

function myFunction($a) {
    switch ($a) {
        case 'foo':
            $x = 1;
            break;

        case 'bar':
            $x = 2;
            break;
    }

    // $x is potentially undefined here.
    echo $x;
}

In the above example, the variable $x is defined if you pass “foo” or “bar” as argument for $a. However, since the switch statement has no default case statement, if you pass any other value, the variable $x would be undefined.

Available Fixes

  1. Check for existence of the variable explicitly:

    function myFunction($a) {
        switch ($a) {
            case 'foo':
                $x = 1;
                break;
    
            case 'bar':
                $x = 2;
                break;
        }
    
        if (isset($x)) { // Make sure it's always set.
            echo $x;
        }
    }
    
  2. Define a default value for the variable:

    function myFunction($a) {
        $x = ''; // Set a default which gets overridden for certain paths.
        switch ($a) {
            case 'foo':
                $x = 1;
                break;
    
            case 'bar':
                $x = 2;
                break;
        }
    
        echo $x;
    }
    
  3. Add a value for the missing path:

    function myFunction($a) {
        switch ($a) {
            case 'foo':
                $x = 1;
                break;
    
            case 'bar':
                $x = 2;
                break;
    
            // We add support for the missing case.
            default:
                $x = '';
                break;
        }
    
        echo $x;
    }
    
Loading history...
68
    }
69
70
    /**
71
     * Updates the redirect limit.
72
     *
73
     * @param integer $limit
74
     * @return League\OAuth2\Client\Provider\AbstractProvider
75
     * @throws InvalidArgumentException
76
     */
77 10
    public function setRedirectLimit($limit)
78
    {
79 10
        if (!is_numeric($limit)) {
80 4
            throw new InvalidArgumentException('setRedirectLimit function only accepts numeric values.');
81
        }
82
83 6
        $this->redirectLimit = $limit;
0 ignored issues
show
Documentation Bug introduced by
It seems like $limit can also be of type double or string. However, the property $redirectLimit is declared as type integer. Maybe add an additional type check?

Our type inference engine has found a suspicous assignment of a value to a property. This check raises an issue when a value that can be of a mixed type is assigned to a property that is type hinted more strictly.

For example, imagine you have a variable $accountId that can either hold an Id object or false (if there is no account id yet). Your code now assigns that value to the id property of an instance of the Account class. This class holds a proper account, so the id value must no longer be false.

Either this assignment is in error or a type check should be added for that assignment.

class Id
{
    public $id;

    public function __construct($id)
    {
        $this->id = $id;
    }

}

class Account
{
    /** @var  Id $id */
    public $id;
}

$account_id = false;

if (starsAreRight()) {
    $account_id = new Id(42);
}

$account = new Account();
if ($account instanceof Id)
{
    $account->id = $account_id;
}
Loading history...
84
85 6
        return $this;
86
    }
87
}
88