Hi!
Wow, that is a great plugin! I am sure many people will love that to easily enhance their sidebar links! Great work.
I just inspected your plugin and only have a few recommendations:
1. You set the defaults of the "Next", "previous" etc. config items to hard-coded english strings. Instead I suggest to use also constants there, like this:
Code: Select all
case 'next':
$propbag->add('type', 'string');
$propbag->add('name', PLUGIN_QUICKLINK_NEXT);
$propbag->add('description', PLUGIN_QUICKLINK_NEXT_BLAHBLAH);
$propbag->add('default', NEXT_PAGE);
break;
2. You use this code to detect limiting:
Code: Select all
if (!empty($serendipity['GET']['start'])) {
$start = $serendipity['GET']['start'];
}else{
$start = 0;
}
This can be used for SQL injection. Use this instaed:
Code: Select all
if (!empty($serendipity['GET']['start'])) {
$start = (int)$serendipity['GET']['start'];
}else{
$start = 0;
}
3. You use a limit clause like this:
Code: Select all
LIMIT ' . $start . ',' . $max_entries;
However that will not work on postgresql. You should use this serendipity function instead:
Code: Select all
' . serendipity_db_limit_sql(serendipity_db_limit($start, $max_entries)) . '
4. You just use
Code: Select all
if ($_SESSION['serendipityAuthedUser'] === true) {
to check if a user can delete quicklinks. With that you also allow any editor to a blog to delete something; instead I suggest to also check the userlevel and restrict it to chiefs:
Code: Select all
if ($_SESSION['serendipityAuthedUser'] === true && $serendipity['serendipityUserlevel'] >= USERLEVEL_CHIEF) {
If you have the time to change thos issues, would you like to commit the plugin for Spartacus? I think it would be great!
Best regards,
Garvin
):
