Item1468: Topics with regex quantifier characters such as + in their names prevent other topics from being moved
Priority: Low
Current State: Closed
Released In: 1.0.5
Target Release: patch
Applies To: Engine
Component:
Branches:
I have a web with a topic called
MyC++
It is a stupid topic name but it is one I managed to create when I did another test.
When I try to move another topic in or out of the web in which I have the
MyC++ topic the move succeeds partially.
The topic is moved but the follow up on changing links seems to fail.
The error is Nested quantifiers in regex; marked by <-- HERE in m/(((?<=[^./])|^)\\b(Myweb\\.)?MyC++ <-- HERE \\b|(?<=\\[\\[)(Myweb\\.)?[Mm]y *[Cc]++(?=\\][][]))/ at /var/www/Release01x00/core/lib/Foswiki/Render.pm line 2195., referer:
http://merlin.lavrsen.dk/foswiki10/bin/rename/Myweb/GetSaveTest2009x04x19x105054?currentwebonly=on
I think at some point we need to look at our different regexes that take variables with topic names and make sure we escape the regex characters.
I set it low priority but I raise the bug so we remember.
--
KennethLavrsen - 19 Apr 2009
Could not help fixing it.
It was easy to fix. And the fix should be safe as all it does is changing + to \+ in sub getReferenceRE
--
KennethLavrsen - 19 Apr 2009
I'm not sure where you go the idea that '+' is valid in topic names.
A valid topic name is defined by Foswiki::isValidTopicName by testing the regex that follows (simplified for clarity)
return ( $name =~ /^(([A-Z]+[a-z0-9]+[A-Z]+[a-z0-9]*)|([A-Z]{3,}s?\b))$/ );
As you can see a valid topic name does not allow '+'. Some character sets effectively redefine A-Z and a-z to support localised definitions of "upper case" and "lower case" but AFAIK none of those maps "+" to either an upper-case or a lower-case character.
Even if we
were to support '+' in topic names, then
$topic =~ quotemeta($topic);
would be the correct approach, as it protects all meta-characters that might break the RE. The web name should be treated similarly.
--
CrawfordCurrie - 20 Apr 2009
We may have a regex we call isValidTopicName but we do not use it.
I can create a + topic. Look
Item1468++
I'll change my fix to quotemeta
--
KennethLavrsen - 20 Apr 2009
I am fixing this as people will have plenty of topics with mysterious characters.
I am all for blocking for making new topics but we need to try and deal with the ones we have.
This is probably one of several fixes needed to do it. And this was was especially nasty because it prevents topic that are perfectly OK from being renamed. One thing is that a silly name topic causes trouble. But it should for sure not prevent other topics from being moved or renamed.
This fix stops that problem. Now fixed to quotemeta which also targets other characters than +
--
KennethLavrsen - 20 Apr 2009
Web name needs to be quoted as well (scope 1.1 to minimise risk to 1.0.5)
--
CrawfordCurrie - 21 Apr 2009
Yes I did think about it but exactly for the same reason I did not dare.
And the frequency people add webs is magnitudes lower than creating topics. And normally only admins add webs and they know that funny chars in webs will most likely cause trouble. I have not yet tested if you actually can create a funny web.
For the funny topic names I found out why I had the ++ topics. I had worked on confirming
Item5253.
So we have a one year old bug for this part of the problem.
I also think I know where the code fails. Maybe I fix that tonight.
--
KennethLavrsen - 22 Apr 2009