HybridSession::setKey()   A
last analyzed

Complexity

Conditions 2
Paths 2

Size

Total Lines 10

Duplication

Lines 0
Ratio 0 %

Importance

Changes 0
Metric Value
dl 0
loc 10
rs 9.9332
c 0
b 0
f 0
cc 2
nc 2
nop 1
1
<?php
2
3
namespace SilverStripe\HybridSessions;
4
5
use SessionHandlerInterface;
6
use SilverStripe\Core\Injector\Injector;
7
use SilverStripe\HybridSessions\Store\BaseStore;
8
9
class HybridSession extends BaseStore
10
{
11
12
    /**
13
     * List of session handlers
14
     *
15
     * @var array
16
     */
17
    protected $handlers = [];
18
19
    /**
20
     * True if this session store has been initialised
21
     *
22
     * @var bool
23
     */
24
    protected static $enabled = false;
25
26
    /**
27
     * @param SessionHandlerInterface[]
28
     *
29
     * @return $this
30
     */
31
    public function setHandlers($handlers)
32
    {
33
        $this->handlers = $handlers;
34
        $this->setKey($this->getKey());
35
36
        return $this;
37
    }
38
39
    /**
40
     * @param string
41
     *
42
     * @return $this
43
     */
44
    public function setKey($key)
45
    {
46
        parent::setKey($key);
47
48
        foreach ($this->getHandlers() as $handler) {
49
            $handler->setKey($key);
0 ignored issues
show
Bug introduced by
It seems like you code against a concrete implementation and not the interface SessionHandlerInterface as the method setKey() does only exist in the following implementations of said interface: SilverStripe\HybridSessions\HybridSession, SilverStripe\HybridSessions\Store\BaseStore, SilverStripe\HybridSessions\Store\CookieStore, SilverStripe\HybridSessions\Store\DatabaseStore, SilverStripe\HybridSessi...s\Store\TestCookieStore.

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

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. 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.
  1. Add the method to the interface:

    interface User
    {
        /** @return string */
        public function getPassword();
    
        /** @return string */
        public function getDisplayName();
    }
    
Loading history...
50
        }
51
52
        return $this;
53
    }
54
55
    /**
56
     * @return SessionHandlerInterface[]
57
     */
58
    public function getHandlers()
59
    {
60
        return $this->handlers ?: [];
61
    }
62
63
    /**
64
     * @param string $save_path
65
     * @param string $name
66
     *
67
     * @return bool
68
     */
69
    public function open($save_path, $name)
70
    {
71
        foreach ($this->getHandlers() as $handler) {
72
            $handler->open($save_path, $name);
73
        }
74
75
        return true;
76
    }
77
78
    /**
79
     * @return bool
80
     */
81
    public function close()
82
    {
83
        foreach ($this->getHandlers() as $handler) {
84
            $handler->close();
85
        }
86
87
        return true;
88
    }
89
90
    /**
91
     * @param string $session_id
92
     *
93
     * @return string
94
     */
95
    public function read($session_id)
96
    {
97
        foreach ($this->getHandlers() as $handler) {
98
            if ($data = $handler->read($session_id)) {
99
                return $data;
100
            }
101
        }
102
103
        return '';
104
    }
105
106
    public function write($session_id, $session_data)
107
    {
108
        foreach ($this->getHandlers() as $handler) {
109
            if ($handler->write($session_id, $session_data)) {
110
                return true;
111
            }
112
        }
113
114
        return false;
115
    }
116
117
    public function destroy($session_id)
118
    {
119
        foreach ($this->getHandlers() as $handler) {
120
            $handler->destroy($session_id);
121
        }
122
123
        return true;
124
    }
125
126
    public function gc($maxlifetime)
127
    {
128
        foreach ($this->getHandlers() as $handler) {
129
            $handler->gc($maxlifetime);
130
        }
131
    }
132
133
    /**
134
     * Register the session handler as the default
135
     *
136
     * @param string $key Desired session key
137
     */
138
    public static function init($key = null)
139
    {
140
        $instance = Injector::inst()->get(__CLASS__);
141
142
        if (empty($key)) {
143
            user_error(
144
                'HybridSession::init() was not given a $key. Disabling cookie-based storage',
145
                E_USER_WARNING
146
            );
147
        } else {
148
            $instance->setKey($key);
149
        }
150
151
        session_set_save_handler($instance, true);
152
153
        self::$enabled = true;
154
    }
155
156
    public static function is_enabled()
157
    {
158
        return self::$enabled;
159
    }
160
}
161