Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

extraction fails for long path names #58

Closed
mattiase opened this issue Apr 8, 2016 · 22 comments
Closed

extraction fails for long path names #58

mattiase opened this issue Apr 8, 2016 · 22 comments
Assignees
Milestone

Comments

@mattiase
Copy link
Contributor

mattiase commented Apr 8, 2016

Extraction (lessmsi x) fails if the combined length of the destination directory and a file in the MSI exceeds the usual 260 character limit:

lessmsi x mypkg.msi destdir\
Extracting 'c:\somedir\mypkg.msi' to 'c:\somedir\destdir\'.
fopen error:2

Error: System.Exception: Error 'MSPACK_ERR_OPEN' extracting file to c:\somedir\destdir\SourceDir\PFiles\and\then\a\very\long\file\name.
   at LibMSPackN.MSCompressedFile.ExtractTo(String destinationFilename)
   at LessMsi.Msi.Wixtracts.ExtractFiles(FileInfo msi, DirectoryInfo outputDir, MsiFile[] filesToExtract, AsyncCallback progressCallback)
   at LessMsi.Cli.Program.DoExtraction(String msiFileName, String outDirName, List`1 filesToExtract)
   at LessMsi.Cli.ExtractCommand.Run(List`1 allArgs)
   at LessMsi.Cli.Program.Main(String[] args)

(The path names in the example above has been edited; pretend they are much longer.)

Despite using a relative path name for the extraction directory (destdir\), it is being converted to an absolute file name when the file is extracted. Permitting relative destination path names all the way might shorten them somewhat, but would do nothing about long paths in the archive file.

Using the Windows 'long' file name syntax, \\?\c:\somedir\destdir\, would solve the problem but does not work in lessmsi at all. Apparently, the .NET system libs don't like that syntax very much, although it works well on the Win32 API level. There are apparently some replacement libs that remedy this.

@activescott
Copy link
Owner

This error is coming directly from the low-level CAB APIs which is a native C lib calling Win32 and not .NET - so it isn't .NET causing these errors. That code is all checked in at https://github.com/activescott/libmspack4n if you want to investigate and provide a fix that works with the longer file names using whatever Win32 APIs work (while still validating file paths). As long as the code is clean and tests pass I'll be more than happy to accept PRs there.

@mattiase
Copy link
Contributor Author

Thank you. I did some experiments with libmspack4n in order to move the handling of long paths there, as you suggested, and it almost works: while the call to mspack_invoke_mscab_decompressor_extract handles long paths correctly, the same method (ExtractTo) then proceeds with some time-stamp and attribute modification in .NET code which I had to comment out. Converting those (File.SetCreationTime, File.SetLastWriteTime and File.SetAttributes) to win32 calls as well should do the trick, I suppose.

Half-finished patch attached to show what I mean; it's the first time I use either C# or Visual Studio, so please show some mercy. I could try finishing the patch with win32 calls for setting the file attributes and time-stamps if you agree that it is the correct solution.
libmspack4n-longfilename-unfinished.patch.txt

@activescott
Copy link
Owner

@mattiase Thanks! I have no concerns on the approach. As long as tests pass, I'll pull it.

@mattiase
Copy link
Contributor Author

Good, I shall have a patch finished by tomorrow. Right now the tests don't work outside your timezone; something needs to be done about that. The timestamps in .CAB files lack any timezone information and are treated as local times when files are extracted. However, the metadata reference files for the tests contain dates with specified timezones, making them absolute points in time. As a result, these tests always fail when I run them here.

The simplest fix is to remove the timezone information from all .csv files under TestFiles/ExpectedOutput: just delete the "-08:00" or "-07:00" suffix of each time stamp (usually two per line). The test will then treat these timestamps as local times, and the test succeeds no matter where on Earth they are run.

I could send you a patch for this if you like, but it's just a simple mechanical change, and the generation code should be updated accordingly as well so that they will be generated without the timezone part next time.

@activescott
Copy link
Owner

thanks. love the idea of removing timezone suffix from generation code and test files!
Looking forward to the PRs!.

@mattiase
Copy link
Contributor Author

The pull requests are there, one for libmspack4n and one for lessmsi.

@activescott
Copy link
Owner

Thanks for the PRs! I'll get to these as soon as I can. Probably this weekend. Thanks again!

On Apr 19, 2016, at 01:17, mattiase notifications@github.com wrote:

The pull requests are there, one for libmspack4n and one for lessmsi.


You are receiving this because you commented.
Reply to this email directly or view it on GitHub

@activescott
Copy link
Owner

@mattiase thanks again for the PRs. Can you point me to a msi that actually exhibits this problem so that I can double-check it before creating a release?

@mattiase
Copy link
Contributor Author

You can use any MSI that you have, as long as you unpack it into a directory long enough so that the combined length of the base directory and at least one of the files in the MSI exceeds 260 characters.

For instance, the longest file name inside python-2.7.3.msi file in your tree is SourceDir/Windows/winsxs/x86_microsoft.vc90.crt_1fc8b3b9a1e18e3b_9.0.21022.8_none_bcb86ed6ac711f91/x86_Microsoft.VC90.CRT_1fc8b3b9a1e18e3b_9.0.21022.8_x-ww_d08d0375.manifest at 173 characters. If you try unpacking that MSI into a directory that is at least 260-173 = 87 characters long, then it will fail without the fix, but succeed with the fix.

Try, for instance, the directory c:\my\own\very\very\very\unusually\long\directory\name\with\cream\sugar\and\chocolate\topping (I just did!) and you will see.

@activescott
Copy link
Owner

Doh, guess I should have thought that through, but thanks for the specific example. I gave this a shot and even with the libmspack4n updates, still encounter a problem. Seems that GetTargetDirectory in Wixtracts (LessMsi.Core) is trying to use System.IO to identify the target directory and it ran into problems. I'm working on fixes for that, but before I continue, I want to make sure that I'm not missing anything. Don't you encounter the same thing? I'm using what is currently in master.

P.S. I found it kind of funny that almost nothing works with those paths. Even cmd.exe, total commander, and more all had weird failures. Good thing I have cygwin and bash installed which work fine with the long windows paths 😜

@mattiase
Copy link
Contributor Author

It does work for me in the extract-all-to-directory form, lessmsi x package.msi destdir\, and that's what we primarily use lessmsi for. Are you selecting a specific directory to extract?

Yes, those "extended" paths aren't recognised much even by Microsoft's own tools, but often they are the only way out. I mostly use cygwin too.

@mattiase
Copy link
Contributor Author

mattiase commented May 3, 2016

It is true that if the combined length of the destination root directory and the longest directory of any file being extracted exceeds 260 characters, then extraction still fails (in GetTargetDirectory, as you observe). That was not a problem I had, but someone else might; in that case, a few more operations need to be replaced with win32 calls: Directory.CreateDirectory in GetTargetDirectory, and a few calls to File.Exists and Directory.Exists. In addition, some Path.Combine operations need to be replaced with plain string concatenations.

I have no immediate plan to do this right now. Is it holding up a release for you?

@activescott
Copy link
Owner

@mattiase sorry for my delay on this. Thanks for confirming that I'm not missing anything. I have a local branch here where I'm working on this. I'm swamped right now with a combination of work and personal activities, but I hope to get back to this and wrap it up over the next week or two.

@activescott
Copy link
Owner

@mattiase could you please try the branch at https://github.com/activescott/lessmsi/tree/tested_support_for_long_paths and see if it works for you and your needs. I tried to add support throughout. If you run into something not working I'd love to have a test added.

@activescott activescott reopened this May 10, 2016
@mattiase
Copy link
Contributor Author

That branch (tested_support_for_long_paths) works for my purposes (absolute path of longest destination file exceeds 260 characters but not its parent directory), but it already worked after my submitted fix that you commited.

However, even in this branch, I wasn't able to extract python-2.7.3.msi to a directory 166 characters long; the call to Directory.CreateDirectory in LessMsi.Msi.Wixtracts.GetTargetDirectory fails.

activescott added a commit that referenced this issue May 16, 2016
@activescott
Copy link
Owner

Okay the branch at https://github.com/activescott/lessmsi/tree/tested_support_for_long_paths contains a longer output path which revealed more issues throughout the code and this latest commit now has a fix for them.

@mattiase I will be grateful if you can give that code a quick build and test to make sure it's all working for you now.

@mattiase
Copy link
Contributor Author

Thank you! It seems to work as far as I have tested it, but I haven't tried it on UNC paths, and I wonder if the conversion to extended path form is done correctly. Unless I'm mistaken, the long form of \\server\thing\stuff is not anything like \\?\\\server\thing\stuff but literally \\?\UNC\server\thing\stuff, as implemented in MSCompressedFile.LongFilename.

@mattiase
Copy link
Contributor Author

I tried extracting to an UNC path, and it does not work at all (even for short paths) because of the reason described in my previous comment.

@activescott
Copy link
Owner

Thanks for the feedback. Its on my list

@activescott
Copy link
Owner

activescott commented May 30, 2016

Only thing left is:

@activescott activescott self-assigned this Aug 12, 2016
@activescott activescott added this to the vNext milestone Sep 8, 2016
@activescott activescott modified the milestones: vNext, v1.5 Sep 8, 2016
@activescott activescott modified the milestones: vNext, v1.5 Mar 12, 2017
@activescott
Copy link
Owner

I believe this has been resolved back with recent versions of LessIO. I'm not sure if any window application -including windows explorer- supports long paths as well as LessMSI now :)

Let me know if I'm missing something and I'll fix it.

@mattiase
Copy link
Contributor Author

Thank you – unfortunately I'm no longer in a position to verify its functioning in this respect, but I'll take your word for it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants