Test Failed
Branch develop (db5506)
by Felipe
03:46
created

SqlController::doFooter()   C

Complexity

Conditions 11
Paths 80

Size

Total Lines 97
Code Lines 52

Duplication

Lines 25
Ratio 25.77 %

Importance

Changes 0
Metric Value
cc 11
eloc 52
nc 80
nop 2
dl 25
loc 97
rs 5.2653
c 0
b 0
f 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
3
namespace PHPPgAdmin\Controller;
4
5
/**
6
 * Base controller class
7
 */
8
class SqlController extends BaseController
9
{
10
    public $_name      = 'SqlController';
11
    public $query      = '';
12
    public $subject    = '';
13
    public $start_time = null;
14
    public $duration   = null;
15
16
    public function render()
0 ignored issues
show
Coding Style introduced by
render uses the super-global variable $_REQUEST which is generally not recommended.

Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable:

// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

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

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
Loading history...
Coding Style introduced by
render uses the super-global variable $_SESSION which is generally not recommended.

Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable:

// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

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

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
Loading history...
Coding Style introduced by
render uses the super-global variable $_GET which is generally not recommended.

Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable:

// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

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

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
Loading history...
Coding Style introduced by
render uses the super-global variable $_POST which is generally not recommended.

Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable:

// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

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

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
Loading history...
Coding Style introduced by
render uses the super-global variable $_FILES which is generally not recommended.

Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable:

// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

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

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
Loading history...
17
    {
18
        $conf = $this->conf;
19
        $lang = $this->lang;
20
        $misc = $this->misc;
21
        $data = $misc->getDatabaseAccessor();
22
23
        set_time_limit(0);
24
25
        // We need to store the query in a session for editing purposes
26
        // We avoid GPC vars to avoid truncating long queries
27
        if (isset($_REQUEST['subject']) && $_REQUEST['subject'] == 'history') {
28
            // Or maybe we came from the history popup
29
            $_SESSION['sqlquery'] = $_SESSION['history'][$_REQUEST['server']][$_REQUEST['database']][$_GET['queryid']]['query'];
30
            $this->query          = $_SESSION['sqlquery'];
31
        } elseif (isset($_POST['query'])) {
32
            // Or maybe we came from an sql form
33
            $_SESSION['sqlquery'] = $_POST['query'];
34
            $this->query          = $_SESSION['sqlquery'];
35
        } else {
36
            echo 'could not find the query!!';
37
        }
38
39
        // Pagination maybe set by a get link that has it as FALSE,
40
        // if that's the case, unset the variable.
41
        if (isset($_REQUEST['paginate']) && $_REQUEST['paginate'] == 'f') {
42
            unset($_REQUEST['paginate']);
43
            unset($_POST['paginate']);
44
            unset($_GET['paginate']);
45
        }
46
47
        if (isset($_REQUEST['subject'])) {
48
            $this->subject = $_REQUEST['subject'];
49
        }
50
51
        // Check to see if pagination has been specified. In that case, send to display
52
        // script for pagination
53
        /* if a file is given or the request is an explain, do not paginate */
54
        if (isset($_REQUEST['paginate']) && !(isset($_FILES['script']) && $_FILES['script']['size'] > 0) && (preg_match('/^\s*explain/i', $this->query) == 0)) {
55
            //if (!(isset($_FILES['script']) && $_FILES['script']['size'] > 0)) {
56
57
            $display_controller = new DisplayController($this->getContainer());
58
59
            return $display_controller->render();
60
        }
61
62
        $this->printHeader($lang['strqueryresults'], null, true, 'header_sqledit.twig');
63
        $this->printBody();
64
        $this->printTrail('database');
65
        $this->printTitle($lang['strqueryresults']);
66
67
        // Set the schema search path
68 View Code Duplication
        if (isset($_REQUEST['search_path'])) {
0 ignored issues
show
Duplication introduced by
This code seems to be duplicated across your project.

Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation.

You can also find more detailed suggestions in the “Code” section of your repository.

Loading history...
69
            if ($data->setSearchPath(array_map('trim', explode(',', $_REQUEST['search_path']))) != 0) {
70
                return $this->printFooter();
71
            }
72
        }
73
74
        // May as well try to time the query
75
        if (function_exists('microtime')) {
76
            list($usec, $sec) = explode(' ', microtime());
77
            $this->start_time = ((float) $usec + (float) $sec);
78
        }
79
80
        $this->doDefault();
81
82
        $this->doFooter(true, 'footer_sqledit.twig');
83
    }
84
85
    public function doDefault()
0 ignored issues
show
Coding Style introduced by
doDefault uses the super-global variable $_FILES which is generally not recommended.

Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable:

// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

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

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
Loading history...
86
    {
87
        $conf        = $this->conf;
88
        $misc        = $this->misc;
89
        $lang        = $this->lang;
90
        $data        = $misc->getDatabaseAccessor();
91
        $_connection = $misc->getConnection();
92
93
        try {
94
            // Execute the query.  If it's a script upload, special handling is necessary
95
            if (isset($_FILES['script']) && $_FILES['script']['size'] > 0) {
96
                return $this->execute_script();
97
            } else {
98
                return $this->execute_query();
99
            }
100
        } catch (\PHPPgAdmin\ADOdbException $e) {
101
            $message   = $e->getMessage();
102
            $trace     = $e->getTraceAsString();
103
            $lastError = $_connection->getLastError();
104
            $this->prtrace(['message' => $message, 'trace' => $trace, 'lastError' => $lastError]);
105
106
            return null;
107
        }
108
    }
109
110
    private function execute_script()
0 ignored issues
show
Coding Style introduced by
execute_script uses the super-global variable $_FILES which is generally not recommended.

Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable:

// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

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

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
Loading history...
111
    {
112
        $conf        = $this->conf;
113
        $misc        = $this->misc;
114
        $lang        = $this->lang;
115
        $data        = $misc->getDatabaseAccessor();
116
        $_connection = $misc->getConnection();
117
118
        /**
119
         * This is a callback function to display the result of each separate query
120
         * @param ADORecordSet $rs The recordset returned by the script execetor
121
         */
122
        $sqlCallback = function ($query, $rs, $lineno) use ($data, $misc, $lang, $_connection) {
123
124
            // Check if $rs is false, if so then there was a fatal error
125
            if ($rs === false) {
126
                echo htmlspecialchars($_FILES['script']['name']), ':', $lineno, ': ', nl2br(htmlspecialchars($_connection->getLastError())), "<br/>\n";
127
            } else {
128
                // Print query results
129
                switch (pg_result_status($rs)) {
130
                    case PGSQL_TUPLES_OK:
131
                        // If rows returned, then display the results
132
                        $num_fields = pg_numfields($rs);
133
                        echo "<p><table>\n<tr>";
134
                        for ($k = 0; $k < $num_fields; $k++) {
135
                            echo '<th class="data">', $misc->printVal(pg_fieldname($rs, $k)), '</th>';
136
                        }
137
138
                        $i   = 0;
139
                        $row = pg_fetch_row($rs);
140
                        while ($row !== false) {
141
                            $id = (($i % 2) == 0 ? '1' : '2');
142
                            echo "<tr class=\"data{$id}\">\n";
143
                            foreach ($row as $k => $v) {
144
                                echo '<td style="white-space:nowrap;">', $misc->printVal($v, pg_fieldtype($rs, $k), ['null' => true]), '</td>';
145
                            }
146
                            echo "</tr>\n";
147
                            $row = pg_fetch_row($rs);
148
                            $i++;
149
                        }
150
                        ;
151
                        echo "</table><br/>\n";
152
                        echo $i, " {$lang['strrows']}</p>\n";
153
                        break;
154
                    case PGSQL_COMMAND_OK:
155
                        // If we have the command completion tag
156
                        if (version_compare(phpversion(), '4.3', '>=')) {
157
                            echo htmlspecialchars(pg_result_status($rs, PGSQL_STATUS_STRING)), "<br/>\n";
158
                        }
159
                        // Otherwise if any rows have been affected
160
                        elseif ($data->conn->Affected_Rows() > 0) {
161
                            echo $data->conn->Affected_Rows(), " {$lang['strrowsaff']}<br/>\n";
162
                        }
163
                        // Otherwise output nothing...
164
                        break;
165
                    case PGSQL_EMPTY_QUERY:
166
                        break;
167
                    default:
168
                        break;
169
                }
170
            }
171
        };
172
173
        return $data->executeScript('script', $sqlCallback);
174
    }
175
176
    private function execute_query()
0 ignored issues
show
Coding Style introduced by
execute_query uses the super-global variable $_SERVER which is generally not recommended.

Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable:

// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

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

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
Loading history...
Coding Style introduced by
execute_query uses the super-global variable $_REQUEST which is generally not recommended.

Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable:

// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

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

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
Loading history...
177
    {
178
        $conf = $this->conf;
179
        $misc = $this->misc;
180
        $lang = $this->lang;
181
        $data = $misc->getDatabaseAccessor();
182
183
        // Set fetch mode to NUM so that duplicate field names are properly returned
184
        $data->conn->setFetchMode(ADODB_FETCH_NUM);
185
186
        $rs = $data->conn->Execute($this->query);
187
188
        echo '<form method="post" id="sqlform" action="' . $_SERVER['REQUEST_URI'] . '">';
0 ignored issues
show
Security introduced by
'<form method="post" id=...R['REQUEST_URI'] . '">' can contain request data and is used in output context(s) leading to a potential security vulnerability.

1 path for user data to reach this point

  1. Read tainted data from array
    in src/controllers/SqlController.php on line 188

Preventing Cross-Site-Scripting Attacks

Cross-Site-Scripting allows an attacker to inject malicious code into your website - in particular Javascript code, and have that code executed with the privileges of a visiting user. This can be used to obtain data, or perform actions on behalf of that visiting user.

In order to prevent this, make sure to escape all user-provided data:

// for HTML
$sanitized = htmlentities($tainted, ENT_QUOTES);

// for URLs
$sanitized = urlencode($tainted);

General Strategies to prevent injection

In general, it is advisable to prevent any user-data to reach this point. This can be done by white-listing certain values:

if ( ! in_array($value, array('this-is-allowed', 'and-this-too'), true)) {
    throw new \InvalidArgumentException('This input is not allowed.');
}

For numeric data, we recommend to explicitly cast the data:

$sanitized = (integer) $tainted;
Loading history...
189
        echo '<textarea width="90%" name="query"  id="query" rows="5" cols="100" resizable="true">';
190
191
        echo htmlspecialchars($this->query);
192
        echo '</textarea><br>';
193
        echo $misc->setForm();
194
        echo '<input type="submit"/></form>';
195
196
        // $rs will only be an object if there is no error
197
        if (is_object($rs)) {
198
            // Request was run, saving it in history
199
            if (!isset($_REQUEST['nohistory'])) {
200
                $misc->saveScriptHistory($this->query);
201
            }
202
203
            // Now, depending on what happened do various things
204
205
            // First, if rows returned, then display the results
206
            if ($rs->recordCount() > 0) {
207
                echo "<table>\n<tr>";
208 View Code Duplication
                foreach ($rs->fields as $k => $v) {
0 ignored issues
show
Duplication introduced by
This code seems to be duplicated across your project.

Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation.

You can also find more detailed suggestions in the “Code” section of your repository.

Loading history...
209
                    $finfo = $rs->fetchField($k);
210
                    echo '<th class="data">', $misc->printVal($finfo->name), '</th>';
211
                }
212
                echo "</tr>\n";
213
                $i = 0;
214
                while (!$rs->EOF) {
215
                    $id = (($i % 2) == 0 ? '1' : '2');
216
                    echo "<tr class=\"data{$id}\">\n";
217 View Code Duplication
                    foreach ($rs->fields as $k => $v) {
0 ignored issues
show
Duplication introduced by
This code seems to be duplicated across your project.

Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation.

You can also find more detailed suggestions in the “Code” section of your repository.

Loading history...
218
                        $finfo = $rs->fetchField($k);
219
                        echo '<td style="white-space:nowrap;">', $misc->printVal($v, $finfo->type, ['null' => true]), '</td>';
220
                    }
221
                    echo "</tr>\n";
222
                    $rs->moveNext();
223
                    $i++;
224
                }
225
                echo "</table>\n";
226
                echo '<p>', $rs->recordCount(), " {$lang['strrows']}</p>\n";
227
            } elseif ($data->conn->Affected_Rows() > 0) {
228
                // Otherwise if any rows have been affected
229
                echo '<p>', $data->conn->Affected_Rows(), " {$lang['strrowsaff']}</p>\n";
230
            } else {
231
                // Otherwise nodata to print
232
                echo '<p>', $lang['strnodata'], "</p>\n";
233
            }
234
        }
235
    }
236
237
    private function doFooter($doBody = true, $template = 'footer.twig')
0 ignored issues
show
Coding Style introduced by
doFooter uses the super-global variable $_REQUEST which is generally not recommended.

Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable:

// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

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

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
Loading history...
238
    {
239
        $conf = $this->conf;
240
        $misc = $this->misc;
241
        $lang = $this->lang;
242
        $data = $misc->getDatabaseAccessor();
243
244
        // May as well try to time the query
245
        if ($this->start_time !== null) {
246
            list($usec, $sec) = explode(' ', microtime());
247
            $end_time         = ((float) $usec + (float) $sec);
248
            // Get duration in milliseconds, round to 3dp's
249
            $this->duration = number_format(($end_time - $this->start_time) * 1000, 3);
250
        }
251
252
        // Reload the browser as we may have made schema changes
253
        $misc->setReloadBrowser(true);
254
255
        // Display duration if we know it
256
        if ($this->duration !== null) {
257
            echo '<p>', sprintf($lang['strruntime'], $this->duration), "</p>\n";
258
        }
259
260
        echo "<p>{$lang['strsqlexecuted']}</p>\n";
261
262
        $navlinks = [];
263
        $fields   = [
264
            'server'   => $_REQUEST['server'],
265
            'database' => $_REQUEST['database'],
266
        ];
267
268
        if (isset($_REQUEST['schema'])) {
269
            $fields['schema'] = $_REQUEST['schema'];
270
        }
271
272
        // Return
273 View Code Duplication
        if (isset($_REQUEST['return'])) {
0 ignored issues
show
Duplication introduced by
This code seems to be duplicated across your project.

Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation.

You can also find more detailed suggestions in the “Code” section of your repository.

Loading history...
274
            $urlvars          = $misc->getSubjectParams($_REQUEST['return']);
275
            $navlinks['back'] = [
276
                'attr'    => [
277
                    'href' => [
278
                        'url'     => $urlvars['url'],
279
                        'urlvars' => $urlvars['params'],
280
                    ],
281
                ],
282
                'content' => $lang['strback'],
283
            ];
284
        }
285
286
        // Edit
287
        $navlinks['alter'] = [
288
            'attr'    => [
289
                'href' => [
290
                    'url'     => 'database.php',
291
                    'urlvars' => array_merge($fields, [
292
                        'action' => 'sql',
293
                    ]),
294
                ],
295
            ],
296
            'content' => $lang['streditsql'],
297
        ];
298
299
        // Create view and download
300
        if ($this->query !== '' && isset($rs) && is_object($rs) && $rs->recordCount() > 0) {
301
            // Report views don't set a schema, so we need to disable create view in that case
302 View Code Duplication
            if (isset($_REQUEST['schema'])) {
0 ignored issues
show
Duplication introduced by
This code seems to be duplicated across your project.

Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation.

You can also find more detailed suggestions in the “Code” section of your repository.

Loading history...
303
                $navlinks['createview'] = [
304
                    'attr'    => [
305
                        'href' => [
306
                            'url'     => 'views.php',
307
                            'urlvars' => array_merge($fields, [
308
                                'action' => 'create',
309
                            ]),
310
                        ],
311
                    ],
312
                    'content' => $lang['strcreateview'],
313
                ];
314
            }
315
316
            if (isset($_REQUEST['search_path'])) {
317
                $fields['search_path'] = $_REQUEST['search_path'];
318
            }
319
320
            $navlinks['download'] = [
321
                'attr'    => [
322
                    'href' => [
323
                        'url'     => 'dataexport.php',
324
                        'urlvars' => $fields,
325
                    ],
326
                ],
327
                'content' => $lang['strdownload'],
328
            ];
329
        }
330
331
        $this->printNavLinks($navlinks, 'sql-form', get_defined_vars());
332
333
        return $this->printFooter($doBody, $template);
334
    }
335
}
336