Issues in CookieJar.php - All Issues - Inspection of "Merge pull request #1642 from gmponos/issue_1631" - guzzle/guzzle - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — master ( 8bc9d2...ec70be )

by Márk

created 2017-01-05 06:32 UTC

src/Cookie/CookieJar.php (8 issues)

Labels

Severity

Upgrade to new PHP Analysis Engine

These results are based on our legacy PHP analysis, consider migrating to our new PHP analysis engine instead. Learn more

<?php
namespace GuzzleHttp\Cookie;

use Psr\Http\Message\RequestInterface;
use Psr\Http\Message\ResponseInterface;

/**
 * Cookie jar that stores cookies as an array
 */
class CookieJar implements CookieJarInterface
{
    /** @var SetCookie[] Loaded cookie data */
    private $cookies = [];

    /** @var bool */
    private $strictMode;

    /**
     * @param bool $strictMode   Set to true to throw exceptions when invalid
     *                           cookies are added to the cookie jar.
     * @param array $cookieArray Array of SetCookie objects or a hash of
     *                           arrays that can be used with the SetCookie
     *                           constructor
     */
    public function __construct($strictMode = false, $cookieArray = [])
    {
        $this->strictMode = $strictMode;

        foreach ($cookieArray as $cookie) {
            if (!($cookie instanceof SetCookie)) {
                $cookie = new SetCookie($cookie);
            }
            $this->setCookie($cookie);
        }
    }

    /**
     * Create a new Cookie jar from an associative array and domain.
     *
     * @param array  $cookies Cookies to create the jar from
     * @param string $domain  Domain to set the cookies to
     *
     * @return self
     */
    public static function fromArray(array $cookies, $domain)
    {
        $cookieJar = new self();
        foreach ($cookies as $name => $value) {
            $cookieJar->setCookie(new SetCookie([
                'Domain'  => $domain,
                'Name'    => $name,
                'Value'   => $value,
                'Discard' => true
            ]));
        }

        return $cookieJar;
    }

    /**
     * @deprecated
     */
    public static function getCookieValue($value)
    {
        return $value;
    }

    /**
     * Evaluate if this cookie should be persisted to storage
     * that survives between requests.
     *
     * @param SetCookie $cookie Being evaluated.
     * @param bool $allowSessionCookies If we should persist session cookies
     * @return bool
     */
    public static function shouldPersist(
        SetCookie $cookie,
        $allowSessionCookies = false
    ) {
        if ($cookie->getExpires() || $allowSessionCookies) {
            if (!$cookie->getDiscard()) {
$a = canBeFalseAndNull();

// Instead of
if ( ! $a) { }

// Better use one of the explicit versions:
if ($a !== null) { }
if ($a !== false) { }
if ($a !== null && $a !== false) { }
                return true;
            }
        }

        return false;
    }

    public function toArray()
    {
        return array_map(function (SetCookie $cookie) {
            return $cookie->toArray();
        }, $this->getIterator()->getArrayCopy());
interface User
{
    /** @return string */
    public function getPassword();
}

class MyUser implements User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}
    }

    public function clear($domain = null, $path = null, $name = null)
    {
        if (!$domain) {
''   == false // true
''   == null  // true
'ab' == false // false
'ab' == null  // false

// It is often better to use strict comparison
'' === false // false
'' === null  // false
            $this->cookies = [];
            return;
        } elseif (!$path) {
''   == false // true
''   == null  // true
'ab' == false // false
'ab' == null  // false

// It is often better to use strict comparison
'' === false // false
'' === null  // false
            $this->cookies = array_filter(
                $this->cookies,
                function (SetCookie $cookie) use ($path, $domain) {
                    return !$cookie->matchesDomain($domain);
                }
            );
        } elseif (!$name) {
''   == false // true
''   == null  // true
'ab' == false // false
'ab' == null  // false

// It is often better to use strict comparison
'' === false // false
'' === null  // false
            $this->cookies = array_filter(
                $this->cookies,
                function (SetCookie $cookie) use ($path, $domain) {
                    return !($cookie->matchesPath($path) &&
                        $cookie->matchesDomain($domain));
                }
            );
        } else {
            $this->cookies = array_filter(
                $this->cookies,
                function (SetCookie $cookie) use ($path, $domain, $name) {
                    return !($cookie->getName() == $name &&
                        $cookie->matchesPath($path) &&
                        $cookie->matchesDomain($domain));
                }
            );
        }
    }

    public function clearSessionCookies()
    {
        $this->cookies = array_filter(
            $this->cookies,
            function (SetCookie $cookie) {
                return !$cookie->getDiscard() && $cookie->getExpires();
$a = canBeFalseAndNull();

// Instead of
if ( ! $a) { }

// Better use one of the explicit versions:
if ($a !== null) { }
if ($a !== false) { }
if ($a !== null && $a !== false) { }
            }
        );
    }

    public function setCookie(SetCookie $cookie)
    {
        // If the name string is empty (but not 0), ignore the set-cookie
        // string entirely.
        $name = $cookie->getName();
        if (!$name && $name !== '0') {
            return false;
        }

        // Only allow cookies with set and valid domain, name, value
        $result = $cookie->validate();
        if ($result !== true) {
            if ($this->strictMode) {
                throw new \RuntimeException('Invalid cookie: ' . $result);
            } else {
                $this->removeCookieIfEmpty($cookie);
                return false;
            }
        }

        // Resolve conflicts with previously set cookies
        foreach ($this->cookies as $i => $c) {

            // Two cookies are identical, when their path, and domain are
            // identical.
            if ($c->getPath() != $cookie->getPath() ||
                $c->getDomain() != $cookie->getDomain() ||
                $c->getName() != $cookie->getName()
            ) {
                continue;
            }

            // The previously set cookie is a discard cookie and this one is
            // not so allow the new cookie to be set
            if (!$cookie->getDiscard() && $c->getDiscard()) {
$a = canBeFalseAndNull();

// Instead of
if ( ! $a) { }

// Better use one of the explicit versions:
if ($a !== null) { }
if ($a !== false) { }
if ($a !== null && $a !== false) { }
                unset($this->cookies[$i]);
                continue;
            }

            // If the new cookie's expiration is further into the future, then
            // replace the old cookie
            if ($cookie->getExpires() > $c->getExpires()) {
                unset($this->cookies[$i]);
                continue;
            }

            // If the value has changed, we better change it
            if ($cookie->getValue() !== $c->getValue()) {
                unset($this->cookies[$i]);
                continue;
            }

            // The cookie exists, so no need to continue
            return false;
        }

        $this->cookies[] = $cookie;

        return true;
    }

    public function count()
    {
        return count($this->cookies);
    }

    public function getIterator()
    {
        return new \ArrayIterator(array_values($this->cookies));
    }

    public function extractCookies(
        RequestInterface $request,
        ResponseInterface $response
    ) {
        if ($cookieHeader = $response->getHeader('Set-Cookie')) {
            foreach ($cookieHeader as $cookie) {
                $sc = SetCookie::fromString($cookie);
                if (!$sc->getDomain()) {
''   == false // true
''   == null  // true
'ab' == false // false
'ab' == null  // false

// It is often better to use strict comparison
'' === false // false
'' === null  // false
                    $sc->setDomain($request->getUri()->getHost());
                }
                $this->setCookie($sc);
            }
        }
    }

    public function withCookieHeader(RequestInterface $request)
    {
        $values = [];
        $uri = $request->getUri();
        $scheme = $uri->getScheme();
        $host = $uri->getHost();
        $path = $uri->getPath() ?: '/';

        foreach ($this->cookies as $cookie) {
            if ($cookie->matchesPath($path) &&
                $cookie->matchesDomain($host) &&
                !$cookie->isExpired() &&
                (!$cookie->getSecure() || $scheme === 'https')
            ) {
                $values[] = $cookie->getName() . '='
                    . $cookie->getValue();
            }
        }

        return $values
            ? $request->withHeader('Cookie', implode('; ', $values))
            : $request;
    }

    /**
     * If a cookie already exists and the server asks to set it again with a
     * null value, the cookie must be deleted.
     *
     * @param SetCookie $cookie
     */
    private function removeCookieIfEmpty(SetCookie $cookie)
    {
        $cookieValue = $cookie->getValue();
        if ($cookieValue === null || $cookieValue === '') {
            $this->clear(
                $cookie->getDomain(),
                $cookie->getPath(),
                $cookie->getName()
            );
        }
    }
}


Push — master ( 8bc9d2...ec70be )

src/Cookie/CookieJar.php (8 issues)

Labels

Severity

Introduced By

Upgrade to new PHP Analysis Engine

Available Fixes

1			<?php
2			namespace GuzzleHttp\Cookie;
3
4			use Psr\Http\Message\RequestInterface;
5			use Psr\Http\Message\ResponseInterface;
6
7			/**
8			* Cookie jar that stores cookies as an array
9			*/
10			class CookieJar implements CookieJarInterface
11			{
12			/** @var SetCookie[] Loaded cookie data */
13			private $cookies = [];
14
15			/** @var bool */
16			private $strictMode;
17
18			/**
19			* @param bool $strictMode Set to true to throw exceptions when invalid
20			* cookies are added to the cookie jar.
21			* @param array $cookieArray Array of SetCookie objects or a hash of
22			* arrays that can be used with the SetCookie
23			* constructor
24			*/
25			public function __construct($strictMode = false, $cookieArray = [])
26			{
27			$this->strictMode = $strictMode;
28
29			foreach ($cookieArray as $cookie) {
30			if (!($cookie instanceof SetCookie)) {
31			$cookie = new SetCookie($cookie);
32			}
33			$this->setCookie($cookie);
34			}
35			}
36
37			/**
38			* Create a new Cookie jar from an associative array and domain.
39			*
40			* @param array $cookies Cookies to create the jar from
41			* @param string $domain Domain to set the cookies to
42			*
43			* @return self
44			*/
45			public static function fromArray(array $cookies, $domain)
46			{
47			$cookieJar = new self();
48			foreach ($cookies as $name => $value) {
49			$cookieJar->setCookie(new SetCookie([
50			'Domain' => $domain,
51			'Name' => $name,
52			'Value' => $value,
53			'Discard' => true
54			]));
55			}
56
57			return $cookieJar;
58			}
59
60			/**
61			* @deprecated
62			*/
63			public static function getCookieValue($value)
64			{
65			return $value;
66			}
67
68			/**
69			* Evaluate if this cookie should be persisted to storage
70			* that survives between requests.
71			*
72			* @param SetCookie $cookie Being evaluated.
73			* @param bool $allowSessionCookies If we should persist session cookies
74			* @return bool
75			*/
76			public static function shouldPersist(
77			SetCookie $cookie,
78			$allowSessionCookies = false
79			) {
80			if ($cookie->getExpires() \|\| $allowSessionCookies) {
81			if (!$cookie->getDiscard()) {
			0 ignored issues – show Bug Best Practice introduced 2015-11-23 00:43 UTC by Report Bug Copy Issue Report Show Similar Issues like this The expression `$cookie->getDiscard()` of type `null\|boolean` is loosely compared to `false`; this is ambiguous if the boolean can be false. You might want to explicitly use `!== null` instead. If an expression can have both `false`, and `null` as possible values. It is generally a good practice to always use strict comparison to clearly distinguish between those two values. $a = canBeFalseAndNull(); // Instead of if ( ! $a) { } // Better use one of the explicit versions: if ($a !== null) { } if ($a !== false) { } if ($a !== null && $a !== false) { } Loading history...
82			return true;
83			}
84			}
85
86			return false;
87			}
88
89			public function toArray()
90			{
91			return array_map(function (SetCookie $cookie) {
92			return $cookie->toArray();
93			}, $this->getIterator()->getArrayCopy());
			0 ignored issues – show Bug introduced 2016-07-23 18:04 UTC by Report Bug Copy Issue Report Show Similar Issues like this It seems like you code against a concrete implementation and not the interface `Traversable` as the method `getArrayCopy()` does only exist in the following implementations of said interface: `ArrayIterator`, `ArrayObject`, `DoctrineTest\Instantiato...tAsset\ArrayObjectAsset`, `DoctrineTest\Instantiato...lizableArrayObjectAsset`, `DoctrineTest\Instantiato...ceptionArrayObjectAsset`, `DoctrineTest\Instantiato...sset\WakeUpNoticesAsset`, `Issue523`, `RecursiveArrayIterator`. Let’s take a look at an example: interface User { /** @return string / public function getPassword(); } class MyUser implements User { public function getPassword() { // return something } public function getDisplayName() { // return some name. } } class AuthSystem { public function authenticate(User $user) { $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName())); // do something. } } In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different implementation of User which does not have a getDisplayName() method, the code will break. Available Fixes Change the type-hint for the parameter: class AuthSystem { public function authenticate(MyUser $user) { / ... / } } Add an additional type-check: class AuthSystem { public function authenticate(User $user) { if ($user instanceof MyUser) { $this->logger->info(/* ... /); } // or alternatively if ( ! $user instanceof MyUser) { throw new \LogicException( '$user must be an instance of MyUser, ' .'other instances are not supported.' ); } } } Note:* PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case. Add the method to the interface: interface User { /** @return string / public function getPassword(); /* @return string */ public function getDisplayName(); } Loading history...
94			}
95
96			public function clear($domain = null, $path = null, $name = null)
97			{
98			if (!$domain) {
			0 ignored issues – show Bug Best Practice introduced 2015-11-23 00:40 UTC by Report Bug Copy Issue Report Show Similar Issues like this The expression `$domain` of type `string\|null` is loosely compared to `false`; this is ambiguous if the string can be empty. You might want to explicitly use `=== null` instead. In PHP, under loose comparison (like `==`, or `!=`, or `switch` conditions), values of different types might be equal. For `string` values, the empty string `''` is a special case, in particular the following results might be unexpected: '' == false // true '' == null // true 'ab' == false // false 'ab' == null // false // It is often better to use strict comparison '' === false // false '' === null // false Loading history...
99			$this->cookies = [];
100			return;
101			} elseif (!$path) {
			0 ignored issues – show Bug Best Practice introduced 2015-11-23 00:40 UTC by Report Bug Copy Issue Report Show Similar Issues like this The expression `$path` of type `string\|null` is loosely compared to `false`; this is ambiguous if the string can be empty. You might want to explicitly use `=== null` instead. In PHP, under loose comparison (like `==`, or `!=`, or `switch` conditions), values of different types might be equal. For `string` values, the empty string `''` is a special case, in particular the following results might be unexpected: '' == false // true '' == null // true 'ab' == false // false 'ab' == null // false // It is often better to use strict comparison '' === false // false '' === null // false Loading history...
102			$this->cookies = array_filter(
103			$this->cookies,
104			function (SetCookie $cookie) use ($path, $domain) {
105			return !$cookie->matchesDomain($domain);
106			}
107			);
108			} elseif (!$name) {
			0 ignored issues – show Bug Best Practice introduced 2015-11-23 00:40 UTC by Report Bug Copy Issue Report Show Similar Issues like this The expression `$name` of type `string\|null` is loosely compared to `false`; this is ambiguous if the string can be empty. You might want to explicitly use `=== null` instead. In PHP, under loose comparison (like `==`, or `!=`, or `switch` conditions), values of different types might be equal. For `string` values, the empty string `''` is a special case, in particular the following results might be unexpected: '' == false // true '' == null // true 'ab' == false // false 'ab' == null // false // It is often better to use strict comparison '' === false // false '' === null // false Loading history...
109			$this->cookies = array_filter(
110			$this->cookies,
111			function (SetCookie $cookie) use ($path, $domain) {
112			return !($cookie->matchesPath($path) &&
113			$cookie->matchesDomain($domain));
114			}
115			);
116			} else {
117			$this->cookies = array_filter(
118			$this->cookies,
119			function (SetCookie $cookie) use ($path, $domain, $name) {
120			return !($cookie->getName() == $name &&
121			$cookie->matchesPath($path) &&
122			$cookie->matchesDomain($domain));
123			}
124			);
125			}
126			}
127
128			public function clearSessionCookies()
129			{
130			$this->cookies = array_filter(
131			$this->cookies,
132			function (SetCookie $cookie) {
133			return !$cookie->getDiscard() && $cookie->getExpires();
			0 ignored issues – show Bug Best Practice introduced 2015-11-23 00:40 UTC by Report Bug Copy Issue Report Show Similar Issues like this The expression `$cookie->getDiscard()` of type `null\|boolean` is loosely compared to `false`; this is ambiguous if the boolean can be false. You might want to explicitly use `!== null` instead. If an expression can have both `false`, and `null` as possible values. It is generally a good practice to always use strict comparison to clearly distinguish between those two values. $a = canBeFalseAndNull(); // Instead of if ( ! $a) { } // Better use one of the explicit versions: if ($a !== null) { } if ($a !== false) { } if ($a !== null && $a !== false) { } Loading history...
134			}
135			);
136			}
137
138			public function setCookie(SetCookie $cookie)
139			{
140			// If the name string is empty (but not 0), ignore the set-cookie
141			// string entirely.
142			$name = $cookie->getName();
143			if (!$name && $name !== '0') {
144			return false;
145			}
146
147			// Only allow cookies with set and valid domain, name, value
148			$result = $cookie->validate();
149			if ($result !== true) {
150			if ($this->strictMode) {
151			throw new \RuntimeException('Invalid cookie: ' . $result);
152			} else {
153			$this->removeCookieIfEmpty($cookie);
154			return false;
155			}
156			}
157
158			// Resolve conflicts with previously set cookies
159			foreach ($this->cookies as $i => $c) {
160
161			// Two cookies are identical, when their path, and domain are
162			// identical.
163			if ($c->getPath() != $cookie->getPath() \|\|
164			$c->getDomain() != $cookie->getDomain() \|\|
165			$c->getName() != $cookie->getName()
166			) {
167			continue;
168			}
169
170			// The previously set cookie is a discard cookie and this one is
171			// not so allow the new cookie to be set
172			if (!$cookie->getDiscard() && $c->getDiscard()) {
			0 ignored issues – show Bug Best Practice introduced 2015-11-23 00:40 UTC by Report Bug Copy Issue Report Show Similar Issues like this The expression `$cookie->getDiscard()` of type `null\|boolean` is loosely compared to `false`; this is ambiguous if the boolean can be false. You might want to explicitly use `!== null` instead. If an expression can have both `false`, and `null` as possible values. It is generally a good practice to always use strict comparison to clearly distinguish between those two values. $a = canBeFalseAndNull(); // Instead of if ( ! $a) { } // Better use one of the explicit versions: if ($a !== null) { } if ($a !== false) { } if ($a !== null && $a !== false) { } Loading history...
173			unset($this->cookies[$i]);
174			continue;
175			}
176
177			// If the new cookie's expiration is further into the future, then
178			// replace the old cookie
179			if ($cookie->getExpires() > $c->getExpires()) {
180			unset($this->cookies[$i]);
181			continue;
182			}
183
184			// If the value has changed, we better change it
185			if ($cookie->getValue() !== $c->getValue()) {
186			unset($this->cookies[$i]);
187			continue;
188			}
189
190			// The cookie exists, so no need to continue
191			return false;
192			}
193
194			$this->cookies[] = $cookie;
195
196			return true;
197			}
198
199			public function count()
200			{
201			return count($this->cookies);
202			}
203
204			public function getIterator()
205			{
206			return new \ArrayIterator(array_values($this->cookies));
207			}
208
209			public function extractCookies(
210			RequestInterface $request,
211			ResponseInterface $response
212			) {
213			if ($cookieHeader = $response->getHeader('Set-Cookie')) {
214			foreach ($cookieHeader as $cookie) {
215			$sc = SetCookie::fromString($cookie);
216			if (!$sc->getDomain()) {
			0 ignored issues – show Bug Best Practice introduced 2015-11-23 00:40 UTC by Report Bug Copy Issue Report Show Similar Issues like this The expression `$sc->getDomain()` of type `string\|null` is loosely compared to `false`; this is ambiguous if the string can be empty. You might want to explicitly use `=== null` instead. In PHP, under loose comparison (like `==`, or `!=`, or `switch` conditions), values of different types might be equal. For `string` values, the empty string `''` is a special case, in particular the following results might be unexpected: '' == false // true '' == null // true 'ab' == false // false 'ab' == null // false // It is often better to use strict comparison '' === false // false '' === null // false Loading history...
217			$sc->setDomain($request->getUri()->getHost());
218			}
219			$this->setCookie($sc);
220			}
221			}
222			}
223
224			public function withCookieHeader(RequestInterface $request)
225			{
226			$values = [];
227			$uri = $request->getUri();
228			$scheme = $uri->getScheme();
229			$host = $uri->getHost();
230			$path = $uri->getPath() ?: '/';
231
232			foreach ($this->cookies as $cookie) {
233			if ($cookie->matchesPath($path) &&
234			$cookie->matchesDomain($host) &&
235			!$cookie->isExpired() &&
236			(!$cookie->getSecure() \|\| $scheme === 'https')
237			) {
238			$values[] = $cookie->getName() . '='
239			. $cookie->getValue();
240			}
241			}
242
243			return $values
244			? $request->withHeader('Cookie', implode('; ', $values))
245			: $request;
246			}
247
248			/**
249			* If a cookie already exists and the server asks to set it again with a
250			* null value, the cookie must be deleted.
251			*
252			* @param SetCookie $cookie
253			*/
254			private function removeCookieIfEmpty(SetCookie $cookie)
255			{
256			$cookieValue = $cookie->getValue();
257			if ($cookieValue === null \|\| $cookieValue === '') {
258			$this->clear(
259			$cookie->getDomain(),
260			$cookie->getPath(),
261			$cookie->getName()
262			);
263			}
264			}
265			}
266

guzzle / guzzle

Push — master ( 8bc9d2...ec70be )

src/Cookie/CookieJar.php (8 issues)

Labels

Severity

Introduced By

Upgrade to new PHP Analysis Engine

Available Fixes

Duplication Side-by-Side

Filter issues like