MyBB Community Forums

First of all, this has nothing to do with MyBB, that's why it's in the Chit-Chat section.

Now, I'm working on a messaging system and I'm having issues with archiving messages.

Basically, when a user clicks "archive" on a message in their inbox, it uses a function to change the db setting for that message to show that it's been archived.

Then, when the user clicks "renew" on a message in their archive, it will essentially reverse the former process.

The links use $_GET to pass the information needed for this:

<?php
if(isset($_GET['do']))
	$do = $_GET['do'];
if(isset($_GET['mid']))
	$mid = $_GET['mid'];
	
	
	
if($do = 'archive' and $mid != null) {
	archiveMessage($mid);
	header('Location: msg.php?success=archive');
}

if($do = 'renew' and $mid != null) {
	renewMessage($mid);
	header('Location: msg.php?act=archive&success=renew');
}
?>

My problem is that no matter what $do equals, the latter "If" statement is executed.

Here are the functions called above:

function archiveMessage($mid) {
	$mid = $mid;
	$sql = "UPDATE messages SET archived='1' WHERE mid='$mid'";
	mysql_query($sql);
}

function renewMessage($mid) {
	$mid = $mid;
	$sql = "UPDATE messages SET archived='0' WHERE mid='$mid'";
	mysql_query($sql);
}

I'm sure it's something small that I'm just not catching so I'd appreciate some extra eyes.

if($do = 'archive'
if($do = 'renew'

Should be

if($do == 'archive'
if($do == 'renew'

Otherwise (I think) it'll assign 'archive' or 'renew' to $do and the if statement return true because that assignment succeeded. In other words:

if($test = 'test')

This will assign 'test' to $test and the if statement will return true because it did that. Whereas this:

if($test == 'test')

Checks if $test is already equal to 'test'.

It's running both if statements, because they will both return true, it just looks like it's only doing the last one as the last function runs a query that negates the effect of the query in the first function.

Plus this is completely unnecessary:

$mid = $mid;

If the function is renewMessage($mid) then $mid would be used within the function, you don't need to assign it to itself.

Haha, I knew it would be something small. Thanks for catching that.

(2010-06-03, 10:30 PM)MattRogowski Wrote: [ -> ]Plus this is completely unnecessary:

$mid = $mid;

If the function is renewMessage($mid) then $mid would be used within the function, you don't need to assign it to itself.

I was having some trouble with that before and assigning it to itself seemed to work. I've just been doing that to play it safe.

Your leaving yourself open to attacks with that code.

In the functions replace:

$mid = $mid;

With:

$mid = intval($mid);

This isn't the final code. I'm well aware that what i have now isn't secure.

I've got my answer, thanks.