Connection::onRead()   F
last analyzed

Complexity

Conditions 25
Paths 2

Size

Total Lines 147

Duplication

Lines 0
Ratio 0 %

Importance

Changes 0
Metric Value
cc 25
dl 0
loc 147
rs 3.3333
c 0
b 0
f 0
nc 2
nop 0

How to fix   Long Method    Complexity   

Long Method

Small methods make your code easier to understand, in particular if combined with a good name. Besides, if your method is small, finding a good name is usually much easier.

For example, if you find yourself adding comments to a method's body, this is usually a good sign to extract the commented part to a new method, and use the comment as a starting point when coming up with a good name for this new method.

Commonly applied refactorings include:

1
<?php
2
namespace PHPDaemon\Servers\Socks;
3
4
/**
5
 * @package    NetworkServers
6
 * @subpackage SocksServer
7
 * @author     Vasily Zorin <[email protected]>
8
 */
9
class Connection extends \PHPDaemon\Network\Connection
10
{
11
    /**
12
     * @TODO DESCR
13
     */
14
    const STATE_ABORTED = 1;
15
    /**
16
     * @TODO DESCR
17
     */
18
    const STATE_HANDSHAKED = 2;
19
    /**
20
     * @TODO DESCR
21
     */
22
    const STATE_AUTHORIZED = 3;
23
    /**
24
     * @TODO DESCR
25
     */
26
    const STATE_DATAFLOW = 4;
27
    /**
28
     * @var string protocol version (X'04' / X'05')
29
     */
30
    protected $ver;
31
    /**
32
     * @var integer State: 0 - start, 1 - aborted, 2 - handshaked, 3 - authorized, 4 - data exchange
33
     */
34
    protected $state = 0;
35
    protected $slave;
36
    protected $lowMark = 2;
37
    protected $highMark = 32768;
38
39
    /**
40
     * Called when new data received.
41
     * @return void
42
     */
43
    public function onRead()
44
    {
45
        if ($this->state === self::STATE_DATAFLOW) {
46
            // Data exchange
47
            if ($this->slave) {
48
                do {
49
                    $this->slave->writeFromBuffer($this->bev->input, $this->bev->input->length);
50
                } while ($this->bev->input->length > 0);
51
            }
52
53
            return;
54
        }
55
56
        start:
57
58
        $l = $this->bev->input->length;
59
60
        if ($this->state === self::STATE_ROOT) {
61
            // Start
62
            if ($l < 2) {
63
                // Not enough data yet
64
                return;
65
            }
66
            $n = ord($this->look(1, 1));
67
            if ($l < $n + 2) {
68
                // Not enough data yet
69
                return;
70
            }
71
            $this->ver = $this->look(1);
0 ignored issues
show
Documentation Bug introduced by
It seems like $this->look(1) can also be of type false. However, the property $ver is declared as type string. 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...
72
            $this->drain(2);
73
            $methods = $this->read($n);
74
75
            if (!$this->pool->config->auth->value) {
76
                // No auth
77
                $m = "\x00";
78
                $this->state = self::STATE_AUTHORIZED;
79
            } elseif (mb_orig_strpos($methods, "\x02") !== false) {
0 ignored issues
show
Security Bug introduced by
It seems like $methods defined by $this->read($n) on line 73 can also be of type false; however, mb_orig_strpos() does only seem to accept string, did you maybe forget to handle an error condition?

This check looks for type mismatches where the missing type is false. This is usually indicative of an error condtion.

Consider the follow example

<?php

function getDate($date)
{
    if ($date !== null) {
        return new DateTime($date);
    }

    return false;
}

This function either returns a new DateTime object or false, if there was an error. This is a typical pattern in PHP programming to show that an error has occurred without raising an exception. The calling code should check for this returned false before passing on the value to another function or method that may not be able to handle a false.

Loading history...
80
                // Username/Password authentication
81
                $m = "\x02";
82
                $this->state = self::STATE_HANDSHAKED;
83
            } else {
84
                // No allowed methods
85
                $m = "\xFF";
86
                $this->state = self::STATE_ABORTED;
87
            }
88
89
            $this->write($this->ver . $m);
90
91
            if ($this->state === self::STATE_ABORTED) {
92
                $this->finish();
93
            } else {
94
                goto start;
95
            }
96
        } elseif ($this->state === self::STATE_HANDSHAKED) {
97
            // Handshaked
98
            if ($l < 3) {
99
                // Not enough data yet
100
                return;
101
            }
102
103
            $ver = $this->look(1);
104
105
            if ($ver !== $this->ver) {
106
                $this->finish();
107
                return;
108
            }
109
110
            $ulen = ord($this->look(1, 1));
111
            if ($l < 3 + $ulen) {
112
                // Not enough data yet
113
                return;
114
            }
115
            $username = $this->look(2, $ulen);
116
            $plen = ord($this->look(2 + $ulen, 1));
117
118
            if ($l < 3 + $ulen + $plen) {
119
                // Not enough data yet
120
                return;
121
            }
122
            $this->drain(3 + $ulen);
123
            $password = $this->read($plen);
124
125
            if (($username !== $this->pool->config->username->value)
126
                || ($password !== $this->pool->config->password->value)
127
            ) {
128
                $this->state = self::STATE_ABORTED;
129
                $m = "\x01";
130
            } else {
131
                $this->state = self::STATE_AUTHORIZED;
132
                $m = "\x00";
133
            }
134
            $this->write($this->ver . $m);
135
136
            if ($this->state === self::STATE_ABORTED) {
137
                $this->finish();
138
            } else {
139
                goto start;
140
            }
141
        } elseif ($this->state === self::STATE_AUTHORIZED) {
142
            // Ready for query
143
            if ($l < 4) {
144
                // Not enough data yet
145
                return;
146
            }
147
            $ver = $this->read(1);
148
149
            if ($ver !== $this->ver) {
150
                $this->finish();
151
                return;
152
            }
153
154
            $cmd = $this->read(1);
0 ignored issues
show
Unused Code introduced by
$cmd 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...
155
            $this->drain(1);
156
            $atype = $this->read(1);
157
            if ($atype === "\x01") {
158
                $address = inet_ntop($this->read(4));
159
            } elseif ($atype === "\x03") {
160
                $len = ord($this->read(1));
161
                $address = $this->read($len);
162
            } elseif ($atype === "\x04") {
163
                $address = inet_ntop($this->read(16));
164
            } else {
165
                $this->finish();
166
                return;
167
            }
168
169
            $u = unpack('nport', $this->read(2));
170
            $port = $u['port'];
171
172
            $this->destAddr = $address;
0 ignored issues
show
Documentation introduced by
The property destAddr does not exist on object<PHPDaemon\Servers\Socks\Connection>. Since you implemented __set, maybe consider adding a @property annotation.

Since your code implements the magic setter _set, this function will be called for any write access on an undefined variable. You can add the @property annotation to your class or interface to document the existence of this variable.

<?php

/**
 * @property int $x
 * @property int $y
 * @property string $text
 */
class MyLabel
{
    private $properties;

    private $allowedProperties = array('x', 'y', 'text');

    public function __get($name)
    {
        if (isset($properties[$name]) && in_array($name, $this->allowedProperties)) {
            return $properties[$name];
        } else {
            return null;
        }
    }

    public function __set($name, $value)
    {
        if (in_array($name, $this->allowedProperties)) {
            $properties[$name] = $value;
        } else {
            throw new \LogicException("Property $name is not defined.");
        }
    }

}

Since the property has write access only, you can use the @property-write annotation instead.

Of course, you may also just have mistyped another name, in which case you should fix the error.

See also the PhpDoc documentation for @property.

Loading history...
173
            $this->destPort = $port;
0 ignored issues
show
Documentation introduced by
The property destPort does not exist on object<PHPDaemon\Servers\Socks\Connection>. Since you implemented __set, maybe consider adding a @property annotation.

Since your code implements the magic setter _set, this function will be called for any write access on an undefined variable. You can add the @property annotation to your class or interface to document the existence of this variable.

<?php

/**
 * @property int $x
 * @property int $y
 * @property string $text
 */
class MyLabel
{
    private $properties;

    private $allowedProperties = array('x', 'y', 'text');

    public function __get($name)
    {
        if (isset($properties[$name]) && in_array($name, $this->allowedProperties)) {
            return $properties[$name];
        } else {
            return null;
        }
    }

    public function __set($name, $value)
    {
        if (in_array($name, $this->allowedProperties)) {
            $properties[$name] = $value;
        } else {
            throw new \LogicException("Property $name is not defined.");
        }
    }

}

Since the property has write access only, you can use the @property-write annotation instead.

Of course, you may also just have mistyped another name, in which case you should fix the error.

See also the PhpDoc documentation for @property.

Loading history...
174
            $this->pool->connect('tcp://' . $this->destAddr . ':' . $this->destPort, function ($conn) {
0 ignored issues
show
Documentation introduced by
The property destAddr does not exist on object<PHPDaemon\Servers\Socks\Connection>. Since you implemented __get, maybe consider adding a @property annotation.

Since your code implements the magic getter _get, this function will be called for any read access on an undefined variable. You can add the @property annotation to your class or interface to document the existence of this variable.

<?php

/**
 * @property int $x
 * @property int $y
 * @property string $text
 */
class MyLabel
{
    private $properties;

    private $allowedProperties = array('x', 'y', 'text');

    public function __get($name)
    {
        if (isset($properties[$name]) && in_array($name, $this->allowedProperties)) {
            return $properties[$name];
        } else {
            return null;
        }
    }

    public function __set($name, $value)
    {
        if (in_array($name, $this->allowedProperties)) {
            $properties[$name] = $value;
        } else {
            throw new \LogicException("Property $name is not defined.");
        }
    }

}

If the property has read access only, you can use the @property-read annotation instead.

Of course, you may also just have mistyped another name, in which case you should fix the error.

See also the PhpDoc documentation for @property.

Loading history...
Documentation introduced by
The property destPort does not exist on object<PHPDaemon\Servers\Socks\Connection>. Since you implemented __get, maybe consider adding a @property annotation.

Since your code implements the magic getter _get, this function will be called for any read access on an undefined variable. You can add the @property annotation to your class or interface to document the existence of this variable.

<?php

/**
 * @property int $x
 * @property int $y
 * @property string $text
 */
class MyLabel
{
    private $properties;

    private $allowedProperties = array('x', 'y', 'text');

    public function __get($name)
    {
        if (isset($properties[$name]) && in_array($name, $this->allowedProperties)) {
            return $properties[$name];
        } else {
            return null;
        }
    }

    public function __set($name, $value)
    {
        if (in_array($name, $this->allowedProperties)) {
            $properties[$name] = $value;
        } else {
            throw new \LogicException("Property $name is not defined.");
        }
    }

}

If the property has read access only, you can use the @property-read annotation instead.

Of course, you may also just have mistyped another name, in which case you should fix the error.

See also the PhpDoc documentation for @property.

Loading history...
175
                if (!$conn) {
176
                    // Early connection error
177
                    $this->write($this->ver . "\x05");
178
                    $this->finish();
179
                } else {
180
                    $conn->setClient($this);
181
                    $this->state = self::STATE_DATAFLOW;
182
                    $conn->getSocketName($addr, $port);
0 ignored issues
show
Bug introduced by
The variable $addr does not exist. Did you forget to declare it?

This check marks access to variables or properties that have not been declared yet. While PHP has no explicit notion of declaring a variable, accessing it before a value is assigned to it is most likely a bug.

Loading history...
Bug introduced by
The variable $port does not exist. Did you forget to declare it?

This check marks access to variables or properties that have not been declared yet. While PHP has no explicit notion of declaring a variable, accessing it before a value is assigned to it is most likely a bug.

Loading history...
183
                    $this->slave = $conn;
184
                    $this->onSlaveReady(0x00, $addr, $port);
185
                    $this->onReadEv(null);
0 ignored issues
show
Documentation introduced by
null is of type null, but the function expects a object.

It seems like the type of the argument is not accepted by the function/method which you are calling.

In some cases, in particular if PHP’s automatic type-juggling kicks in this might be fine. In other cases, however this might be a bug.

We suggest to add an explicit type cast like in the following example:

function acceptsInteger($int) { }

$x = '123'; // string "123"

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);
Loading history...
186
                }
187
            }, 'SocksServerSlaveConnection');
188
        }
189
    }
190
191
    /**
192
     * @TODO
193
     * @param integer $code
194
     * @param string $addr
195
     * @param integer $port
196
     */
197
    public function onSlaveReady($code, $addr, $port)
0 ignored issues
show
Unused Code introduced by
The parameter $port is not used and could be removed.

This check looks from parameters that have been defined for a function or method, but which are not used in the method body.

Loading history...
198
    {
199
        $reply =
200
            $this->ver // Version
201
            . chr($code) // Status
202
            . "\x00"; // Reserved
203
        if ($addr) {
204
            $reply .=
205
                (mb_orig_strpos($addr, ':') === false ? "\x01" : "\x04") // IPv4/IPv6
206
                . inet_pton($addr) // Address
207
                . "\x00\x00"; //pack('n',$port) // Port
208
        } else {
209
            $reply .=
210
                "\x01"
211
                . "\x00\x00\x00\x00"
212
                . "\x00\x00";
213
        }
214
215
        $this->write($reply);
216
    }
217
218
    /**
219
     * @TODO DESCR
220
     */
221
    public function onFinish()
222
    {
223
        if (isset($this->slave)) {
224
            $this->slave->finish();
225
            unset($this->slave);
226
        }
227
    }
228
}
229