CVS edit link for r0kawa

I've create a chat application that mimic gmail / facebook chat . The modules have finish and working at my website http://www.php.net.my . You can view screenshot , download and try to install it at http://code.google.com/p/zenchat/

The chat apps mimic Gmail chat or Facebook chat. It's a nice inline window on website that provide interactivity to the visitor.

CommentFileSizeAuthor
#4 zenchat-0.54.zip11.01 KBr0kawa
#1 zenchat-0.53.zip10.97 KBr0kawa

Comments

r0kawa’s picture

r0kawa commented
Status: Postponed (maintainer needs more info) » Needs review
StatusFileSize
new zenchat-0.53.zip10.97 KB
avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Issue tags: +Module review
AjK’s picture

AjK commented
Assigned: Unassigned » AjK
Status: Needs review » Needs work
  1. In function zenchat_ajax_close() please call module_invoke_all('exit'); before doing exit(0);
  2. When querying teh database for a list of users you should include "WHERE status = 1" to ensure you don't also select blocked users
  3. Did you mean to leave this in your submitted module?
    /*
 * Debug function pr
 */
function pr($d) {
	echo '<pre>';
	var_dump($d);
	echo '</pre>';
}

    As a side note, devel module offers dpm(), dpr() and other debugging functions.

  4. //sanitize ajax input
	$messagesan = $_POST['message'];
	$to = $_POST['to'];

    Eh? And does this introduce a CSRF security? (http://drupal.org/node/178896)

  5.         $a['username'] = $user->name;
	drupal_json($a);

    Is this a potential XSS issue?

r0kawa’s picture

r0kawa commented
Assigned: AjK » Unassigned
Status: Needs work » Needs review
StatusFileSize
new zenchat-0.54.zip11.01 KB

Hi Andy,
Thank you for taking your time to review my module

# In function zenchat_ajax_close() please call module_invoke_all('exit'); before doing exit(0);
- done

# When querying teh database for a list of users you should include "WHERE status = 1" to ensure you don't also select blocked users
- My query only involve in online user which join user table and session table. Block user can't login to Drupal. The query I'm using is a copy from the online user block at user core module.

# Did you mean to leave this in your submitted module?
- I've delete it and will use devel module instead.

/*
* Debug function pr
*/
function pr($d) {
    echo '<pre>';
    var_dump($d);
    echo '</pre>';
}

I've check that drupal_write_record seem to escape all the input that I've send. I've tried to find more info about this on the API, but nothing explain it.

//sanitize ajax input
    $messagesan = $_POST['message'];
    $to = $_POST['to'];

#Eh? And does this introduce a CSRF security? (http://drupal.org/node/178896)
- I'm not sure whether user from DB have escape it, but I've filter it using filter_xss function 

     $a['username'] = filter_xss($user->name);
     drupal_json($a);

In this post, I've include the version that incorporate the required change.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented

Please change only the status, when you upload new code; other metadata are not thought to be changed by the applicant.
This issue has been assigned to AjK, and you removed that assignment.

r0kawa’s picture

r0kawa commented

Do I need to resend the request ?

r0kawa’s picture

r0kawa commented

I don't think I've the option to remove an assignment. I didn't have any drop down that list AjK as the Assigned . Only Unassigned or r0kawa. Please advise on this matter.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented

You cannot restore the field Assigned as it was before; as for the fact you changed that field content, that is clear in comment #4.
My point was that you just need to change the status.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Status: Needs review » Needs work

This script may be used for non-commercial purposes only. For any
commercial purposes, please contact the author at
anant.garg@inscripts.com

I think that is contrary to GPL license, to which any project committed in Drupal.org CVS is subject.

The control structures are sometimes not indented as the coding standards suggest.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Status: Needs work » Closed (won't fix)

There have not been replies from the OP in the past 7 days. I am marking this report as won't fix.