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

FilesToExtract Count Can Be Incorrect When Installer Contains Multiple CAB Files #119

Open
hexxellor opened this issue Dec 7, 2018 · 4 comments

Comments

@hexxellor
Copy link

In the code a new ExtractionProgress object is created in Wixtracts.cs like this:
progress = new ExtractionProgress(progressCallback, filesToExtract.Length);

However, this assumes it should set the progress bar maximum value to "filesToExtract.Length"

The problem with this is if the MSI contains multiple CAB files with varying amounts of files in each (some being duplicates), the total can be incorrect.

The correct total can actually be obtained later on in the same source code where it's parsing/merging the CAB files. Something similar to this would work:

public static void ExtractFiles(Path msi, string outputDir, MsiFile[] filesToExtract, AsyncCallback progressCallback)
{
'REMOVED ORIGINAL CODE FOR CLARITY
                int realTotalFilesToExtract = 0;
	        try
                {
	            foreach (MSCabinet decompressor in cabDecompressors)
	            {
	                realTotalFilesToExtract += decompressor.GetFiles().Count();
	            }
                }
                '...
'REMOVED FOLLOWING CODE FOR CLARITY
}

I'm not suggesting you use this exact code. Besides being pretty crude it's also obvious at this point in the code the ExtractionProgress object has already been created and the total number of files set (incorrectly). So this chunk of code would need to be reworked to fix this.

An example MSI with this issue is here:
https://github.com/hexxellor/TemporaryJunk/raw/master/LessMSI%20Issue/Example%20MSI%20With%20Multiple%20CABs.zip

If you need this file, please grab it and keep a copy right away. I can't promise I'll be hosting it for more a month (after January 7th, 2019).

Thanks.

@activescott
Copy link
Owner

@activescott
Copy link
Owner

Thanks for the report! I really appreciate it. I'd love it if you wanted to submit a PR. Let me know if you plan to or not.

@activescott activescott added this to the vNext milestone Dec 9, 2018
@activescott activescott removed this from the vNext milestone Dec 9, 2018
@hexxellor
Copy link
Author

Thanks for hosting the file so I can remove mine. I will take a look at it if I can get the time, but from what I recall the way it was written will require some big changes so it just comes down to how long it'll take. If I can find an elegant quick fix I'll submit a PR.

@miles192
Copy link

I also see this issue in our terrible installer files.
Quick fix is to limit the progress value to the max value. It gets to 100% too early but doesn't crash out.

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

No branches or pull requests

3 participants