Denied

jettyrat · Moderator Joined: Nov 28, 2005 Posts: 1040

Not so surprisingly, the mod submission to the phpbb mods database was denied. Confused

Some of the issues I don't necessarily agree with and some are good points that should be addressed.

Anyway, here is the report I received if anyone is interested...

Quote: › All files except integration.xml, modx.prosilver.en.xsl and license.txt should be removed or moved to contrib/. Same for most of the dirs.

All new files added by your MOD should be placed in a directory called root/.

Quote: › Regarding easymod, it is recommended these mods be carried out by hand. It is not garaunteed that easymod will be be able to process all of these mods correctly. Because:

1. The integration has not been tested with easymod.
2. The # sign is a valid .css character and it is the comment character for phpBB mod files.
3. Directories named 'local' need to be created in the gallery themes templates directories and files copied there.
4. There are mods to Gallery 2 files that normally reside outside of the phpBB directory.
5. The entire g2image directory needs to be copied to the phpBB3 directory.

If you insist on using easymod, it will be up to you to ensure that all mods are carried out correctly and you do so at your own risk!

There is no easymod for phpBB3.

New permissions should not be added with a sql query, but with a correct call to the function in includes/acp/auth.php.

Code: ›
$referer = (empty($_SERVER['HTTP_REFERER'])) ? "{$phpbb_root_path}index.$phpEx" : $_SERVER['HTTP_REFERER'];

if ($user->data['is_registered'])
{
redirect(append_sid($referer));
}

login_box(request_var('redirect', $referer));

Should use $user->page to prevent XSS.

Instead of adding a new table for config settings I suggest you use phpbb_config. This will cause for less needed querys.

All files containing php should have as extension .php. Also, all included file should have a check for IN_PHPBB.

trigger_error should be used for error handling, not msg_handler();.

coding guidelines stuff: New braces should be always at a newline.

Code: ›
$sql = 'SELECT DISTINCT g.group_name FROM ' . GROUPS_TABLE . ' g, ' . USER_GROUP_TABLE . " ug WHERE ug.user_id = $id AND ug.group_id = g.group_id AND g.group_name NOT IN (" . implode(', ', $phpbbDefaultGroups) . ')';

Should use $db->sql_in_set.

templates/ dir doesnt follow package guidelines.

Please read our package guidelines and code guidelines before you resubmit your MOD.

AdBot

jettyrat · Moderator Joined: Nov 28, 2005 Posts: 1040

I got all these issues fixed over the weekend, plus a lot more coding issues and made some better use of some of the built-in functions in phpbb3.

alitogata · Beginner Joined: Nov 06, 2007 Posts: 7

that's good news jettyrat. This is by far one of the best mods for phpbb3 Wink

That means that there will be a new version once it is accepted?

jettyrat · Moderator Joined: Nov 28, 2005 Posts: 1040

Eventually there will have to be. 1.0.0 was denied so the next one I submit has to be a different version number. I'm working on 1.0.1 now. Parts of the code have changed, but functionality-wise it still does the same things. I'm just taking what I already have and trying to make it acceptable to their mod standards.

I did have to remove the login redirect stuff. They are worried about XSS vulnerabilities. I couldn't get $user->page to work as suggested in the report. It always returns the current page, not the referring page so I don't see how to use that. It's a minor detail of the integration that was actually suggested here on the forum, so I've just taken it out for now.

Also, had to remove the progress bar during user export because it opens in a page outside of the phpbb template system and I don't see any other obvious way to make that work and I'm trying to eliminate any potential reasons for rejection this time because that is one of the mod rules that every page goes in the template system. Kind of unfortunate because I actually liked the progress bar, but now it just sits there until it completes and goes on to the status page. That's the way phpbb works when you install it anyways; just sits there while it's building the db and all...

alitogata · Beginner Joined: Nov 06, 2007 Posts: 7

that's good news, I am sure you'll do an excellent job with the mod standards. Can't wait for the final version

darkxst · Beginner Joined: Mar 25, 2006 Posts: 10

jettyrat wrote: ›

I did have to remove the login redirect stuff. They are worried about XSS vulnerabilities. I couldn't get $user->page to work as suggested in the report. It always returns the current page, not the referring page so I don't see how to use that. It's a minor detail of the integration that was actually suggested here on the forum, so I've just taken it out for now.

I dont think $user->page contains any information about the referring page, just the current page.

However you could check the domain in the URL returned from $_SERVER['HTTP_REFERER']. This would stop XSS.

jettyrat · Moderator Joined: Nov 28, 2005 Posts: 1040

The server var is what it was using. That's what they didn't like and told me to use $user->page.