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:

<?php
#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

Comments

filster’s picture

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’s picture

thankyou filster :]

enorp’s picture

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

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

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.

I will soon be leaving the Drupal forums permanently. To understand why, please see this thread.

grobemo’s picture

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

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

I will soon be leaving the Drupal forums permanently. To understand why, please see this thread.

nevets’s picture

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’s picture

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