Item1936: Search.pm assumes the {editby} field is a cUID and tries to getWikiName before it finds the real cUID

This is perhaps related to Item5773.

The problem is that Foswiki::Search::searchWeb calls _sortTopics at line 719 (1.0.6) which uses

$users->getWikiName( $topicInfo->{$topic}->{editby} );

to find the WikiName, but {editby} isn't necessarily a cUID.

Later, at Foswiki::Search::searchWeb contains code starting at line 796 to find the actual cUID no matter what it is given. However, this is done too late to solve the above issue.

            my $cUID = $users->getCanonicalUserID($ru);
            if ( !$cUID ) {

                # Not a login name or a wiki name. Is it a valid cUID?
                my $ln = $users->getLoginName($ru);
                $cUID = $ru if defined $ln && $ln ne 'unknown';
            }

The really cool part of this bug is that if your Mapping module's getWikiName returns a WikiName even though it wasn't given a cUID then it caches the WikiName as a cUID and everything that uses the mapping caches later gets totally hosed.

-- JoshuaCharlesCampbell - 17 Aug 2009

$topicInfo}->{$topic} is populated by

    my ( $revdate, $revuser, $revnum ) = $meta->getRevisionInfo();
    $info->{editby}   = $revuser || '';

$meta->getRevisionInfo() is poorly defined as returning the author but in fact returns wither the author field in the TOPICINFO or the user stored in the RCS history. The author field is populated with the user login name, but the RCS history uses the cUID.

Confirmed. Regraded from Normal to Urgent. trunk needs to be checked for this, especially in light of Item2080

-- CrawfordCurrie - 21 Sep 2009

I'm going to try to work this out today/tomorrow - first up is working a set of unit tests that show the root issue, then some ASSERTs to highlight it in dev mode, and then to move the code Joshua highlights..

beware of Item6000 and assorted other pain. (at least that has unit tests)

oh, mmm - there's more to this bug

once I've fixed this issue, we have another - and that changes the TestCaseAutoSearchOrder test - as that has been incorrectly displaying the non-existant user's editby - but even more interestingly, the sort itself does order by the meta value - but then we render 'UnknownUser'

I've commited what I think is a 'correct' fix, but i think this will break some of the careful hacks made to support non-mapper setups (like Kenneth's)

hopefully he will try it and tell me the result.

-- SvenDowideit - 19 Oct 2009

I applied the patch at work and all seems to work OK.

Our setup still uses WikiUsers to map user login to wikiname. We use ApacheLogin and no password manager and Apache configured for mod_ldap. But it is still TopicUsermapping that maps the login to WikiName. If the person is not registered they are simply known by their login.

All seems normal. Any specific place I should double check.

-- KennethLavrsen - 19 Oct 2009

mmm, thats already good news - after sleeping on it, I think that the problem is when you have non-registered users who make changes to topics - they will be listed in webchanges as UnknownUser

which i have a memory of being a reported bug we fixed via this hack :/

-- SvenDowideit - 19 Oct 2009

yes. WebChanges shows UnknownUser which is very bad for a site that needs the audit trail. It does not look good to see UnknownUser. QA guys and IT guys react negatively on that.

That needs to get fixed so we see the whatever login or CUID that could not be mapped.

Since Cairo the WebChanges has shown Main.username (e.g. Main.c12179). The Main is bullocks but we have learned to live with it. The important thing was that you can see who it was that did whatever.

This must be a problem that we can solve in the code that shows author. It can show the login everywhere else I have looked. Also in the history view, in attachment history etc. It is only in WebChanges (ie SEARCH) that we have the problem.

-- KennethLavrsen - 20 Oct 2009

There is a related problem mentioned here

-- GuruprasadIyer - 21 Oct 2009

Kenneth - excellent, thankyou for confirming it, thats exactly what I was expecting.

turns out it was well worth commiting and getting you to comment, as your history/attachment etc point got me to look sidewards at some other code, and I think I have a much better fix.

except it will take me a few days to commit - in the process I think I've found a memory leak, and a possible security vector - both of which I'll need to examine enough to either resolve, or to prove to myself that they're not exploitable :/

its been a good week for finding stuff

-- SvenDowideit - 21 Oct 2009

ok, the other things were taking too long to track down, so I've commited the initial fix to get some testing. More refinements are possible, but slightly more risky - so might be better left for trunk.

-- SvenDowideit - 24 Oct 2009

fix is in trunk and 1.0.8

-- SvenDowideit - 29 Oct 2009