Might I request the ability to upload multiple attachments at once, through a .zip or .tar.gz archive?

We're rolling out TWiki (customized solely through templates and plugins, woo!) for use with documenting a complex and frequently-changing software product, and most documents have lots of images and other attachments. It'd be nice if we could just zip up all the files, check off "multiple attachment archive," and only have to upload that one single file. The archive would be uncompressed and the next screen would show all the files inserted into the attachments table, where you could specify which files would be hidden, comments for each one, etc., and finally submit it and return to the document.

As it stands now, uploading them one at a time is a bit of a pain.

-- VitoMiliano - 30 Oct 2001

Well, no-one said anything, so we went ahead and did it ourselves. But lemme tell ya, TWiki's got some mighty annoying code in it. Like, what's with hitting up RCS every time you want to check a version? Why not just set a variable and pass that around? But, I digress...

Attached now is a replacement upload file for AthensRelease (but it also works fine with TWikiRelease01Sep2001), as well as a new default attach.tmpl. This allows you to, along with regular attachments, to upload attachment archives, which is a .zip file of files to attach.

• Upon uploading, the zip file is uncompressed, and all the files are attached as proper attachments, inserted into RCS and metadata and everything.
• Zip file comments for each file are preserved.
• The "hidden" checkbox is made global; if it's checked off, all the files are made hidden. If it's not, they're all visible.
• Same goes for the "link" checkbox. If it's checked off, all the files get linked to.
• The zip file itself is deleted when it's done with.
• If this is how most of your attachments are going to be dealt with, you can set a new TWiki variable, ARCHIVEFILE, to be checked, just as with other checkboxes.

The sample text from the attach.tmpl file looks like this:

The "note" section has been updated as well, with:

• Archive checkbox: If the uploaded file is a ZIP archive and you check the box, the archive will be decompressed and all files contained therein will be attached to the topic. Zipfile properties for each file will be maintained (i.e. file visibility and file comment).

And the section you'll want to add into TWiki.TWikiVariables or wherever looks something like:

• Default state of the archive check box in the attach file page. Checkbox is initially checked if Set ARCHIVEFILE = checked, or unchecked if empty (Set ARCHIVEFILE =). If checked, any archive files uploaded (i.e. files with the extension .zip) will be decompressed and all files contained therein will be attached to the topic:
  • Set ARCHIVEFILE = checked

Support for .tar and .tar.gz/.tgz archives was planned, but apparently Archive::Tar doesn't work when strict subs are in use. So it's just .zip files for now.

We've been hammering on this for a while now, and it seems pretty solid. I'd love it if this could become part of the standard codebase. What's the protocol for that? Do I have to assign copyright to PeterThoeny?

That's it. Any questions, lemme know.

-- VitoMiliano - 14 Jan 2002

Just a note on the way I made these modifications: When the archive checkbox is on, the files will be sent to the doUnzip sub to be processed. The sub returns a hash consisting of pairs as follows:

• "temporary filename" => "real filename|file comment"

As such, other uploaded-file-processing subs could easily be added as long as they return a hash with the same structure (i.e. if someone wrote a sub to, say, convert uploaded MS Word documents to plain text or something, they could be processed and handled by only adding a single sub). This will also make further archive handling simpler as well, since subs can easily be added for .tars, gzips, bzip2s, etc.

I'm pretty sure I marked the major changes to the existing code with my initials (RNF), so you can search for that if you want to see what I've done.

-- RyanFreebern - 14 Jan 2002

POSSIBLE SECURITY PROBLEM (not see below)

Doesn't this suffer from the same problem as ViewContentsofArchiveFileAttachment?

I just created a zip file with paths that use ../../etc/passwd and tried to unzip it from two levels below root. Sure enough unzip tried to overwrite /etc/passwd. Not so good 8-( . I coldn't overwrite the /etc/passwd file because I don't have permission, but just imagine how much damage I could cause by overwriting a perl CGI, or creating a .pl page to open a shell at a port on the server.

How do you handle such attachments?

I love the idea, but its the implementation details that make me nervous.

Also it looks like Archive::Any from CPAN has at least rudamentary support for security using is_naughty. The Archive::tar and Archive::Zip didn't seem to address it at all.

NAME
       Archive::Any - Single interface to deal with zips and
       tarballs

SYNOPSIS
         use Archive::Any;

         my $archive = Archive::Any->new($archive_file);

         my @files = $archive->files;

         $archive->extract;

         my $type = $archive->type;

         $archive->is_impolite;
         $archive->is_naughty;


DESCRIPTION
       This module is a single interface for manipulating
       different archive formats.  Tarballs, zip files, etc...

       Currently only tar (with or without gzip) and zip are
       supported.

       Currently only supports unpacking.

       new
             my $archive = Archive::Any->new($archive_file);
             my $archive = Archive::Any->new($archive_file, $type);

           Start working on the given file.

           $type is optional.  It lets you force the file type
           in-case Archive::Any can't figure it out.  'tar' or
           'zip'.

       files
             my @file = $archive->files;

           Filess the file in the archive.

       extract
             $archive->extract;
             $archive->extract($dir);

           Extracts the files in the archive to the given
           the current working directory.

       type
             my $type = $archive->type;
           Returns the type of archive this is.  'zip' or 'tar'.

       is_impolite
             my $is_impolite = $archive->is_impolite;

           Checks to see if this archive is going to unpack into
           the current directory rather than create it's own.

       is_naughty
             my $is_naughty = $archive->is_naughty;

           Checks to see if this archive is going to unpack
           outside the current directory.

-- JohnRouillard - 18 Jan 2002

hrm, that's definitely a problem. Except that the implementation is supposed to ignore stored paths entirely, since you can't have attachments in subdirectories. It's supposed to junk them. I do it here, and I just get "passwd" attached to the doc, making it not a security problem at all.

Maybe I uploaded an old version or something, I'll check. Thanks for pointing this out. I've moved this back to FeatureUnderConstruction until it gets sorted out.

-- VitoMiliano - 19 Jan 2002

The code to unzip archives strips paths from the archived filenames before it unzips them, because I knew that creating subdirectories would definitely cause some problems. As it is, if the zip contains both the files test.txt and subdir/test.txt, one of them will get overwritten by the other, but no directories will be created. I couldn't see how to handle subdirectories without compromising system security, so I just eliminated them for now.

I'm setting the classification back to FeatureDone, because the problem isn't a problem.

-- RyanFreebern - 19 Jan 2002

That works for me. However, I have to say that the Archive::tar code I saw did look like it allowed extraction of directories. You may want to attach the exact versions of Archive::Zip (and archive:tar) into this page since it's use requires them.

-- JohnRouillard - 19 Jan 2002

Also as stated above, this isn't needed either. Archive::Tar doesn't function in strict mode, and so is neither used by the plugin, nor required for it's use. There are two functions (both which should work correctly outside of strict mode, and shouldn't pose a security risk) in the replacement upload script that's provided below that were left in, in case a future version of Archive::Tar does work with TWiki, but they're not currently used.

I appreciate your attempts to help, but claiming security issues, even hedging it by saying "possible," without checking the plugin itself just hurts my attempts to help out here, and is perhaps even a bit irresponsible, to boot.

That said, we discovered an intermittent problem with the plugin the other day, where changing properties on an existing attachment (like the comment, or the hidden flag) wouldn't always work. The updated file below is the same version we're using on our live site, and doesn't appear to have either any security holes, or problems dealing with existing attachments.

-- VitoMiliano - 19 Jan 2002

Category: TWikiPatches

Also, I guess this is still underconstruction since I haven't written proper documentation for it.

-- VitoMiliano - 25 Feb 2002

Core Team: can you please take a look at the code and, if its ready, include it in TWikiAlpha. If it is not ready, please comment what needs to be done.

Vito: Thanks for this, it sounds like it is almost ready. If you can supply documentation for this then it can considered for inclusion in BeijingRelease. If not, it is prone to being pushed back to CairoRelease (Dec 02). (copied to email)

-- MartinCleaver - 13 Aug 2002

Wow, thanks for considering this for the next release. What kind of documentation is appropriate? An update to TWikiDocumentation? Anything else that should be expanded upon?

-- VitoMiliano - 13 Aug 2002

Sounds good to me but I'm not core team, just a contributor trying to squeeze another release. Core Team: what docs would be required?

-- MartinCleaver - 13 Aug 2002

I've made updates to the FileAttachment topic text that detail the changes made by this modification and attached the update here as a plain text file. If the core team integrates the mod, they can use this text (or some variation thereof) in the official documentation.

-- RyanFreebern - 16 Aug 2002

I have further updated the FileAttachment topic attached below to show the "archive file" check box in the graphics shown in this document....

Thanks to VitoMiliano for this nifty little enhancement which came in really handy just today...

It might be worth pointing out that one needs to install Archive::Zip from CPAN for this to work.

-- ThomasWeigert - 16 Aug 2002

I've updated CpanPerlModulesRequirement to reflect this. Please vote whether you think it is okay to use CPAN modules on that topic.

-- MartinCleaver - 20 Aug 2002

I think there should be a clearer error message when users are tyring to upload a zip file where there are directories in the zip file. What currently happens is that all files are extracted into a single directory, but then the upload fails when it is looking for the directories which are not created. For a user this is completely confusing as an error message. I think it would be wiser to either make it work (as the unzipping did already take care of unzipping things into a common directory) or to create a descriptive error saying that "upload of directory structures is not supported."

-- ThomasWeigert - 28 Aug 2002

This addon negatively affects the normal file upload in the following way: If you take a file that has spaces in the filename and try to upload (without the "archive flag" set), then the upload fails as the file name is not properly handled. Several files are attempted to be created. Note that this problem does not exist if the "archive flag" is set. The error messages that I get back in this situation (standard file upload) are attached.

-- ThomasWeigert - 28 Aug 2002

Regarding the first issue, it's supposed to ignore the directories entirely. The second issue is even more bizarre. Why is TWiki trying to parse the filename (the ? after WinZip in the filename)? Why isn't it automatically escaped by the browser when it's uploaded?

Thanks for the feedback. It probably won't get fixed for a while, depending on Ryan's schedule, but we'll get to it.

-- VitoMiliano - 29 Aug 2002

I agree with Thomas that there must minimally be a warning message. I'd say this needs to be in place before we could accept the changes into the core. CoreTeam: do you agree and would you be happy to accept the code into the core once that's in place?

-- MartinCleaver - 30 Aug 2002

I just installed this on our internal server and immediately encountered a problem. Specifically the routine bombs out on any attachments that have a space in the name. Granted I know its easy to make people use underscores or something, but with all the windows users of the world naming everything with spaces I think this one needs to get resolved before it is officially put in the the core.

-- JohnCavanaugh - 09 Sep 2002

It sounds like a few updates are needed to get this feature ready for CairoRelease. It sounds like a very useful feature if implemented well. I've moved the status to Cairo and changed the implementation and documentation percentage complete to 90%.

-- GrantBow - 22 Jan 2003

I've taken Vito's upload script and merged it with the BeijingRelease upload script, and added a few minor refinements.

The BeijingRelease seems to fix the problem of attachments with spaces in their file name - it strips the spaces - somebody must've made an executive decision during the Beijing development. Personally, I think replacing them with underscores would be more desirable - others seem to prefer conversion into a WikiWord-like filename - wonder why

Anyway, the same now also occurs for any files in the archive that have spaces in their names if you choose the "Decompress archived file..." option when you upload the archive.

I have not attempted to address the problem of archive subdirectory handling. The archive contents gets flattened to one level, and it's pot-luck which file you get if there are duplicate filenames in different subdirectories. For now, I recommend you upload flat-level archives if you wish to avoid problems. I may attempt to do something about this in the near future... Any thoughts on how this should be handled??

-- RonKellam - 20 Mar 2003

I think what is important is that the behavior is consistent across the mechanisms for upload. I would have preferred that the Beijing release had preserved the file names rather than "wikifying" them, but so be it...

Regarding RonKellam's question, as long as it is documented, flattening the zip archive is fine. The user issue this feature addresses is to make the upload of a large number of files convenient, which it does.

-- ThomasWeigert - 13 Apr 2003

We've actually updated this code for Beijing, and added several features, and fixed a few bugs.

Yes, the archive paths are intentionally ignored. Since you can't have subdirectories of topics in TWiki, and since it's only meant to allow easy uploading of multiple attachments, there's no point in keeping them around.

To deal with possible exploits (zip files with highly compressed, large files in them that use up all your disk space when uncompressed), and to help alleviate the filename compatibility issues (whether a zip archive or not), we've also added a few variables to TWikiPreferences:

• UPLOADZIPLIMIT sets an upper limit on the uncompressed size of an uploaded archive. If the variable is blank or zero, there is no limit on uploads. The value can be written as X, XK, and XM for X bytes, X * 1024 bytes, and X * 1024 * 1024 bytes, respectively. e.g: Set UPLOADZIPLIMIT = 15M
• UPLOADNOSPACES determines whether or not to strip spaces from uploaded filenames. If Set UPLOADNOSPACES = Yes spaces will be stripped, if blank or Set UPLOADNOSPACES = No spaces will be replaced by underscores.
• UPLOADLOWERCASE determines whether or not to lowercase uploaded filenames. If Set UPLOADLOWERCASE = Yes the filename will be lowercased, if blank or Set UPLOADLOWERCASE = No it will be left alone.

We just finished rolling out Beijing (a real chore due to many little tweaks we had made, unfortunately), so I'll see about getting the new code posted.

-- VitoMiliano - 14 Apr 2003

In addition, we solved a race condition when unzipping files and added three new oops'es:

• oopsduplicate.tmpl - Oops when duplicate filenames are found in the zip, either when files with the same name were in different directories in the zip or if duplicate filenames are created through the filename munging.
• oopstoobig.tmpl - Oops if the uncompressed size of the file exceeds UPLOADZIPLIMIT
• oopszip.tmpl - Oops if the zip file is corrupt or for some other reason the unzipping fails.

Attached is a zip with the new code.

-- ZacharyHamm - 14 Apr 2003

I've used this code for quite a while without problems with Apache 1.3.26 and perl 5.6.1 on a Debian/GNU Linux system. Now I had to move everything to a RedHat system with an Apache 2.0.47 compiled from sources and RedHat's perl 5.8.0. Unfortunately, things don't work for me any more

Perl kept complaining about a variable beeing tainted when extracting the zip attachment. Apparently, perl 5.8 has stricter (or broken?) taint checks than perl 5.6 and $foo=pattern; $foo=$1 will not untaint foo. I fixed this problem by adding an extra auxiliary variable in bin/upload (see upload.diff below).

Now, uploading zip files kind of works again. The zip file will be unpacked and the META information will be updated for all but the last member of the zip file. bin/upload hangs on the last TWiki::Store::saveTopic(). I haven't been able to figure out why yet.

-- JohannesMartin - 17 Nov 2003

Anybody got this working with the Jan2004 beta?

-- MattWilkie - 29 Jan 2004

Do others think that this should be rewritten to use the BeforeAttachmentSaveHandler? I think would allow a much cleaner implementation

-- MartinCleaver - 03 Apr 2004

I've merged the files in the previous zip'd version into those from the 20040320 beta and have attached a new distribution below. So far its working for me with Apache 1.3.29 & Perl 5.8.2.

-- DiabJerius - 09 Apr 2004

While this is a great idea, I'm afraid it's not suitable for the core code because it requires the addition of Archive::Zip to run, and one of the design goals TWiki is to have the core to have minimal CPAN requirements. However, I've just committed the BeforeAttachmentSaveHandler patch, which might make it possible to do this from inside a plugin. Another possible alternative is to write a version that dynamically detects whether Archive::Zip is present and simple disables this feature if not.

-- WalterMundt - 11 Apr 2004

I think that this would be much better as a plugin - it should be pretty straightforward now.

-- MartinCleaver - 12 Apr 2004

I note that Sven dropped this as a core feature from Cairo. If anyone cares enough I guess they'll turn it into a plugin using the BeforeAttachmentSaveHandler. (I hope they do).

-- MartinCleaver - 12 May 2004

Its not a plugin at the moment, and at it seems the TWikiTrain has lost momentum in this area as Vito does not use TWiki at his new work.

Looking at the plugin, it seems that it contains a bunch of oops - and of course that is something that plugins don't have. I see no reason why this should be though.

-- MartinCleaver - 26 Oct 2004

People - I've rebuilt this as a plugin. Still in testing. It will be ready soon. Let me know if you want early access.

-- MartinCleaver - 27 Oct 2004

I need a name for it. I don't like the name MultipleAttachmentsPlugin - it does not reveal its intention. Suggestions?

-- MartinCleaver - 27 Oct 2004

Something with FileUpload?

-- ArthurClemens - 28 Oct 2004

See BatchUploadPlugin

-- MartinCleaver - 31 Oct 2004