views:

59

answers:

1

I was dealing with refactoring of my small web app. all night. Today when I started testing, first bug what I found was problem with system PHP function nl2br().

On my localhost I have PHP version 5.2.9 and as I see on PHP site from version 4.0.5 nl2br() is XHTML compliant.

Then I absolutely don't understand why does my nl2br() return <br> without second argument set to false instead of <br />.

Here is my method where I found this bug:

public function eliminateTags($msg) {
    $setBrakes = nl2br($msg);
    $decodeHTML = htmlspecialchars_decode($setBrakes);

    # Check PHP version
    if((int)version_compare(PHP_VERSION, '4.0.5') == 1) {
        $withoutTags = strip_tags($decodeHTML, '<br />');
    } else {
        $withoutTags = strip_tags($decodeHTML, '<br>');
    }

    return $withoutTags;
}
+4  A: 

I'm not sure I understand what you're trying to accomplish with this function. First of all you insert HTML breaks in every new line and then you strip all tags except the breaks.

Wouldn't it be more sensible to strip the tags first and then insert the HTML line breaks?

public function eliminateTags($msg) {
    $decodeHTML = htmlspecialchars_decode($msg);
    $withoutTags = strip_tags($decodeHTML);
    $setBreaks = nl2br($withoutTags);

    return $setBreaks;
}

Edit:

Apparently you are not using strip_tags() correctly. You need to ask PHP which tag to exclude, which is <br>, not <br />. Asking PHP to exclude<br /> is like asking to exclude, say, <p></p> which won't work.

Which in turn means you must not check for a PHP version - strip_tags() will work as is in all versions.

Anax
+1 for the observation
Eineki
not really an answer, though.
Artefacto
Okey, that worked out. But was could be the problem when I tried the other way around? I wanted to make method for eliminating unwanted tags. For exmaple what would happen if I wrote `<script>alert();</script>`, but using this method it will return `alret();`.
Eugene
@Eugene strip_tags has an "allowed tags" parameter.
Pekka
@Pekka yes and I used it, didn't I? And problem still came up.
Eugene
@Artefacto: Indeed I did not solve the problem. Instead I chose to eliminate it. Nevertheless I believe I found what was causing the bug as well.
Anax
@Anax yep, I guess you did. Now it works as I planed, by the way good tip about the recognition of the tag, since there is no mentioning of it.
Eugene
@Anax No, you didn't. The code path that uses `"<br />" as an argument is never executed.
Artefacto
@Anax Sorry, you're right, that's the code path that is executed, not the correct one. Still, I cannot reproduce the bug when "<br />" is used as an argument (it simply strips the `<br />` anyway).
Artefacto
@Anax your code is much better for what I was trying to accomplish. Thank you very much. Also thank you for the tip about difference in `<br>` and `<br />` as argument in function `strip_tags`.
Eugene