Tags:
create new tag
view all tags

Bug: Rename topic does 'Main.Main.UserName' for attachments

Moving a topic from the Main web to a different web changes user in file attachment table from 'Main.UserName' to 'Main.Main.UserName'.

Test case

  • Create a topic in the Main web
  • Attach file to it
  • Move topic to Test web. In this case the 'ClickHere' topic in the Main web.
    • Note: Not sure if relevant, first attempt failed because topic of that name already existed in the Test web. So, retried with new name 'ClickHereToo'.
  • User name in File attachment table shows 'Main.Main.UserName'.
  • See example in Sandbox.ClickHereToo

Environment

TWiki version: 12 Jan 2002
TWiki plugins: N/A
Server OS: Linux
Web server: Apache
Perl version: N/A
Client OS: N/A
Web Browser: N/A

-- PeterThoeny - 22 Jan 2002

This bug still exists in BeijingRelease based code.

-- MichaelSparks - 20 Jul 2003

Fix record

interesting. when you goto http://twiki.org/cgi-bin/view/Sandbox/ClickHereToo?raw=debug you can see that the data file seems correct, so this might be a rendering issue - and when you look at the debug render with my newly added attachement you can see that there is another problem.

%META:FILEATTACHMENT{name="rasp1.gif" attr="" comment="Just testing a Funny Attachment." date="1011633154" path="C:\WINNT\Profiles\l-account\Desktop\rasp1.gif" size="5979" user="Main.YaldremMajeed" version="1.1"}% %META:FILEATTACHMENT{name="ascii_manwalk.gif" attr="" comment="Just testing a Funny Attachment." date="1011633595" path="C:\WINNT\Profiles\l-account\Desktop\ascii_manwalk.gif" size="14872" user="Main.YaldremMajeed" version="1.1"}% %META:FILEATTACHMENT{name="at.gif" attr="" comment="testing to find Codev/RenameTopicDoesMainMainUser" date="1089545430" path="at.gif" size="849" user="SvenDowideit" version="1.1"}% %META:TOPICMOVED{by="PeterThoeny" date="1011687439" from="Main.ClickHere" to="Sandbox.ClickHereToo"}%

I'd expect that the user's name would be the WIKIUSERNAME with web, but its not.

I'm thinking of fixing both

  1. the rendering issue to that it renders user="Main.YaldremMajeed" correctly
  2. and when attachments are added that user= contains the webname by default. that way it should also follow renames or the user correctly.

-- SvenDowideit - 11 Jul 2004

turns out that TWiki::userToWikiName just prepended $mainWebname. to the passed in username, even if it could not convert it from a username to a TWikiUserName. I've changed that so it only does that if the conversion works

this is now in SVN

-- SvenDowideit - 19 Jul 2004

I think this fix is incorrect. It changes the spec of userToWikiName. This function should add the web name unless told not to do so. Now it never does it for a user not in the list. This could introduce bugs elsewhere.

I think the real issue is the algorithm that fixes WikiNames if moved between webs. Assume you have a topic in the Main containing WikiWord links to other topics in the Main web. If you move that topic from the Main web to the Sandbox web, the rename function prefixes WikiWords with "Main." so that those links work after the rename. This should be done only for WikiNames that get linked, e.g. if preceeded by whitespace or parenthesis, or if in a double square bracket link. It looks like rename fixes the user name prefixed by quotes, which it should not do.

-- PeterThoeny - 19 Jul 2004

I made a change later today that improves bug compatibility, but at somestage we are going to have to bite the bullet and fix these sorts of ambiguitites.

userToWikiName should never be called for something that is not a valid user, and it is probably easier for long term maintainence to start thinking about real error mechanisms.

unfortuanatly the old way resulted in Main. being prepended to UserName, which should never happen as in this case it is not even a username, its a TWikiUserName.

so, I'm kinda voteing for lets see how much it breaks in the next week or so, and if we see anything major we can back it out then (especially seeing as both of us are heavy users of that code (I'm running a test version of my work wiki on SVN head for my testing)

on the moveing issue - yes, that needs looking at, but not now.

-- SvenDowideit - 19 Jul 2004

I just discovered an issue with this recent fix. The signature of new topics have just WikiName instead of Main.WikiName. Better to revert the change and to fix the rename to exclude "WikiNames" in quotes.

-- PeterThoeny - 20 Jul 2004

seeing as you didn't even bother to fix this in 2.5 years, you couldn't even be bothered to show me any respect by reporting what your issue is so i could have a look at doing somehting properly. I don't see any need to revert anything yet. the week of testing is not up.

-- SvenDowideit - 20 Jul 2004

Sven, I apologize for not being more verbose (went to bed at 2am last night and had a long day at work). This happens in my test installation where TWiki handles the user registrations, e.g. all users' login names are WikiWords. Here are the steps:

  • Create a new topic
  • The signature shows up as:
    -- PeterThoeny - 20 Jul 2004
  • Notice the missing "Main."

Related new bug: WebChanges shows user names pointing to the current web instead of the Main web.

-- PeterThoeny - 20 Jul 2004

interestingly, the old way used to show lots of "Main.svenagain\sven" on my test machine, which is just as broken. I'm playing with another idea, but talk about broken.

I just commited a more robust fix to SVN

-- SvenDowideit - 21 Jul 2004

I did some digging in the code.

I still think the recent fix is not correct since it changes the spec of TWiki::userToWikiName. This function should always return "Main.user" unless told not to do so. This is because not all users might be in the list (and in fact do not have to be in the list). Here is an esoteric case just to explain the current spec. Only users that have a

  • JohnSmith - jsmith - 12 Aug 2002
entry are in the list, but not a
  • McCammon - 13 Aug 2002
entry. That is, McCammon could be a valid login name and user name which does not need to be mapped, but it needs a "Main.McCammon" when being asked to link to the user's home page. Also, user mapping can be turned of alltogether by setting the $doMapUserToWikiName in TWiki.cfg, in which case the %userToWikiList hash is empty.

The current spec only supports users in the Main web. This could be changed if there is a need.

Since TWiki::userToWikiName is used in so many places I do not recommend to make this change shortly before the release.

The real cause of this issue is that rename fixes too many WikiWord links. Example, the rename function changes this here:

%META:FILEATTACHMENT{name="at.gif" attr="" comment="-" date="1090391460" path="C:\Data\Temp\at.gif" size="873" user="PeterThoeny" version="1.1"}%

to this here:

%META:FILEATTACHMENT{name="at.gif" attr="" comment="-" date="1090391460" path="C:\Data\Temp\at.gif" size="873" user="Main.PeterThoeny" version="1.1"}%

which is incorrect (and which gets faithfully rendered as Main.Main.PeterThoeny, as it should).

The rename function TWiki::Store::renameTopic calls TWiki::Store::changeRefTo, which fixes links like FooBar to Main.FooBar when moving a topic from the Main web to the Sandbox web. The changeRefTo function currently distinguishes between normal text and meta data; the prefix for meta data is set to:

   my $metaPreTopic = '"|[\s[,\(-]';

That is, WikiWords prefixed by quotes get fixed. This logic needs to be enhanced, it needs to be done conditionally for meta data. Quoted login names (that might be WikiWords) found in META:ATTACHMENT{ user="" ... } and META:TOPICMOVED{ by="" } must not be altered (new spec). Quoted WikiWords in other meta data need to be prefixed by the web name (current spec).

The following fix is tested (with userToWikiName reverted to its original state), it fixes WikiWords in meta data other then login names:

*** bu3/Store.pm        Sat Jul 10 23:42:59 2004
--- Store.pm    Thu Jul 22 01:43:40 2004
***************
*** 271,277 ****
--- 271,280 ----
             foreach my $topic ( @topics ) {
                if( $topic ne $oldTopic ) {
                    if( /^%META:/ ) {
+                       s/^(%META:FILEATTACHMENT.*? user\=\")(\w)/$1$TWiki::TranslationToken$2/;
+                       s/^(%META:TOPICMOVED.*? by\=\")(\w)/$1$TWiki::TranslationToken$2/;
                        s/($metaPreTopic)\Q$topic\E(?=$metaPostTopic)/$1$oldWeb.$topic/g;
+                       s/$TWiki::TranslationToken//;
                    } else {
                        s/($preTopic)\Q$topic\E(?=$postTopic)/$1$oldWeb.$topic/g;
                    }

-- PeterThoeny - 22 Jul 2004

the code currently commited is :

sub userToWikiName
{
    my( $loginUser, $dontAddWeb ) = @_;
    
    if( !$loginUser ) {
        return "";
    }

    $loginUser =~ s/$securityFilter//go;
    my $wUser = $userToWikiList{ $loginUser };
    if ( ! $wUser ) { #if we didn't find a user, just use the input param (NoPasswdUser, or de-registered)
      $wUser = $loginUser;
   }

    if( ! $dontAddWeb ) {
      my $web = "$mainWebname";
      ( $web, $wUser ) = TWiki::Store::normalizeWebTopicName( $web, $wUser );

      if ( isWikiName($wUser) ) {
            $wUser = "$web.$wUser";
      }
    }
   
   return $wUser;
}

as far as I can see (and my intention was) that a username of McCammon, being a valid WikiWord (isWikiName is badly named as it tests for WikiWord-ness) would get %MAINWEB% added to it. However, %MAINWEB% would not be added to non-WikiWords, and would not get prepended to a username that already has a Web specified. (which is what normalizeWebTopicName is for)

Also, I don't think renaming the username to include the web specifier in META:: is incorrect, I think its quite a bit safer than the alternative. (though its interesting that somethines its filled in with the username, and other times with the WIKIUSERNAME.

-- SvenDowideit - 22 Jul 2004

I am trying hard to understand your points and to find a synergy, but I must admit so far I could not. Could you please elaborate in more detail why you think the recent change works in all cases?

I am trying my best to explain. According to the current spec:

  1. Only login names ($userName, e.g. "jsmith") are persistently stored (modifier of last topic version, attachment uploader, person who renamed topic etc)
  2. They must never be mixed with TWiki's WikiNames ($wikiName, e.g. "JohnSmith") or ($wikiUserName, e.g. "Main.JohnSmith"), although $userName and $wikiName happen to be identical for sites that do not need to map the login name to WikiName like here on TWiki.org
  3. There are utilities to map between the names, they expect one type as input and must return another type

Now, because of 2., rename must be aware of login names, e.g. it must avoid prefixing the web name to login names (that happen to be WikiWords). That is the reason for my tested patch in Store.pm.

Because of 3., userToWikiName( $loginUser, $dontAddWeb ) should always return type $wikiName if $dontAddWeb is set, else it should return type $wikiUserName. This is no longer the case if I understand your fix correctly.

The spec for user handling/mapping can be changed if needed, but this needs to be designed carefully. In the interest CairoRelease timing I would prefer to stick with the current spec. This is to avoid the chance of introducing last minute bugs. The affected function userToWikiName() is used 39 times in the TWiki core code and is also exposed to all Plugins via TWiki::Func::userToWikiName()

-- PeterThoeny - 23 Jul 2004

I have not yet read & analyised your reply - I was toying with some test code for the changes I made and this is the initial result. Can you add any other test cases that you see as relevant to userToWikiName. I'll read your reply above after i've eaten and gotten over work smile

$user OLDuserToWikiName($user) NEWuserToWikiName($user)  
guest TWikiGuest TWikiGuest  
someguy Main.someguy someguy bug in OLD
someGuy Main.someGuy someGuy bug in OLD
SomeGuy SomeGuy SomeGuy  
ntdomain/username Main.ntdomain/username username bug in NEW? (bug in OLD too)
McCinnon McCinnon McCinnon  
UserName Main.Main.UserName UserName bug in OLD
Strange/Web.NameOfTopic Main.Strange/Web.NameOfTopic Strange/Web.NameOfTopic bug in OLD

  • testUser: test script - run from twiki/bin

  • Sven, if you want to verify this with testing you need to take more dimensions into consideration: ($doMapUserToWikiName on or off) x (.htpasswd auth or external auth with WikiNames or external auth with Unix login names) x (different forms of user names) -- PTh

-- SvenDowideit - 23 Jul 2004

  1. is not entriely correct. the UserTopicName is presistently stored in many deployments (those not using htpassword files, but rather using some of the session plugins to identify users)
    • That is what I meant by 2. All persistently stored names are $userName, they can be WikiNames in which case $userName and $wikiName will contain the same string. Again, all I/O should be done on $userName. -- PTh
  2. unfortuanatly they do get mixed
    • Not in the core TWiki (once the rename bug is fixed). I can't speak for the session Plugins. -- PTh
  3. if this were true, then i would expect this to be the case with userToWikiName, but as you correctly pointed out, this function deals with non-login names
    • No, not like this. Your confusion comes from the fact that the login name may contain a WikiWord string. userToWikiName takes the login name ($userName, %USERNAME%) as input, not $wikiName. It returns the WikiName ($wikiName, %WIKINAME%) if $dontAddWeb is set, else it returns the Web.WikiName ($wikiUserName, %WIKIUSERNAME%). The current fix violates that rule. -- PTh

I agree that in future we need to address the issue that rename has a number of problems, but due to time constraints i feel more comfortable makeing userToWikiName more robust, and defering the rename changes to when we have time to analise them more (there are at least 4 other rename issues that I was forced to defer already)

I think we should revisit the user mapping isuue later, as storing username as a persistent unique identifier is also not really true with the addition of DeleteUserAccount (work scheduled to be continued in Dakar - though I hope that MartinCleaver can help).

-- SvenDowideit - 24 Jul 2004

I'd prefer a designed approach to user handling which would include all these mappings (see PublishUsernameCanonicalizationThroughPluginsAPI to see what I mean) but in the absence of that, I'd make the following points:

  1. The current spec is consistent, in that the "login name" (whatever that is) is consistently the only thing stored in meta.
  2. If a twikiname gets stored in META then the bug is with the code doing the storing, not with the mapping mechanism.

At this late stage in Cairo we can't afford to pussy-foot around, so I'd stick with the current spec, and fix whatever it is that's storing Main.WikiName in the META.

  • i recon that fixing every place that is storing Main.WikiName in META, including mapped users, and all the old topic data that already contains this is impossible -- SvenDowideit

-- CrawfordCurrie - 24 Jul 2004

As mentioned before, the affected function userToWikiName() is used 39 times in the TWiki core code and is also exposed to all Plugins via TWiki::Func::userToWikiName(). In addition, it is exposed to TWikiApplications via SEARCH (FormattedSearch), REVINFO, . We can't afford to suddenly change the spec.

If there are no strong objections, and in the interest of everyones time, I will make the core code behave like the current spec.

-- PeterThoeny - 25 Jul 2004

if you like.. but this bug will need to then be defered to DakarRelease, as the userToWikiName change is the only one that will adress the topics that already have the WikiUserName in the METADATA. I still cannot see (from the table above) how the change could possibly have any bad ramifications, so in the interests of Dakar, can you add the test cases that you are worried about? (and assigne the bug to yourself while your at it.

-- SvenDowideit - 25 Jul 2004

I fixed it so that it behaves like spec, is now in SVN for Cairo.

Fixing broken usernames in existing METADATA is out of scope of CairoRelease.

-- PeterThoeny - 26 Jul 2004

Topic attachments
I Attachment History Action Size Date Who Comment
Unknown file formatext testUser r1 manage 2.8 K 2004-07-23 - 10:19 UnknownUser test script - run from twiki/bin
Edit | Attach | Watch | Print version | History: r22 < r21 < r20 < r19 < r18 | Backlinks | Raw View | Raw edit | More topic actions
Topic revision: r22 - 2004-07-26 - PeterThoeny
 
  • Learn about TWiki  
  • Download TWiki
This site is powered by the TWiki collaboration platform Powered by Perl Hosted by OICcam.com Ideas, requests, problems regarding TWiki? Send feedback. Ask community in the support forum.
Copyright © 1999-2024 by the contributing authors. All material on this collaboration platform is the property of the contributing authors.