Various fixes and improvements.

Components/PathFunc.pas

@@ -612,8 +612,7 @@ function ValidateAndCombinePath(const ADestDir, AFilename: String;
   Returns True if all security checks pass, with the combination of ADestDir
   and AFilename in AResultingPath.
   ADestDir is assumed to be normalized already and have a trailing backslash.
-  AFilename may be a file or directory name.
-  If ADestDir is empty then only AFilename is checked. }
+  AFilename may be a file or directory name. }
 begin
   { - Don't allow empty names
     - Don't allow forward slashes or repeated slashes
@@ -626,23 +625,20 @@ begin
      not PathIsRooted(AFilename) and
      not PathCharIsSlash(AFilename[High(AFilename)]) and
      not PathHasInvalidCharacters(AFilename, False) then begin
-    if ADestDir <> '' then begin
-      { Our validity checks passed. Now pass the combined path to PathExpand
-        (GetFullPathName) to see if it thinks the path needs normalization.
-        If the returned path isn't exactly what was passed in, then consider
-        the name invalid.
-        One way that can happen is if the path ends in an MS-DOS device name:
-        PathExpand('c:\path\NUL') returns '\\.\NUL'. Obviously we don't want
-        devices being opened, so that must be rejected. }
-      var CombinedPath := ADestDir + AFilename;
-      var TestExpandedPath: String;
-      if PathExpand(CombinedPath, TestExpandedPath) and
-         (CombinedPath = TestExpandedPath) then begin
-        AResultingPath := CombinedPath;
-        Result := True;
-      end;
-    end else
+    { Our validity checks passed. Now pass the combined path to PathExpand
+      (GetFullPathName) to see if it thinks the path needs normalization.
+      If the returned path isn't exactly what was passed in, then consider
+      the name invalid.
+      One way that can happen is if the path ends in an MS-DOS device name:
+      PathExpand('c:\path\NUL') returns '\\.\NUL'. Obviously we don't want
+      devices being opened, so that must be rejected. }
+    var CombinedPath := ADestDir + AFilename;
+    var TestExpandedPath: String;
+    if PathExpand(CombinedPath, TestExpandedPath) and
+       (CombinedPath = TestExpandedPath) then begin
+      AResultingPath := CombinedPath;
       Result := True;
+    end;
   end;
 end;
 

Projects/Src/Compression.SevenZipDLLDecoder.pas

@@ -15,10 +15,11 @@ unit Compression.SevenZipDLLDecoder;
 interface
 
 uses
-  Windows, Shared.FileClass, Shared.VerInfoFunc, Compression.SevenZipDecoder;
+  Windows, SysUtils, Shared.FileClass, Shared.VerInfoFunc, Compression.SevenZipDecoder;
 
 function SevenZipDLLInit(const SevenZipLibrary: HMODULE;
   [ref] const VersionNumbers: TFileVersionNumbers): Boolean;
+procedure SevenZipDLLDeInit;
 
 procedure ExtractArchiveRedir(const DisableFsRedir: Boolean;
   const ArchiveFilename, DestDir, Password: String; const FullPaths: Boolean;
@@ -27,9 +28,11 @@ procedure ExtractArchiveRedir(const DisableFsRedir: Boolean;
 { These functions work similar to Windows' FindFirstFile, FindNextFile, and
   FindClose with the exception that recursion is built-in and that the
   resulting FindFileData.cFilename contains not just a filename but also the
-  subdir }
+  subdir. Also, ArchiveFindFirstFileRedir throws an exception for most errors:
+  INVALID_HANDLE_VALUE is only used if the archive is ok but no suitable file
+  was found. }
 type
-  TArchiveFindHandle = type Cardinal;
+  TArchiveFindHandle = type NativeUInt;
   TOnExtractToHandleProgress = procedure(Bytes: Cardinal);
 function ArchiveFindFirstFileRedir(const DisableFsRedir: Boolean;
   const ArchiveFilename, DestDir, Password: String;
@@ -46,11 +49,12 @@ type
     function HasTime: Boolean;
   end;
 
+  ESevenZipError = class(Exception);
+
 implementation
 
 uses
-  Classes, SysUtils, Forms, Variants,
-  ActiveX, ComObj, Generics.Collections,
+  Classes, Forms, Variants, ActiveX, ComObj, Generics.Collections,
   Compression.SevenZipDLLDecoder.Interfaces, PathFunc,
   Shared.Int64Em, Shared.SetupMessageIDs, Shared.CommonFunc,
   SetupLdrAndSetup.Messages, SetupLdrAndSetup.RedirFunc,
@@ -192,7 +196,7 @@ procedure SevenZipError(const LogMessage, ExceptMessage: String);
   ExceptMessage should not. }
 begin
   LogFmt('ERROR: %s', [LogMessage]); { Just like 7zMain.c }
-  raise Exception.Create(ExceptMessage);
+  raise ESevenZipError.Create(ExceptMessage);
 end;
 
 procedure SevenZipWin32Error(const FunctionName: String; LastError: DWORD = 0); overload;
@@ -483,10 +487,11 @@ begin
     const Indices = E.GetIndices;
     const NIndices = Length(Indices);
     if NIndices > 0 then begin
-      var NumberOfItems: UInt32;
-      E.FInArchive.GetNumberOfItems(NumberOfItems);
        { From IArchive.h: indices must be sorted. Also: 7-Zip's code crashes if
          sent an invalid index. So we check them fully. }
+      var NumberOfItems: UInt32;
+      if E.FInArchive.GetNumberOfItems(NumberOfItems) <> S_OK then
+        InternalError('GetNumberOfItems failed');
       for var I := 0 to NIndices-1 do
         if (Indices[I] >= NumberOfItems) or ((I > 0) and (Indices[I-1] >= Indices[I])) then
           InternalError('NIndices invalid');
@@ -1047,24 +1052,26 @@ begin
     State.Password := Password;
     State.RecurseSubDirs := RecurseSubDirs;
 
-    for var currentIndex: UInt32 := 0 to State.numItems-1 do begin
-      if State.GetInitialCurrentFindData(FindFileData) then begin
-        { Finish state }
-        State.currentIndex := currentIndex;
-
-        { Save state }
-        if ArchiveFindStates = nil then
-          ArchiveFindStates := TArchiveFindStates.Create;
-        ArchiveFindStates.Add(State);
-
-        { Warn about solid }
-        var Solid: Boolean;
-        if ExtractIntent and GetProperty(State.InArchive, $FFFF, kpidSolid, Solid) and Solid then
-          LogFmt('Archive %s is solid; extraction performance may degrade', [State.ExtractedArchiveName]);
-
-        { Finish find data & exit }
-        State.FinishCurrentFindData(FindFileData);
-        Exit(ArchiveFindStates.Count-1);
+    if State.numItems > 0 then begin
+      for var currentIndex: UInt32 := 0 to State.numItems-1 do begin
+        if State.GetInitialCurrentFindData(FindFileData) then begin
+          { Finish state }
+          State.currentIndex := currentIndex;
+
+          { Save state }
+          if ArchiveFindStates = nil then
+            ArchiveFindStates := TArchiveFindStates.Create;
+          ArchiveFindStates.Add(State);
+
+          { Warn about solid }
+          var Solid: Boolean;
+          if ExtractIntent and GetProperty(State.InArchive, $FFFF, kpidSolid, Solid) and Solid then
+            LogFmt('Archive %s is solid; extraction performance may degrade', [State.ExtractedArchiveName]);
+
+          { Finish find data & exit }
+          State.FinishCurrentFindData(FindFileData);
+          Exit(ArchiveFindStates.Count-1);
+        end;
       end;
     end;
     Result := INVALID_HANDLE_VALUE;
@@ -1147,12 +1154,12 @@ begin
   Result := (dwLowDateTime <> 0) or (dwHighDateTime <> 0);
 end;
 
-initialization
+{ SevenZipDLLDeInit }
 
-finalization
-  if (ArchiveFindStates <> nil) and
-     (ArchiveFindStates.Count > 0) then { Not allowed because it has references to 7-Zip which is probably already unloaded }
-    InternalError('ArchiveFindStates.Count > 0');
+procedure SevenZipDLLDeInit;
+begin
+  { ArchiveFindStates has references to 7-Zip so must be cleared before the DLL is unloaded }
   ArchiveFindStates.Free;
+end;
 
 end.

Projects/Src/Setup.MainFunc.pas

@@ -1862,22 +1862,22 @@ begin
           end;
         end
         else begin
-          try
-            { External file }
-            SourceWildcard := ExpandConst(CurFile^.SourceFilename);
-            Excludes.DelimitedText := CurFile^.Excludes;
-            if foExtractArchive in CurFile^.Options then begin
+          { External file }
+          SourceWildcard := ExpandConst(CurFile^.SourceFilename);
+          Excludes.DelimitedText := CurFile^.Excludes;
+          if foExtractArchive in CurFile^.Options then begin
+            try
               if not RecurseExternalArchiveFiles(DisableFsRedir, SourceWildcard,
                  CurFile^.ExtractArchivePassword, Excludes, CurFile) then
                 Exit(False);
-            end else begin
-              if not RecurseExternalFiles(DisableFsRedir, PathExtractPath(SourceWildcard), '',
-                 PathExtractName(SourceWildcard), IsWildcard(SourceWildcard), Excludes, CurFile) then
-                Exit(False);
+            except on E: ESevenZipError do
+              { Ignore archive errors for now, will show up with proper UI during
+                installation }
             end;
-          except
-            { Ignore expections, just like minimum disk space calculation by
-              InitializeSetup does }
+          end else begin
+            if not RecurseExternalFiles(DisableFsRedir, PathExtractPath(SourceWildcard), '',
+               PathExtractName(SourceWildcard), IsWildcard(SourceWildcard), Excludes, CurFile) then
+              Exit(False);
           end;
         end;
       end;
@@ -2823,7 +2823,8 @@ var
     Result.Lo := 0;
 
     var FindData: TWin32FindData;
-    var H := ArchiveFindFirstFileRedir(DisableFsRedir, ArchiveFilename, '',
+    var H := ArchiveFindFirstFileRedir(DisableFsRedir, ArchiveFilename,
+      AddBackslash(TempInstallDir), { DestDir isn't known yet, pass a placeholder }
       Password, RecurseSubDirs, False, FindData);
     if H <> INVALID_HANDLE_VALUE then begin
       try
@@ -3533,8 +3534,9 @@ begin
 	                LExcludes, foRecurseSubDirsExternal in Options);
 	            end;
 	          except
-	            { Ignore exceptions. One notable exception we want to ignore is
-	              the one about "app" not being initialized. Also see EnumFiles. }
+	            { Ignore exceptions. Two notable exceptions we want to ignore are
+	              the one about "app" not being initialized and also archive errors
+                (ESevenZipError). Also see EnumFiles. }
 	          end;
 	        end;
 	        if Components = '' then { no types or a file that doesn't belong to any component }
@@ -3630,8 +3632,10 @@ begin
   { Free the _isdecmp.dll and _is7z.dll handles }
   if DecompressorDLLHandle <> 0 then
     FreeLibrary(DecompressorDLLHandle);
-  if SevenZipDLLHandle <> 0 then
+  if SevenZipDLLHandle <> 0 then begin
+    SevenZipDLLDeInit;
     FreeLibrary(SevenZipDLLHandle);
+  end;
 
   { Free the shfolder.dll handle }
   UnloadSHFolderDLL;