Hi,
The latest commit to common.lib.php does not respect PEAR standards,
IMO. Too much white space.
Have a look at how they recommend to format "if":
if ($arg1 == 'good' || $arg1 == 'fair') {
$this->foo = $arg1;
return 1;
} elseif ($arg1 == 'poor' && $arg2 > 1) {
$this->foo = 'poor';
return 2;
} else {
return false;
}
http://pear.php.net/manual/en/standards.sample.php
Hi,
please look at my commit for sql.php, revision 2.76. I have just changed
$goto by $_REQUEST['goto']. Is this the way we want to take?
It's a bit weird for me to use $_REQUEST['goto'] on the left side of an
assignment, but anyway tell me your comments so I can continue :)
Marc
Ok, I am getting lost in all those threads. I guess it's time to vote.
I'll try to summarize the issues:
- remove grab_globals, moving the GLOBALS overwrite protection into
common.lib.php
- everywhere in the code, find the variables that were set from
grab_globals and replace them by $_REQUEST['foo'] if they originated
from GET, POST or COOKIE, or by a reference to $_FILES, $_ENV or
$_SERVER. Possibly taking into account that $_ENV might not be readable
(use of getenv() ?)
- sanitize individually what can be echoed (like $message) with
PMA_sanitize(), for XSS protection. Any need to sanitize something else?
- (later) in an effort to clean global space, replace $str by constants
Comments, please :)
Marc
Causes a big bug: can no longer create a table.
This is because, in tbl_create.php:
// Does table exist?
else if (!(PMA_DBI_get_fields($db, $table) === FALSE)) {
PMA_mysqlDie(sprintf($strTableAlreadyExists,
htmlspecialchars($table)), '', '', $err_url);
and now, this function returns an empty array instead of returning false.
Marc
Hi,
how about using PHP_Compat ?
for example in register_globals()
we could just do
<?php
require_once 'PHP/Compat.php';
PHP_Compat::loadFunction('array_walk_recursive');
array_walk_recursive( $_SERVER, 'strip_tags' );
array_walk_recursive( $_ENV, 'strip_tags' );
?>
this will improve speed on systems with PHP having this functions natively.
i think there are more places where tis would be a benefit
--
Sebastian Mendel
www.sebastianmendel.dewww.sf.net/projects/phpdatetime | www.sf.net/projects/phptimesheet
Congratulations Michal for this excellent script. I have a few comments:
- In the Servers section, could we have just one dropdown of servers,
then the 2 buttons Delete and Edit next to the dropdown?
- In the doc, should we give the UNIX commands for proper permission and
ownership of the config directory? We are already doing it in the doc
for another subject.
- PMA's behavior (with default configuration) has always been to try to
connect to a localhost server with root and no password (to match
default MySQL installation), but now I suggest that PMA goes into setup
mode if no config.inc.php is found.
- Maybe in the Servers section or in the "Configure Server" dialog a
"Add default MySQL server" button or something like that.
- Maybe in the Configuration section, display Save and Load in grey when
they are not available, with a link to the doc explaining why.
Marc
