Zipper: now computeing crc32 on the fly during zip creation

src/libs/utils/impl/Path.cpp

@@ -26,17 +26,16 @@
 #include <array>
 #include <fstream>
 
-#include <boost/crc.hpp>  // for boost::crc_32_type
 #include <boost/tokenizer.hpp>
 
+#include "utils/Crc32Calculator.hpp"
 #include "utils/Exception.hpp"
 #include "utils/Logger.hpp"
 
 std::uint32_t
 computeCrc32(const std::filesystem::path& p)
 {
-	using crc_type = boost::crc_32_type;
+	Utils::Crc32Calculator crc32;
-	crc_type result;
 
 	std::ifstream ifs {p.string().c_str(), std::ios_base::binary};
 	if (ifs)
@@ -46,7 +45,7 @@ computeCrc32(const std::filesystem::path& p)
 			std::array<char,1024>	buffer;
 
 			ifs.read( buffer.data(), buffer.size() );
-			result.process_bytes( buffer.data(), ifs.gcount() );
+			crc32.processBytes( reinterpret_cast<const std::byte*>(buffer.data()), ifs.gcount() );
 		}
 		while (ifs);
 	}
@@ -56,8 +55,7 @@ computeCrc32(const std::filesystem::path& p)
 		throw LmsException("Failed to open file '" + p.string() + "'" );
 	}
 
-
+	return crc32.getResult();
-	return result.checksum();
 }
 
 bool

src/libs/utils/impl/Zipper.cpp

@@ -23,7 +23,6 @@
 #include <fstream>
 
 #include "utils/Path.hpp"
-#include "utils/Logger.hpp"
 
 // Done using specs from https://pkware.cachefly.net/webdocs/casestudies/APPNOTE.TXT
 
@@ -49,6 +48,8 @@ namespace Zip
 				NoCompression = 0,
 			};
 
+			static constexpr std::size_t UnknownCrc32 {0};
+			static constexpr std::size_t UnknownFileSize {0};
 
 
 		protected:
@@ -89,82 +90,34 @@ namespace Zip
 			using ZipHeader::ZipHeader;
 
 			// Setters
-			void setSignature();
+			void setSignature() { write32(0, 0x04034b50); }
-			void setVersionNeededToExtract(unsigned major, unsigned minor);
+			void setVersionNeededToExtract(unsigned major, unsigned minor) { assert(minor < 10); write16(4, major*10 + minor); }
-			void setGeneralPurposeFlags(std::uint16_t flags);
+			void setGeneralPurposeFlags(std::uint16_t flags) { write16(6, flags); }
-			void setCompressionMethod(CompressionMethod compressionMethod);
+			void setCompressionMethod(CompressionMethod compressionMethod) { write16(8, compressionMethod); }
-			void setLastModifiedDateTime();
+			void setLastModifiedDateTime()
-			void setCrc32UncompressedData(std::uint32_t crc);
+			{ // TODO
-			void setCompressedSize(std::size_t size);
+				write16(10, 0); // time
-			void setUncompressedSize(std::size_t size);
+				write16(12, 0); // date
-			void setFileNameLength(std::size_t size);
+			}
-			void setExtraFieldLength(std::size_t size);
+			void setCrc32UncompressedData(std::uint32_t crc) { write32(14, crc); }
+			void setCompressedSize(std::size_t size) { write32(18, size); }
+			void setUncompressedSize(std::size_t size) { write32(22, size); }
+			void setFileNameLength(std::size_t size) { write16(26, size); }
+			void setExtraFieldLength(std::size_t size) { write16(28, size); }
 			static constexpr std::size_t getHeaderSize() { return 30; }
 	};
 
-	void
+	class DataDescriptor : public ZipHeader
-	LocalFileHeader::setSignature()
 	{
-		write32(0, 0x04034b50);
+		public:
-	}
+			using ZipHeader::ZipHeader;
-
-	void
-	LocalFileHeader::setVersionNeededToExtract(unsigned major, unsigned minor)
-	{
-		assert(minor < 10);
-		write16(4, major*10 + minor);
-	}
-
-	void
-	LocalFileHeader::setGeneralPurposeFlags(std::uint16_t flags)
-	{
-		write16(6, flags);
-	}
-
-	void
-	LocalFileHeader::setCompressionMethod(CompressionMethod compressionMethod)
-	{
-		write16(8, compressionMethod);
-	}
-
-	void
-	LocalFileHeader::setLastModifiedDateTime()
-	{
-		// TODO
-		write16(10, 0); // time
-		write16(12, 0); // date
-	}
-
-	void
-	LocalFileHeader::setCrc32UncompressedData(std::uint32_t crc)
-	{
-		write32(14, crc);
-	}
-
-	void
-	LocalFileHeader::setCompressedSize(std::size_t size)
-	{
-		write32(18, size);
-	}
-
-	void
-	LocalFileHeader::setUncompressedSize(std::size_t size)
-	{
-		write32(22, size);
-	}
-
-	void
-	LocalFileHeader::setFileNameLength(std::size_t size)
-	{
-		write16(26, size);
-	}
-
-	void
-	LocalFileHeader::setExtraFieldLength(std::size_t size)
-	{
-		write16(28, size);
-	}
 
+			void setSignature() { write32(0, 0x08074b50 ); }
+			void setCrc32UncompressedData(std::uint32_t crc32) { write32(4, crc32); }
+			void setCompressedSize(std::size_t size) { write32(8, size); }
+			void setUncompressedSize(std::size_t size) { write32(12, size); }
+			static constexpr std::size_t getHeaderSize() { return 16; }
+	};
 
 	class CentralDirectoryHeader : public ZipHeader
 	{
@@ -210,8 +163,7 @@ namespace Zip
 			static constexpr std::size_t getHeaderSize() { return 22; }
 	};
 
-	Zipper::Zipper(const std::map<std::string, std::filesystem::path>& files, CompressionMethod compMethod)
+	Zipper::Zipper(const std::map<std::string, std::filesystem::path>& files)
-	: _compMethod {compMethod}
 	{
 		for (const auto& [filename, filePath] : files)
 		{
@@ -221,13 +173,7 @@ namespace Zip
 			std::error_code ec;
 			fileContext.fileSize = std::filesystem::file_size(filePath, ec);
 			if (ec)
-			{
+				throw ZipperException {"Cannot get file size for '" + filePath.string() + "': " + ec.message()};
-				LMS_LOG(UTILS, INFO) << "Cannot get file size for '" << filePath.string() << "': " << ec.message();
-				continue;
-			}
-			fileContext.fileCrc32 = computeCrc32(filePath);
-
-			LMS_LOG(UTILS, DEBUG) << "Processing '" << filePath.string() << "': File size = " << fileContext.fileSize;
 
 			_files[filename] = std::move(fileContext);
 		}
@@ -247,9 +193,6 @@ namespace Zip
 		{
 			std::size_t nbWrittenBytes {};
 
-			LMS_LOG(UTILS, DEBUG) << "Global offset = " << _currentZipOffset;
-			LMS_LOG(UTILS, DEBUG) << "Buffer ptr = " << buffer << ", remaining size = " << bufferSize;
-
 			switch (_writeState)
 			{
 				case WriteState::LocalFileHeader:
@@ -264,6 +207,10 @@ namespace Zip
 					nbWrittenBytes = writeFileData(buffer, bufferSize);
 					break;
 
+				case WriteState::DataDescriptor:
+					nbWrittenBytes = writeDataDescriptor(buffer, bufferSize);
+					break;
+
 				case WriteState::CentralDirectoryHeader:
 					nbWrittenBytes = writeCentralDirectoryHeader(buffer, bufferSize);
 					break;
@@ -280,8 +227,6 @@ namespace Zip
 					break;
 			}
 
-			LMS_LOG(UI, DEBUG) << "nbWrittenBytes = " << nbWrittenBytes;
-
 			buffer += nbWrittenBytes;
 			bufferSize -= nbWrittenBytes;
 			_currentZipOffset += nbWrittenBytes;
@@ -311,21 +256,15 @@ namespace Zip
 			return 0;
 		}
 
-		LMS_LOG(UTILS, INFO) << "writeLocalFileHeader. crc = " << _currentFile->second.fileCrc32;
 		LocalFileHeader header {buffer, bufferSize};
 
 		header.setSignature();
 		header.setVersionNeededToExtract(1, 0);
-		header.setGeneralPurposeFlags(ZipHeader::GeneralPurposeFlag::LanguageEncoding);
+		header.setGeneralPurposeFlags(ZipHeader::GeneralPurposeFlag::LanguageEncoding | ZipHeader::GeneralPurposeFlag::UseDataDescriptor);
-		header.setCrc32UncompressedData(_currentFile->second.fileCrc32);
+		header.setCompressionMethod(ZipHeader::CompressionMethod::NoCompression);
-		switch (_compMethod)
+		header.setCrc32UncompressedData(ZipHeader::UnknownCrc32);
-		{
+		header.setCompressedSize(ZipHeader::UnknownFileSize);
-			case CompressionMethod::NoCompression:
+		header.setUncompressedSize(ZipHeader::UnknownFileSize);
-				header.setCompressionMethod(ZipHeader::CompressionMethod::NoCompression);
-				header.setCompressedSize(_currentFile->second.fileSize);
-				header.setUncompressedSize(_currentFile->second.fileSize);
-				break;
-		}
 		header.setLastModifiedDateTime(); // getLastWriteTime(*_currentFile));
 		header.setFileNameLength(_currentFile->first.size());
 		header.setExtraFieldLength(0);
@@ -339,6 +278,8 @@ namespace Zip
 	std::size_t
 	Zipper::writeLocalFileHeaderFileName(std::byte* buffer, std::size_t bufferSize)
 	{
+		assert(_currentFile != std::end(_files));
+
 		const std::string& fileName {_currentFile->first};
 
 		assert(_currentOffset <= fileName.size());
@@ -349,10 +290,7 @@ namespace Zip
 			return 0;
 		}
 
-		LMS_LOG(UTILS, INFO) << "writeLocalFileHeaderFileName";
-
 		const std::size_t nbBytesToCopy {std::min<std::size_t>(fileName.size() - _currentOffset, bufferSize)};
-		LMS_LOG(UTILS, INFO) << "\tnbBytesToCopy = " << nbBytesToCopy;
 
 		std::copy(std::next(std::begin(fileName), _currentOffset), std::next(std::begin(fileName), nbBytesToCopy), reinterpret_cast<unsigned char*>(buffer));
 
@@ -363,12 +301,12 @@ namespace Zip
 	std::size_t
 	Zipper::writeFileData(std::byte* buffer, std::size_t bufferSize)
 	{
+		assert(_currentFile != std::end(_files));
+
 		if (_currentOffset == _currentFile->second.fileSize)
 		{
 			_currentOffset = 0;
-			++_currentFile;
+			_writeState = WriteState::DataDescriptor;
-			_writeState = WriteState::LocalFileHeader;
-
 			return 0;
 		}
 
@@ -391,13 +329,32 @@ namespace Zip
 		ifs.read(reinterpret_cast<char*>(buffer), nbBytesToRead );
 		const ::uint64_t actualReadSize {static_cast<::uint64_t>(ifs.gcount())};
 
-		LMS_LOG(UTILS, INFO) << "writeFileData: to read = " << nbBytesToRead << ", actually read = " << actualReadSize;
+		_currentFile->second.fileCrc32.processBytes(buffer, actualReadSize);
-
 		_currentOffset += actualReadSize;
 
 		return actualReadSize;
 	}
 
+	std::size_t
+	Zipper::writeDataDescriptor(std::byte* buffer, std::size_t bufferSize)
+	{
+		assert(bufferSize >= minOutputBufferSize);
+		static_assert(DataDescriptor::getHeaderSize() <= minOutputBufferSize);
+
+		assert(_currentFile != std::end(_files));
+
+		DataDescriptor desc {buffer, bufferSize};
+		desc.setSignature();
+		desc.setCrc32UncompressedData(_currentFile->second.fileCrc32.getResult());
+		desc.setCompressedSize(_currentFile->second.fileSize);
+		desc.setUncompressedSize(_currentFile->second.fileSize);
+
+		++_currentFile;
+		_writeState = WriteState::LocalFileHeader;
+
+		return desc.getHeaderSize();
+	}
+
 	std::size_t
 	Zipper::writeCentralDirectoryHeader(std::byte* buffer, std::size_t bufferSize)
 	{
@@ -405,10 +362,7 @@ namespace Zip
 		static_assert(CentralDirectoryHeader::getHeaderSize() <= minOutputBufferSize);
 
 		if (_currentFile == std::begin(_files))
-		{
-			LMS_LOG(UI, INFO) << "First record! _currentZipOffset = " << _currentZipOffset;
 			_centralDirectoryOffset = _currentZipOffset;
-		}
 
 		if (_currentFile == std::end(_files))
 		{
@@ -417,23 +371,16 @@ namespace Zip
 			return 0;
 		}
 
-		LMS_LOG(UTILS, INFO) << "writeCentralDirectoryHeader. Relative offset = " << _currentFile->second.localFileHeaderOffset << ", crc = " << std::hex << _currentFile->second.fileCrc32;
-
 		CentralDirectoryHeader header {buffer, bufferSize};
 		header.setSignature();
 		header.setVersionMadeBy(2, 0);
 		header.setVersionNeededToExtract(1, 0);
-		header.setGeneralPurposeFlags(ZipHeader::GeneralPurposeFlag::LanguageEncoding);
+		header.setGeneralPurposeFlags(ZipHeader::GeneralPurposeFlag::LanguageEncoding | ZipHeader::GeneralPurposeFlag::UseDataDescriptor);
-		switch (_compMethod)
+		header.setCompressionMethod(ZipHeader::CompressionMethod::NoCompression);
-		{
+		header.setCompressedSize(_currentFile->second.fileSize);
-			case CompressionMethod::NoCompression:
+		header.setUncompressedSize(_currentFile->second.fileSize);
-				header.setCompressionMethod(ZipHeader::CompressionMethod::NoCompression);
-				header.setCompressedSize(_currentFile->second.fileSize);
-				header.setUncompressedSize(_currentFile->second.fileSize);
-				break;
-		}
 		header.setLastModifiedDateTime(); // getLastWriteTime(*_currentFile));
-		header.setCrc32UncompressedData(_currentFile->second.fileCrc32);
+		header.setCrc32UncompressedData(_currentFile->second.fileCrc32.getResult());
 		header.setFileNameLength(_currentFile->first.size());
 		header.setExtraFieldLength(0);
 		header.setFileCommentLength(0);
@@ -463,7 +410,6 @@ namespace Zip
 			return 0;
 		}
 
-		LMS_LOG(UTILS, INFO) << "writeCentralDirectoryHeaderFileName";
 		const std::size_t nbBytesToCopy {std::min<std::size_t>(fileName.size() - _currentOffset, bufferSize)};
 
 		std::copy(std::next(std::begin(fileName), _currentOffset), std::next(std::begin(fileName), nbBytesToCopy), reinterpret_cast<unsigned char*>(buffer));
@@ -482,8 +428,6 @@ namespace Zip
 
 		EndOfCentralDirectoryRecord record {buffer, bufferSize};
 
-		LMS_LOG(UTILS, DEBUG) << "Writing EOR. nb records = " << _files.size() << ", offset = " << _centralDirectoryOffset << ", size = " << _centralDirectorySize;
-
 		record.setSignature();
 		record.setDiskNumber(0);
 		record.setCentralDirectoryDiskNumber(0);

src/libs/utils/include/utils/Crc32Calculator.hpp

@@ -0,0 +1,47 @@
+/*
+ * Copyright (C) 2020 Emeric Poupon
+ *
+ * This file is part of LMS.
+ *
+ * LMS is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * LMS is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with LMS.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#pragma once
+
+#include <boost/crc.hpp>  // for boost::crc_32_type
+
+namespace Utils
+{
+
+	class Crc32Calculator
+	{
+		public:
+
+			void processBytes(const std::byte* _data, std::size_t dataSize)
+			{
+				_result.process_bytes(_data, dataSize);
+			}
+
+			std::uint32_t getResult() const
+			{
+				return _result.checksum();
+			}
+
+		private:
+			using Crc32Type = boost::crc_32_type;
+			Crc32Type _result;
+	};
+
+}
+

src/libs/utils/include/utils/Zipper.hpp

@@ -24,6 +24,7 @@
 #include <filesystem>
 
 #include "Exception.hpp"
+#include "utils/Crc32Calculator.hpp"
 
 namespace Zip
 {
@@ -33,17 +34,12 @@ namespace Zip
 		using LmsException::LmsException;
 	};
 
-	// Very simple on-the-fly zip creator
+	// Very simple on-the-fly zip creator, "store" method only
 	class Zipper
 	{
 		public:
 
-			enum class CompressionMethod
+			Zipper(const std::map<std::string, std::filesystem::path>& files);
-			{
-				NoCompression,
-			};
-
-			Zipper(const std::map<std::string, std::filesystem::path>& files, CompressionMethod comp = CompressionMethod::NoCompression);
 
 			static constexpr std::size_t minOutputBufferSize = 64;
 			std::size_t writeSome(std::byte* buffer, std::size_t bufferSize);
@@ -55,6 +51,7 @@ namespace Zip
 			std::size_t writeLocalFileHeader(std::byte* buffer, std::size_t bufferSize);
 			std::size_t writeLocalFileHeaderFileName(std::byte* buffer, std::size_t bufferSize);
 			std::size_t writeFileData(std::byte* buffer, std::size_t bufferSize);
+			std::size_t writeDataDescriptor(std::byte* buffer, std::size_t bufferSize);
 			std::size_t writeCentralDirectoryHeader(std::byte* buffer, std::size_t bufferSize);
 			std::size_t writeCentralDirectoryHeaderFileName(std::byte* buffer, std::size_t bufferSize);
 			std::size_t writeEndOfCentralDirectoryRecord(std::byte* buffer, std::size_t bufferSize);
@@ -63,7 +60,7 @@ namespace Zip
 			{
 				std::filesystem::path filePath;
 				std::size_t fileSize;
-				std::uint32_t fileCrc32;
+				Utils::Crc32Calculator fileCrc32;
 				std::size_t localFileHeaderOffset {};
 			};
 
@@ -75,13 +72,13 @@ namespace Zip
 				LocalFileHeader,
 				LocalFileHeaderFileName,
 				FileData,
+				DataDescriptor,
 				CentralDirectoryHeader,
 				CentralDirectoryHeaderFileName,
 				EndOfCentralDirectoryRecord,
 				Complete,
 			};
 
-			CompressionMethod _compMethod {CompressionMethod::NoCompression};
 			WriteState _writeState {WriteState::LocalFileHeader};
 			FileContainer::iterator _currentFile;
 			std::size_t _currentOffset {};

src/lms/ui/resource/DownloadResource.cpp

@@ -19,6 +19,9 @@
 
 #include "DownloadResource.hpp"
 
+#include <iostream>
+#include <iomanip>
+
 #include <Wt/WApplication.h>
 #include <Wt/Http/Response.h>
 
@@ -124,20 +127,18 @@ static
 std::string
 getTrackPathName(Database::Track::pointer track)
 {
-	std::string fileName;
+	std::ostringstream fileName;
 
-	auto trackNumber {track->getTrackNumber()};
+	if (auto discNumber {track->getDiscNumber()})
-	auto discNumber {track->getDiscNumber()};
+		fileName << *discNumber << ".";
+	if (auto trackNumber {track->getTrackNumber()})
+		fileName << std::setw(2) << std::setfill('0') << *trackNumber << " - ";
 
-	if (discNumber)
+	fileName << StringUtils::replaceInString(track->getName(), "/", "_") << track->getPath().filename().extension().string();
-		fileName += std::to_string(*discNumber) + ".";
-	if (trackNumber)
-		fileName += std::to_string(*trackNumber) + " - ";
 
-	fileName += track->getName() + track->getPath().filename().extension().string();
+	LOG(DEBUG) << "Forged filename = '" << fileName.str() << "'";
-	fileName = StringUtils::replaceInString(fileName , "/", "_");
 
-	return fileName;
+	return fileName.str();
 }
 
 static
@@ -166,7 +167,7 @@ createZipper(const std::vector<Database::Track::pointer>& tracks)
 		files.emplace(fileName, track->getPath());
 	}
 
-	return std::make_unique<Zip::Zipper>(files, Zip::Zipper::CompressionMethod::NoCompression);
+	return std::make_unique<Zip::Zipper>(files);
 }
 
 DownloadReleaseResource::DownloadReleaseResource(Database::IdType releaseId)

src/tools/zipper/LmsZipper.cpp

@@ -54,7 +54,7 @@ int main(int argc, char* argv[])
 		return EXIT_FAILURE;
 	}
 
-	Zipper zipper {files, Zipper::CompressionMethod::NoCompression};
+	Zipper zipper {files};
 
 	while (!zipper.isComplete())
 	{