Item1847: mailnotify doesn't expand BASEWEB properly

Behaviour

On a fresh install of Foswiki 1.0.6,

Running (on Debian Lenny):

cd /var/www/foswiki/bin
sudo -u www-data ../tools/mailnotify -news Sandbox

Where in Sandbox.WebNotify I have:

   * MyUserName: WebHome!

And in Sandbox.WebHome I have:

WEB: %WEB% BASEWEB: %BASEWEB% INCLUDINGWEB: %INCLUDINGWEB%

Which in the E-mail I receive expands as:

WEB: Sandbox BASEWEB: Main INCLUDINGWEB: Sandbox 

... where I expect BASEWEB to be Sandbox instead.

Bug

The problem seems to be a shared $twiki instantiation on line 74 of MailerContrib.pm (no parameters so session context defaults to %MAINWEB%). Webs are then processed in the loop down at line 89, all with the same $twiki instantiation, so %BASEWEB% expands as %MAINWEB% .

-- PaulHarvey - 23 Jul 2009

This is a problem that bites PublishPlugin as well. I just checked in a fix on trunk that makes sure it's fixed for future Foswiki versions, and Olivier is working on a fix for the existing code, based on what I did in the PublishPlugin.

-- CrawfordCurrie - 23 Jul 2009

I've coded a unit test for this, and it seems the temporary fix you did for PublishPlugin doesn't fix it. I still have an issue with BASETOPIC being MailNotify, but your fix makes my unit test pass

-- OlivierRaginel - 23 Jul 2009

I've attached a diff to MailerContrib.pm that ships with 1.0.6 which fixes this problem for me. However, I'm not a brilliant Perl coder and I haven't had time to thoroughly understand the things I'm touching here. In particular I'm dubious as to how command_line context key value is actually used anywhere.

At first glance it seems it would be useful if Foswiki.pm was modified to check if command_line=1 and set up webName and topicName (if true) via the %initialContext hash instead of bothering to derive the proper web/topic context from the CGI query object.

• MailerContrib.pm.diff: This patch fixes the issue for me; needs review.

-- PaulHarvey - 24 Jul 2009

That's really not what the initial context hash is for. There's a much simpler solution, which Olivier has coded.

-- CrawfordCurrie - 24 Jul 2009

The only issue I have left with my code is that it doesn't seem to expose any problem on the release branch. Only on trunk, and you fixed it.

I'll try to test it again manually (odd, because my unit test works well on trunk) to see...

-- OlivierRaginel - 24 Jul 2009

Ok, it seems the unit tests are broken on the release branch, and I can't really seem to fix that. As the release branch will anyway be dumped "soon", I don't want to invest too much time in this.

PaulHarvey, please wait for 1.1 to be released and it should fix your issue, so you shouldn't have to re-apply your patch (which is OK for you, it might not be OK for people with loads of webs).

-- OlivierRaginel - 27 Jul 2009