PHP function problem

Question

My PHP function for sidebar element generation looks like that

function makeSidebarEl ($side, $name, $lang, $db)
{
    $title='title_'.$lang;
    $txt='txt_'.$lang;
    $query=$db->query("SELECT $title, $txt FROM sidebar WHERE side='$side' AND name='$name'");
    $result=$query->fetch_array(MYSQLI_BOTH);
    $title=makeTitle($result[$title], $lang, $db);
    $txt=makeTxt($result[$txt], $lang, $db);
    echo '<div class="nav">'.$title.$txt.'</div>'."\n";
    }

But i'm getting result something like this

<div class="nav"></div>
<div class="nav_1"><img border="0" src=core/design/img/left_nav.png alt="" height="25px"/></div>
...

I mean it opens and closes <div class="nav"></div> at the begining of the every element but in fact function must echo result within this div:

echo '<div class="nav">'.$title.$text.'</div>'."\n"; How to fix that problem?

UPDATE

function makeTitle($title) {

    echo '<div class="nav_1"><img border="0" src=core/design/img/left_nav.png alt="" height="25px"/></div>
    <div class="nav_2">'.$title.'</div>
    <div class="nav_3"><img border="0" src=core/design/img/left_nav.png alt="" height="25px"/></div>
    <div style="clear:both;"></div>';

    }

function makeTxt($txt) {
    echo '<div id="parts" class="parts_txt">'.$txt.'</div>';
}

Dynamic table names, bad bad idea. This is an SQL-injection nightmare and no amount of escaping is going to help you. See this question: http://stackoverflow.com/questions/5811834/how-to-prevent-sql-injection-with-dynamic-tablenames — Johan, Sep 21 '11 at 09:31
Can you show us your `makeTitle()` and `makeTxt()` functions? Also, replace `$text` with `$txt` at the last line. — Stanislav Shabalin, Sep 21 '11 at 09:34
Lol I happen to know that high rep guy from that question %) — Your Common Sense, Sep 21 '11 at 09:34
@Johan first i wanna get it work, then i'll think about security — Tural Ali, Sep 21 '11 at 09:44
@TuralTeyyuboglu, that's the wrong order of doing things. After it works, you will forget about the security. Security is not something you can bold on later. It needs to be included from the start. — Johan, Sep 21 '11 at 09:46
well, what do you suggest to do? I can't find different way to realize my idea — Tural Ali, Sep 21 '11 at 09:48
@TuralTeyyuboglu, keep the idea, but check the input against a whitelist of allowed tablenames, but like in the linked question. It's really not complicated code, just an extra test against a pre-defined array. See the answers to the linked question for example code. — Johan, Sep 21 '11 at 10:09

score 2 · Answer 1 · answered Sep 21 '11 at 09:51

2

You need to replace echo to return at functions makeTitle and makeTxt

answered Sep 21 '11 at 09:51

antyrat

27,479
9
75
76

score 2 · Accepted Answer · answered Sep 21 '11 at 09:51

makeTitle() and makeText() should return strings rather than echo them.

As it is now, they echo text when they are called and return NULL, which is then concatenated into string when you call echo '<div class="nav">'.$title.$txt.'</div>'."\n";, i.e., that line evaluates to echo '<div class="nav">'.NULL.NULL.'</div>'."\n";

PHP function problem

2 Answers2