Add error reporting when extracting files

.gitmodules

@@ -1,3 +1,4 @@
 [submodule "third_party/p7zip"]
 	path = third_party/p7zip
-	url = https://github.com/jinfeihan57/p7zip.git
+	url = https://github.com/p7zip-project/p7zip
+	branch = p7zip17

CMakeLists.txt

@@ -39,7 +39,7 @@ SET(P7ZIP_INCLUDE_PATH   "${P7ZIP_SOURCE_DIR}"
 
 
 if ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "GNU")
-  set(warnings -Wall -Wextra -Werror -Wno-unused-parameter)
+  set(warnings -Wall -Wextra -Werror -Wno-unused-parameter -Wno-misleading-indentation)
   set(cxx_warnings -Wno-class-memaccess)
   set(no_undefined -Wl,--no-undefined)
 elseif ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "Clang")

src/7ZipArchive.cpp

@@ -64,6 +64,8 @@ class CArchiveExtractCallback:
 	public CMyUnknownImp
 {
 public:
+	int lastError;
+
 	MY_UNKNOWN_IMP1(ICryptoGetTextPassword)
 
 	// IProgress
@@ -129,11 +131,14 @@ class C7ZipArchiveImpl : public virtual C7ZipArchive
 									  wstring & val) const;
 	virtual bool GetFileTimeProperty(lib7zip::PropertyIndexEnum propertyIndex,
 									 unsigned __int64 & val) const;
+
+	virtual wstring GetLastExtractError() const;
 private:
 	CMyComPtr<IInArchive> m_pInArchive;
 	C7ZipObjectPtrArray m_ArchiveItems;
 	wstring m_Password;
     std::vector<CMyComPtr<IInArchive>> m_Archives;
+    int lastExtractError;
 };
 
 C7ZipArchiveImpl::C7ZipArchiveImpl(C7ZipLibrary * pLibrary, IInArchive * pInArchive,
@@ -204,7 +209,10 @@ bool C7ZipArchiveImpl::Extract(const C7ZipArchiveItem * pArchiveItem, C7ZipOutSt
 
 	UInt32 nArchiveIndex = pArchiveItem->GetArchiveIndex();
 
-	return m_pInArchive->Extract(&nArchiveIndex, 1, false, extractCallbackSpec) == S_OK;
+	bool success = m_pInArchive->Extract(&nArchiveIndex, 1, false, extractCallbackSpec) == S_OK;
+	lastExtractError = extractCallbackSpec->lastError;
+
+	return success;
 }
 
 void C7ZipArchiveImpl::Close()
@@ -228,6 +236,8 @@ bool C7ZipArchiveImpl::Initialize()
 		}
 	}
 
+	lastExtractError = NArchive::NExtract::NOperationResult::kOK;
+
 	return true;
 }
 
@@ -297,23 +307,22 @@ STDMETHODIMP CArchiveExtractCallback::PrepareOperation(Int32 askExtractMode)
 
 STDMETHODIMP CArchiveExtractCallback::SetOperationResult(Int32 operationResult)
 {
+	STDMETHODIMP operationSuccess;
+
+	lastError = operationResult;
+
 	switch(operationResult)
 	{
 	case NArchive::NExtract::NOperationResult::kOK:
+		operationSuccess = S_OK;
 		break;
 	default:
-		{
-			switch(operationResult)
-			{
-			default:
-				break;
-			}
-		}
+		operationSuccess = S_FALSE;
 	}
 
 	_outFileStream.Release();
 
-	return S_OK;
+	return operationSuccess;
 }
 
 
@@ -488,6 +497,34 @@ bool C7ZipArchiveImpl::GetFileTimeProperty(lib7zip::PropertyIndexEnum propertyIn
 	return false;
 }
 
+wstring C7ZipArchiveImpl::GetLastExtractError() const
+{
+	switch(lastExtractError) {
+	case NArchive::NExtract::NOperationResult::kOK:
+		return L"No error";
+	case NArchive::NExtract::NOperationResult::kUnsupportedMethod:
+		return L"Unsupported method";
+	case NArchive::NExtract::NOperationResult::kDataError:
+		return L"Data error";
+	case NArchive::NExtract::NOperationResult::kCRCError:
+		return L"Checksum error";
+	case NArchive::NExtract::NOperationResult::kUnavailable:
+		return L"Unavailable data";
+	case NArchive::NExtract::NOperationResult::kUnexpectedEnd:
+		return L"Data is truncated";
+	case NArchive::NExtract::NOperationResult::kDataAfterEnd:
+		return L"Extraneous data after end";
+	case NArchive::NExtract::NOperationResult::kIsNotArc:
+		return L"Not an archive";
+	case NArchive::NExtract::NOperationResult::kHeadersError:
+		return L"Header error";
+	case NArchive::NExtract::NOperationResult::kWrongPassword:
+		return L"Incorrect password";
+	default:
+		return L"Unknown error";
+	}
+}
+
 /*------------------- C7ZipArchive -----------*/
 C7ZipArchive::C7ZipArchive()
 {

src/7ZipOpenArchive.cpp

@@ -119,7 +119,8 @@ static int CreateInArchive(pU7ZipFunctions pFunctions,
               continue; //unable to read signature
 
           if (dmg_archive) {
-              ReadStream(inStream, -0x200, FILE_END, signatureEnd);
+              /* Only accept it as a dmg archive if we can read the end signature */
+              dmg_archive &= ReadStream(inStream, -0x200, FILE_END, signatureEnd);
           }
 
           if (signature == pInfo->Signatures[i]
@@ -138,7 +139,8 @@ static int CreateInArchive(pU7ZipFunctions pFunctions,
         continue; //unable to read signature
 
       if (dmg_archive) {
-          ReadStream(inStream, -0x200, FILE_END, signatureEnd);
+          /* Only accept it as a dmg archive if we can read the end signature */
+          dmg_archive &= ReadStream(inStream, -0x200, FILE_END, signatureEnd);
       }
 
       if (signature == pInfo->m_StartSignature ||

src/lib7zip.h

@@ -176,6 +176,8 @@ class C7ZipArchive : public virtual C7ZipObject
 									  wstring & val) const = 0;
 	virtual bool GetFileTimeProperty(lib7zip::PropertyIndexEnum propertyIndex,
 									 unsigned __int64 & val) const = 0;
+
+	virtual wstring GetLastExtractError() const = 0;
 };
 
 class C7ZipLibrary

test/Test7Zip.cpp

@@ -304,7 +304,14 @@ int main(int argc, char * argv[])
 				if (i==0) {
 					//Or set password for each archive item
 					//pArchiveItem->SetArchiveItemPassword(L"test");
-					pArchive->Extract(pArchiveItem, &oStream);
+					if (!pArchive->Extract(pArchiveItem, &oStream)) {
+						wstring strVal = L"", error;
+						pArchiveItem->GetStringProperty(
+								lib7zip::kpidPath, strVal);
+						error = pArchive->GetLastExtractError();
+						wprintf(L"extract file %ls fail: %ls\n",
+							strVal.c_str(), error.c_str());
+					}
 				}
 			} //if
 		}//for

test/Test7Zip2.cpp

@@ -304,7 +304,14 @@ int main(int argc, char * argv[])
 				if (i==0) {
 					//Or set password for each archive item
 					//pArchiveItem->SetArchiveItemPassword(L"test");
-					pArchive->Extract(pArchiveItem, &oStream);
+					if (!pArchive->Extract(pArchiveItem, &oStream)) {
+						wstring strVal = L"", error;
+						pArchiveItem->GetStringProperty(
+								lib7zip::kpidPath, strVal);
+						error = pArchive->GetLastExtractError();
+						wprintf(L"extract file %ls fail: %ls\n",
+							strVal.c_str(), error.c_str());
+					}
 				}
 			} //if
 		}//for

test/Test7ZipDmg.cpp

@@ -303,7 +303,14 @@ int main(int argc, char * argv[])
 				if (i==0) {
 					//Or set password for each archive item
 					//pArchiveItem->SetArchiveItemPassword(L"test");
-					pArchive->Extract(pArchiveItem, &oStream);
+					if (!pArchive->Extract(pArchiveItem, &oStream)) {
+						wstring strVal = L"", error;
+						pArchiveItem->GetStringProperty(
+								lib7zip::kpidPath, strVal);
+						error = pArchive->GetLastExtractError();
+						wprintf(L"extract file %ls fail: %ls\n",
+							strVal.c_str(), error.c_str());
+					}
 				}
 			} //if
 		}//for

test/Test7ZipMulti.cpp

@@ -322,7 +322,14 @@ int main(int argc, char * argv[])
 				if (i==0) {
 					//Or set password for each archive item
 					//pArchiveItem->SetArchiveItemPassword(L"test");
-					pArchive->Extract(pArchiveItem, &oStream);
+					if (!pArchive->Extract(pArchiveItem, &oStream)) {
+						wstring strVal = L"", error;
+						pArchiveItem->GetStringProperty(
+								lib7zip::kpidPath, strVal);
+						error = pArchive->GetLastExtractError();
+						wprintf(L"extract file %ls fail: %ls\n",
+							strVal.c_str(), error.c_str());
+					}
 				}
 		}
 	}

test/Test7ZipRar5.cpp

@@ -304,7 +304,14 @@ int main(int argc, char * argv[])
 				if (i==0) {
 					//Or set password for each archive item
 					//pArchiveItem->SetArchiveItemPassword(L"test");
-					pArchive->Extract(pArchiveItem, &oStream);
+					if (!pArchive->Extract(pArchiveItem, &oStream)) {
+						wstring strVal = L"", error;
+						pArchiveItem->GetStringProperty(
+								lib7zip::kpidPath, strVal);
+						error = pArchive->GetLastExtractError();
+						wprintf(L"extract file %ls fail: %ls\n",
+							strVal.c_str(), error.c_str());
+					}
 				}
 			} //if
 		}//for

test/Test7ZipSignature.cpp

@@ -304,7 +304,14 @@ int main(int argc, char * argv[])
 				if (i==0) {
 					//Or set password for each archive item
 					//pArchiveItem->SetArchiveItemPassword(L"test");
-					pArchive->Extract(pArchiveItem, &oStream);
+					if (!pArchive->Extract(pArchiveItem, &oStream)) {
+						wstring strVal = L"", error;
+						pArchiveItem->GetStringProperty(
+								lib7zip::kpidPath, strVal);
+						error = pArchive->GetLastExtractError();
+						wprintf(L"extract file %ls fail: %ls\n",
+							strVal.c_str(), error.c_str());
+					}
 				}
 			} //if
 		}//for

test/test_archive.cpp

@@ -304,7 +304,14 @@ int main(int argc, char * argv[])
 							val);
 				}
 
-                pArchive->Extract(pArchiveItem, &oStream);
+				if (!pArchive->Extract(pArchiveItem, &oStream)) {
+					wstring strVal = L"", error;
+					pArchiveItem->GetStringProperty(
+							lib7zip::kpidPath, strVal);
+					error = pArchive->GetLastExtractError();
+					wprintf(L"extract file %ls fail: %ls\n",
+						strVal.c_str(), error.c_str());
+				}
 			} //if
 		}//for
 	}

test_cases/README

@@ -0,0 +1,5 @@
+dm_g_uninitialized_read.gz: This is interpreted as a ZIP file after which
+reads from uninitialized memory happen (seemingly as part of an attempt to
+treat it as a DMG file?). This is a lib7zip problem; it happens with
+lib7zip's test_archive but not with p7zip's 7z or 7za. It's from
+dependabot-core-0.50.9.tar.gz.