A construction like

%SEARCH{search="
" format=""}%

or

%SEARCH{search="" format="
"}%

causes TWiki to enter an infinite loop.

SP

This was a systemic problem with newlines in parameter values.

SVN 8165

CC

SVN 8165 is code for a unit test, actual fix is SVN 8163.

I recorded 8165 because the fix wasn't complete until the test was in place.

-- PTh

Reading the code change, parameter values may have now embedded newlines. This might introduce new bugs because parameters are used in many places and not every parse is done with an 's' switch (such as in Plugins). It is safer to fail if any value of a parameter has a newline. That is,

%SEARCH{search="foo
bar" format="|$topic|"}%

%SEARCH{
  search="foo bar"
  format="|$topic|"
}%

AFAIK, the reason to allow multiple lines is readability. Parameter values do not need to span multiple lines for readability.

Please notice that one of last year's security issue was caused by not accounting for newlines in strings. We can't assume that all core code and Plugins parse parmameter values over multiple lines.

Therefore we need to play safe and disallow newlines for Dakar. If there is a need, Edinburgh could support parameter values with newlines.

-- PTh

Let's get real here. Last year's security issue (or at least one of them) was not caused by newlines in strings. It was caused by inadequate filtering of those strings when compiling command lines. The Sandbox filters strings now, so this should no longer be an issue. If you can show me by a test that it does cause a problem, there is a clear security issue and we need to fix the Sandbox.

Allowing multiple lines in parameters is indeed more readable, but that's not the only reason for it. You may want to pass a newline into a variable, for whatever reason. e.g. in a format string, many of which don't support the zany $n syntax. This lets you, and is a much clearer syntax as well.

Can you suggest a scenario in which a missing \s would cause a plugin to fail? AFAIK any plugins that have not been recoded to use registerTagHandler parse their own tags. In so doing they either use //s if they want many lines, or not. If not, they will not recognise a tag with a newline in a parameter; same behaviour as Cairo. Now, let's say they wanted multiple lines and put a /s on the RE. Now they have a string that they pass to extractParameters, and all of a sudden, newlines in the string will not cause the parse to fail. I can only see this as a good thing - unlikely as it is. The worst case scenario is that a tag which was previous ignored because it had a newline in it suddenly leaps into life.

IMHO this is a good thing.

CC

PTh, newlines in parameters is of great value, i.e. everywhere you find a format parameter. Parsing TML is far from trivial and by now we where simply not able to grok parsing newlines in parameters. If, by all means, you need to break parameter lines then use the GluePlugin, which btw works not only with dakar but also in beijing and cairo.

CC, the SEARCH and its security ... heh: a search string is not filtered by the sandbox as there's no way to specify a regexp input string type in a command template. Filtering is done the old way using some magic pattern somwhere in Search.pm (...afaik) which the sandbox blindly untaints. There's no security issue here but only an incompleteness.

MD

OK, it's closed then. My gut feeling says that this is a source of new yet more bugs. Time will tell.

-- PTh