Kroc/NoNonsenseForum

Appends not checked for being double.

Closed this issue · 3 comments

As per “question” on the Camen Design Forum:

If you do want to create a huge thread you could append the same message to your post over and over again. It looks like text that is being appended isn’t checked either.

The main reason NNF is checking for double replies is because they ‘could be an accident’. This goes for appends as well. We should probably try to check for double appends.

This might be a tad more involved than expected. I checked the non-formatted texts like so:

        !(
            ($offset = strlen (
                $stripped = rtrim (strip_tags ($post->description))
            ) - strlen (TEXT)) > -1 &&
            substr_compare ($stripped, TEXT, $offset, strlen (TEXT)) === 0
        )

This was put at line 107, where the TODO note is.

This only works when there are no newlines. After strip_tags there will be 2 empty lines between paragraphs while most people will be trying to submit with only 1 line in between. This means they will bypass the check.

The only way to check for duplicates without missing loads of edge cases is by comparing the append with the original post after running formatText. But to keep titles unique we cannot run formatText separately. (Also, with unique titles, I can submit a double append and it will come up as different if the title in my previous append gained a _1 in its ID.)

The not-as-elegant-but-working solution would be to simply collapse all newlines:

        !(
            ($offset = strlen (
                $strippedPost = rtrim (preg_replace ("/(\r?\n)+/", "\n", strip_tags ($post->description)))
            ) - strlen (
                $strippedAppend = preg_replace ("/(\r?\n)+/", "\n", TEXT)
            )) > -1 &&
            substr_compare($strippedPost, $strippedAppend, $offset, strlen($strippedAppend)) === 0
        )

(Note that we need to check the offset before doing the substr_compare.)

Kroc commented

Ah, just fixed this issue just as your were posting :) The solution is to normalise the append in the same way the original text is treated -- (just format the append, and then immediately strip it) i.e.

if (//normalise the original post and the append, and check the end of the original for a match
    substr (strip_tags ($post->description), -strlen ($_ = strip_tags (formatText (TEXT)))) !== $_
)

Clever, that solution totally passed me by! Thanks for pushing these things through, I feel the new title system is a pretty central update on the v23.