By fiasst-1 on

Hi, I can't get drupal_goto() to work in my custom module. I've tried using header('location: foo') with success but drupal_goto just breaks and returns a white screen..

I have triple checked my code but here it is anyway:

#return users usernode to theme
function getUsernode(){
	global $user;
	$uid = $user->uid;
	$node = db_query("SELECT * FROM {node} WHERE type = 'usernode' AND uid = $uid");
	$usernode = db_fetch_object($node);
	if ($usernode) return $usernode;
}

#take user to usernode or to edit usernode when URL is '/profile' or '/user/XXX'
if (arg(0) == "profile" && !arg(1) || arg(0) == "user" && !arg(1)){
	$usernode = getUsernode();
	if ($usernode){
		if ($usernode->created != $usernode->changed){#has usernode
			drupal_goto("node/$usernode->nid");
	} else {
			drupal_goto("node/$usernode->nid/edit");
		}
	}
}

Any idea's what this may be?

Many thanks,
Marc

⋅ Categories: Drupal 5.x

Comments

filster’s picture

filster commented

because $usernode is an object, you can't just call it's properties between ""

instead of drupal_goto("node/$usernode->nid"); u have to use drupal_goto("node/{$usernode->nid}");
same with drupal_goto("node/$usernode->nid/edit"); -> don't forget {}

btw:
db_query("SELECT * FROM {node} WHERE type = 'usernode' AND uid = $uid")
db_query("SELECT * FROM {node} WHERE type = 'usernode' AND uid = %d", $uid) -> safer (prevents mysql injection)

fiasst-1’s picture

fiasst-1 commented

thankyou filster :]

enorp’s picture

enorp commented

well it should be mentionned that SELECT * FROM is a bad practice, return only what you need... and about argument substitution, i don't understand how only doing that without using mysql_real_escape_string function would provide security

grobemo’s picture

grobemo commented

db_query sanitizes the arguments before inserting them into the query. (Well, really, _db_query_callback is where the arguments are sanitized, but you get the idea.)

Jaypan’s picture

Jaypan commented

db_query does it when using substitutions, not when doing it like he has (as far as I know). It should be written like this:

db_query("SELECT column1, colum2 FROM {node} WHERE type = 'usernode' AND uid = %d", $user->uid);

In this manner the code will be sanitized. Although in this situation, it's a system variable, not user inputted data, so it should be ok.

grobemo’s picture

grobemo commented

Right. Filster suggested the correct code in his first comment, but thanks for clarifying that. People reading the thread later might not notice. db_query only sanitizes variables that are passed via substitution for %s, %d, etc.

Jaypan’s picture

Jaypan commented

Ahh, I missed that Filster had put the correct code in there. My bad, but good to reinforce good coding practices.

nevets’s picture

nevets commented

Actually drupal_goto("node/$usernode->nid") should work since it unambiguous.

This code though looks questionable

#take user to usernode or to edit usernode when URL is '/profile' or '/user/XXX'
if (arg(0) == "profile" && !arg(1) || arg(0) == "user" && !arg(1)){
    $usernode = getUsernode();
    if ($usernode){
        if ($usernode->created != $usernode->changed){#has usernode
            drupal_goto("node/$usernode->nid");
    } else {
            drupal_goto("node/$usernode->nid/edit");
        }
    }
}

as it is outside any function and likely interferes with the normal operation of Drupal.

fiasst-1’s picture

fiasst-1 commented

thanks for your input! I'm learning a lot :]

nevets,

I can't see any problems with this code as It's only redirecting users to usernodes instead of account profiles. I think it's quiet common practice and seems to work well. I never did find out why I had trouble with drupal_goto() but I created a new custom module and pasted my code in there success so all's well..

Best,
Marc