Foswiki on GitHub is open for business! Next release meeting: Monday October 13, 1300Z

Item5967: viewfile sees dots in file names as a web separator

Priority: CurrentState: AppliesTo: Component: WaitingFor:
Urgent Closed Engine    
I'm under the impression that thing like http://twiki.org/cgi-bin/viewfile/Plugins/TreePlugin/TreePlugin_installer should work like http://twiki.org/p/pub/Plugins/TreePlugin/TreePlugin_installer does.

But all you get is oops "Attachment 'TreePlugin_installer' does not exist". I'm having the same problems on slion.net running 4.2.2 and I believe twiki.org has just been upgraded to 4.2.2.

-- TWiki:Main/StephaneLenclud - 28 Aug 2008

testing it here: -- TWiki:Main.PeterThoeny - 28 Aug 2008

The last form should work, according to what is stated at viewfile documentation. Confirmed.

The problem is that the dot within readme.txt is interpreted as a web/topic separator, so viewfile searches for an attachment named readme/txt, that doesn't exist. This is the same problem I discovered today when I tried to link CommandAndCGIScripts#=tick_twiki.pl= (at a 4.2.2 instalation).

-- TWiki:Main.GilmarSantosJr - 29 Aug 2008

I'd thought that last form should be working so that you could specify something like /bin/viewfile as your {PubUrlPath} and actually enforce access control on your attachments. Otherwise how would you make use of viewfile at all?

-- TWiki:Main.StephaneLenclud - 30 Aug 2008

TWiki:Main.GilmarSantosJr looks like you are right. I implemented a workaround in View.pm sub viewfile.

Testing on https://slion.net/viewfile/Sandbox/TestViewFile/uploadtest.txt and after:
my $topic = $session->{topicName};
my $webName = $session->{webName};

I was getting:
$webName==Sandbox/TestViewFile
$topic==Txt
which does not make sense at all so no wonder the store says that attachment does not exists.

To fix it, all I did was overriding $topic and $webName by doing:

$topic = pop( @path );
$webName = pop( @path );

I don't think that's a proper fix especially in respect to sub webs but it works for me. We must investigate why code like:
my $topic = $session->{topicName};
my $webName = $session->{webName};
returns broken values.

-- TWiki:Main.StephaneLenclud - 31 Aug 2008 edited on 3 Sep 2008

Can we get this bug item moving for 4.2.4 or do we defer it to 5.0?

-- TWiki:Main.KennethLavrsen - 18 Sep 2008

I guess I could submit my workaround unless someone disagree. It won't work for nested webs which are already not supported anyway put at least it should work for flat web hierarchy.

It looks like we are trying too hard to support nested web in the parsing of the URL.

-- TWiki:Main.StephaneLenclud - 19 Sep 2008

I'm ready to check-in that patch but I can't cause forbidden (403). I'm just a plug-in developer you know wink Kenneth can you grant me write access to http://svn.twiki.org/svn/twiki/branches/TWikiRelease04x02/ ?

-- TWiki:Main.StephaneLenclud - 21 Sep 2008

I took a fast look over this issue and it seems to me the problem is within TWiki::new(), when it initializes $topic->{webName} and $topic->{topicName} based on PATH_INFO: it uses a greedy regex that do consider hierarchical webs, but ignores the particular possibility of viewfile.

Unfortunately the fix is not trivial: how to know what is a web, topic, and file name if the separator is the same? e.g. /WebName/SubWeb/TopicName/attachment.txt

We could make it incrementally: (component separators: / and .)
  1. verify if WebName exists
  2. if hierarchical webs are enabled repeat 1 until the component doesn't exist as a subweb
  3. Verify if the next component exist as a topic within the (sub)web
  4. The remaining part (if exists) is the attachment
If I have the available time I'll test this solution within this week.

-- TWiki:Main.GilmarSantosJr - 22 Sep 2008

I agree with your analysis Gilmar:
  • It should be fixed in the initialisation of webName and topicName
  • Testing the web existence is the only way out since both . and / can be used as web and topic separator.
-- TWiki:Main.StephaneLenclud - 22 Sep 2008

This requires a proper fix; Stephane's proposed checkin will break hierarchical subwebs (which work, BTW).

-- TWiki:Main.CrawfordCurrie - 27 Oct 2008

I've created a 10 line fix for this in TWiki.pm that updates $topic and $web if the uri contains /viewfile/ and the last .*\..* is a valid file in the pubDir.

I've tested it so I'll check it in...but let me know if you disagree with this.

-- TWiki:Main.RobManson - 10 Nov 2008

ok, not only does this fix not work for simple attachments, but it's not going to work for a topics like TWiki.Graphics that contains lib/files/imgs/something.jpg

I'm working on writing enough unit tests to expose the actually desired outcome, and then will attempt a full fix.

Rob, at this point I cannot find a situation in which your fix works, can you please tell me an example? though it'll be hard unless you can make a change on your local checkout, trunk now also has my Item297 change.

ok, many tests added, and a working fix applied (reverted rob's change, though I may move my change to Foswiki.pm while looking into Item318

-- SvenDowideit - 27 Nov 2008

Core is broken by addition of call to functions in TWiki::Func, but TWikiCompatibilityPlugin is not installed. Also inappropriate untaint. Re-opened.

... and reclosed.

-- CrawfordCurrie - 28 Dec 2008

ItemTemplate edit

Summary viewfile sees dots in file names as a web separator
ReportedBy TWiki:Main.StephaneLenclud
Codebase trunk
SVN Range TWiki-5.0.0, Mon, 18 Aug 2008, build 17431
AppliesTo Engine
Component
Priority Urgent
CurrentState Closed
WaitingFor
Checkins distro:77e31b97d2ca distro:c55629280300 distro:102599913ab8 distro:dfa2c54ac61e distro:cc6c002b1524
TargetRelease patch
ReleasedIn 1.0.0
Topic attachments
I Attachment Action Size Date Who Comment
readme.txttxt readme.txt manage 4.5 K 28 Aug 2008 - 23:56 UnknownUser Test attachment again
Topic revision: r24 - 08 Jan 2009, KwangErnLiew
 
The copyright of the content on this website is held by the contributing authors, except where stated elsewhere. see CopyrightStatement. Creative Commons License