Is this function good?

[Guest190829]

Here's my edit_comment function, is it good? It's coding in OOP, though I kind of lost my understanding for it, so I'm going to have to reread on it. That's why I'm posting this, and also if it meets 3.5 syntax standards correctly, and if it's secure. This way, I can edit all my mistakes in previous functions I wrote. Any comments are greatly appreciated.


function edit_comment() // edit comment
{

global $id, $vbulletin

$this->id = $vbulletin->input->clean_gpc('g', 'commentid', TYPE_UINT);

$getcomment = $vbulletin->db->query_first("SELECT comment_text, comment_title
FROM space_comments
WHERE comment_id = '" . $this->id "'
");

eval('$edit_comment .= "' . fetch_template('edit_comment') . '";');

if ($_REQUEST['do'] == 'submit')
{
$vbulletin->input->clean_array_gpc('p', array('title' => TYPE_STR, 'text' => TYPE_STR))

$this->title = $vbulletin->GPC['title'];
$this->text = $vbulletin->GPC['text'];

$add_edited_comment = $vbulletin->db->query_write("UPDATE space_comments
SET comment_title = '" . $db->escape_string($this->title) "',
comment_text = '" . $db->esacpe_string($this->text) "'
WHERE comment_id = '" $this->id "'
");

}
}

[Adrian Schneider]

I'd change the $_REQUEST['do'] to $_POST['do'] so someone can't manipulate the URL and have it submit.

[Guest190829]

Well I was also going to add permissions to that if statement, would the $_POST['do'] still be needed?

[Adrian Schneider]

It depends what triggers the function and any other security checks you have, but personally I would use $_POST for doing this.

[Andreas]

$_POST can be manipulates as easily as $_GET, so you won't gain much.

[AN-net]

i would go with post, at least its checked from which referrer:)

[Guest190829]

Okay, then I guess I will change it to post for added security. Is everything else okay, besides that?