From 01a2e23194f86df1d0dcc4a3ccb3ea37e8f8c98b Mon Sep 17 00:00:00 2001 From: Jeremy Rose Date: Tue, 4 Aug 2020 11:48:04 -0700 Subject: [PATCH] refactor: mmap asar files (#24470) --- lib/asar/fs-wrapper.ts | 49 +++++++----- shell/common/api/electron_api_asar.cc | 72 ++++++++++++++++-- shell/common/asar/archive.cc | 86 +++++++++------------- shell/common/asar/archive.h | 8 +- shell/common/asar/scoped_temporary_file.cc | 23 ------ shell/common/asar/scoped_temporary_file.h | 6 -- typings/internal-ambient.d.ts | 3 +- 7 files changed, 133 insertions(+), 114 deletions(-) diff --git a/lib/asar/fs-wrapper.ts b/lib/asar/fs-wrapper.ts index 17191165dd7c..441239b5099e 100644 --- a/lib/asar/fs-wrapper.ts +++ b/lib/asar/fs-wrapper.ts @@ -529,17 +529,15 @@ export const wrapFsWithAsar = (fs: Record) => { return fs.readFile(realPath, options, callback); } - const buffer = Buffer.alloc(info.size); - const fd = archive.getFd(); - if (!(fd >= 0)) { - const error = createError(AsarError.NOT_FOUND, { asarPath, filePath }); - nextTick(callback, [error]); - return; - } - logASARAccess(asarPath, filePath, info.offset); - fs.read(fd, buffer, 0, info.size, info.offset, (error: Error) => { - callback(error, encoding ? buffer.toString(encoding) : buffer); + archive.read(info.offset, info.size).then((buf) => { + const buffer = Buffer.from(buf); + callback(null, encoding ? buffer.toString(encoding) : buffer); + }, (err) => { + const error: AsarErrorObject = new Error(`EINVAL, ${err.message} while reading ${filePath} in ${asarPath}`); + error.code = 'EINVAL'; + error.errno = -22; + callback(error); }); }; @@ -572,13 +570,19 @@ export const wrapFsWithAsar = (fs: Record) => { } const { encoding } = options; - const buffer = Buffer.alloc(info.size); - const fd = archive.getFd(); - if (!(fd >= 0)) throw createError(AsarError.NOT_FOUND, { asarPath, filePath }); logASARAccess(asarPath, filePath, info.offset); - fs.readSync(fd, buffer, 0, info.size, info.offset); - return (encoding) ? buffer.toString(encoding) : buffer; + let arrayBuffer: ArrayBuffer; + try { + arrayBuffer = archive.readSync(info.offset, info.size); + } catch (err) { + const error: AsarErrorObject = new Error(`EINVAL, ${err.message} while reading ${filePath} in ${asarPath}`); + error.code = 'EINVAL'; + error.errno = -22; + throw error; + } + const buffer = Buffer.from(arrayBuffer); + return encoding ? buffer.toString(encoding) : buffer; }; const { readdir } = fs; @@ -685,12 +689,17 @@ export const wrapFsWithAsar = (fs: Record) => { return fs.readFileSync(realPath, { encoding: 'utf8' }); } - const buffer = Buffer.alloc(info.size); - const fd = archive.getFd(); - if (!(fd >= 0)) return []; - logASARAccess(asarPath, filePath, info.offset); - fs.readSync(fd, buffer, 0, info.size, info.offset); + let arrayBuffer: ArrayBuffer; + try { + arrayBuffer = archive.readSync(info.offset, info.size); + } catch (err) { + const error: AsarErrorObject = new Error(`EINVAL, ${err.message} while reading ${filePath} in ${asarPath}`); + error.code = 'EINVAL'; + error.errno = -22; + throw error; + } + const buffer = Buffer.from(arrayBuffer); const str = buffer.toString('utf8'); return [str, str.length > 0]; }; diff --git a/shell/common/api/electron_api_asar.cc b/shell/common/api/electron_api_asar.cc index d449e29db791..b72c9304e02a 100644 --- a/shell/common/api/electron_api_asar.cc +++ b/shell/common/api/electron_api_asar.cc @@ -4,6 +4,7 @@ #include +#include "base/numerics/safe_math.h" #include "gin/handle.h" #include "gin/object_template_builder.h" #include "gin/wrappable.h" @@ -12,6 +13,9 @@ #include "shell/common/gin_converters/callback_converter.h" #include "shell/common/gin_converters/file_path_converter.h" #include "shell/common/gin_helper/dictionary.h" +#include "shell/common/gin_helper/error_thrower.h" +#include "shell/common/gin_helper/function_template_extensions.h" +#include "shell/common/gin_helper/promise.h" #include "shell/common/node_includes.h" #include "shell/common/node_util.h" @@ -38,7 +42,8 @@ class Archive : public gin::Wrappable { .SetMethod("readdir", &Archive::Readdir) .SetMethod("realpath", &Archive::Realpath) .SetMethod("copyFileOut", &Archive::CopyFileOut) - .SetMethod("getFd", &Archive::GetFD); + .SetMethod("read", &Archive::Read) + .SetMethod("readSync", &Archive::ReadSync); } const char* GetTypeName() override { return "Archive"; } @@ -104,15 +109,68 @@ class Archive : public gin::Wrappable { return gin::ConvertToV8(isolate, new_path); } - // Return the file descriptor. - int GetFD() const { - if (!archive_) - return -1; - return archive_->GetFD(); + v8::Local ReadSync(gin_helper::ErrorThrower thrower, + uint64_t offset, + uint64_t length) { + base::CheckedNumeric safe_offset(offset); + base::CheckedNumeric safe_end = safe_offset + length; + if (!safe_end.IsValid() || + safe_end.ValueOrDie() > archive_->file()->length()) { + thrower.ThrowError("Out of bounds read"); + return v8::Local(); + } + auto array_buffer = v8::ArrayBuffer::New(thrower.isolate(), length); + auto backing_store = array_buffer->GetBackingStore(); + memcpy(backing_store->Data(), archive_->file()->data() + offset, length); + return array_buffer; + } + + v8::Local Read(v8::Isolate* isolate, + uint64_t offset, + uint64_t length) { + gin_helper::Promise> promise(isolate); + v8::Local handle = promise.GetHandle(); + + base::CheckedNumeric safe_offset(offset); + base::CheckedNumeric safe_end = safe_offset + length; + if (!safe_end.IsValid() || + safe_end.ValueOrDie() > archive_->file()->length()) { + promise.RejectWithErrorMessage("Out of bounds read"); + return handle; + } + + auto backing_store = v8::ArrayBuffer::NewBackingStore(isolate, length); + base::ThreadPool::PostTaskAndReplyWithResult( + FROM_HERE, {base::MayBlock(), base::TaskPriority::USER_VISIBLE}, + base::BindOnce(&Archive::ReadOnIO, isolate, archive_, + std::move(backing_store), offset, length), + base::BindOnce(&Archive::ResolveReadOnUI, std::move(promise))); + + return handle; } private: - std::unique_ptr archive_; + static std::unique_ptr ReadOnIO( + v8::Isolate* isolate, + std::shared_ptr archive, + std::unique_ptr backing_store, + uint64_t offset, + uint64_t length) { + memcpy(backing_store->Data(), archive->file()->data() + offset, length); + return backing_store; + } + + static void ResolveReadOnUI( + gin_helper::Promise> promise, + std::unique_ptr backing_store) { + v8::HandleScope scope(promise.isolate()); + v8::Context::Scope context_scope(promise.GetContext()); + auto array_buffer = + v8::ArrayBuffer::New(promise.isolate(), std::move(backing_store)); + promise.Resolve(array_buffer); + } + + std::shared_ptr archive_; DISALLOW_COPY_AND_ASSIGN(Archive); }; diff --git a/shell/common/asar/archive.cc b/shell/common/asar/archive.cc index 89c5a8d418c9..e552ea3f7fa5 100644 --- a/shell/common/asar/archive.cc +++ b/shell/common/asar/archive.cc @@ -117,78 +117,51 @@ bool FillFileInfoWithNode(Archive::FileInfo* info, } // namespace -Archive::Archive(const base::FilePath& path) - : path_(path), file_(base::File::FILE_OK) { +Archive::Archive(const base::FilePath& path) : path_(path) { base::ThreadRestrictions::ScopedAllowIO allow_io; - file_.Initialize(path_, base::File::FLAG_OPEN | base::File::FLAG_READ); -#if defined(OS_WIN) - fd_ = _open_osfhandle(reinterpret_cast(file_.GetPlatformFile()), 0); -#elif defined(OS_POSIX) - fd_ = file_.GetPlatformFile(); -#endif + if (!file_.Initialize(path_)) { + LOG(ERROR) << "Failed to open ASAR archive at '" << path_.value() << "'"; + } } -Archive::~Archive() { -#if defined(OS_WIN) - if (fd_ != -1) { - _close(fd_); - // Don't close the handle since we already closed the fd. - file_.TakePlatformFile(); - } -#endif - base::ThreadRestrictions::ScopedAllowIO allow_io; - file_.Close(); -} +Archive::~Archive() {} bool Archive::Init() { if (!file_.IsValid()) { - if (file_.error_details() != base::File::FILE_ERROR_NOT_FOUND) { - LOG(WARNING) << "Opening " << path_.value() << ": " - << base::File::ErrorToString(file_.error_details()); - } return false; } - std::vector buf; - int len; - - buf.resize(8); - { - base::ThreadRestrictions::ScopedAllowIO allow_io; - len = file_.ReadAtCurrentPos(buf.data(), buf.size()); - } - if (len != static_cast(buf.size())) { - PLOG(ERROR) << "Failed to read header size from " << path_.value(); + if (file_.length() < 8) { + LOG(ERROR) << "Malformed ASAR file at '" << path_.value() + << "' (too short)"; return false; } uint32_t size; - if (!base::PickleIterator(base::Pickle(buf.data(), buf.size())) - .ReadUInt32(&size)) { - LOG(ERROR) << "Failed to parse header size from " << path_.value(); + base::PickleIterator size_pickle( + base::Pickle(reinterpret_cast(file_.data()), 8)); + if (!size_pickle.ReadUInt32(&size)) { + LOG(ERROR) << "Failed to read header size at '" << path_.value() << "'"; return false; } - buf.resize(size); - { - base::ThreadRestrictions::ScopedAllowIO allow_io; - len = file_.ReadAtCurrentPos(buf.data(), buf.size()); - } - if (len != static_cast(buf.size())) { - PLOG(ERROR) << "Failed to read header from " << path_.value(); + if (file_.length() - 8 < size) { + LOG(ERROR) << "Malformed ASAR file at '" << path_.value() + << "' (incorrect header)"; return false; } + base::PickleIterator header_pickle( + base::Pickle(reinterpret_cast(file_.data() + 8), size)); std::string header; - if (!base::PickleIterator(base::Pickle(buf.data(), buf.size())) - .ReadString(&header)) { - LOG(ERROR) << "Failed to parse header from " << path_.value(); + if (!header_pickle.ReadString(&header)) { + LOG(ERROR) << "Failed to read header string at '" << path_.value() << "'"; return false; } base::Optional value = base::JSONReader::Read(header); if (!value || !value->is_dict()) { - LOG(ERROR) << "Failed to parse header"; + LOG(ERROR) << "Header was not valid JSON at '" << path_.value() << "'"; return false; } @@ -291,11 +264,24 @@ bool Archive::CopyFileOut(const base::FilePath& path, base::FilePath* out) { return true; } + base::CheckedNumeric safe_offset(info.offset); + auto safe_end = safe_offset + info.size; + if (!safe_end.IsValid() || safe_end.ValueOrDie() > file_.length()) + return false; + auto temp_file = std::make_unique(); base::FilePath::StringType ext = path.Extension(); - if (!temp_file->InitFromFile(&file_, ext, info.offset, info.size)) + if (!temp_file->Init(ext)) return false; + base::File dest(temp_file->path(), + base::File::FLAG_OPEN | base::File::FLAG_WRITE); + if (!dest.IsValid()) + return false; + + dest.WriteAtCurrentPos( + reinterpret_cast(file_.data() + info.offset), info.size); + #if defined(OS_POSIX) if (info.executable) { // chmod a+x temp_file; @@ -308,8 +294,4 @@ bool Archive::CopyFileOut(const base::FilePath& path, base::FilePath* out) { return true; } -int Archive::GetFD() const { - return fd_; -} - } // namespace asar diff --git a/shell/common/asar/archive.h b/shell/common/asar/archive.h index d97cb03cc781..86d84aa8e7f8 100644 --- a/shell/common/asar/archive.h +++ b/shell/common/asar/archive.h @@ -11,6 +11,7 @@ #include "base/files/file.h" #include "base/files/file_path.h" +#include "base/files/memory_mapped_file.h" namespace base { class DictionaryValue; @@ -61,16 +62,13 @@ class Archive { // For unpacked file, this method will return its real path. bool CopyFileOut(const base::FilePath& path, base::FilePath* out); - // Returns the file's fd. - int GetFD() const; - + base::MemoryMappedFile* file() { return &file_; } base::FilePath path() const { return path_; } base::DictionaryValue* header() const { return header_.get(); } private: base::FilePath path_; - base::File file_; - int fd_ = -1; + base::MemoryMappedFile file_; uint32_t header_size_ = 0; std::unique_ptr header_; diff --git a/shell/common/asar/scoped_temporary_file.cc b/shell/common/asar/scoped_temporary_file.cc index c09cc2b4d71d..2aba50fbcb01 100644 --- a/shell/common/asar/scoped_temporary_file.cc +++ b/shell/common/asar/scoped_temporary_file.cc @@ -48,27 +48,4 @@ bool ScopedTemporaryFile::Init(const base::FilePath::StringType& ext) { return true; } -bool ScopedTemporaryFile::InitFromFile(base::File* src, - const base::FilePath::StringType& ext, - uint64_t offset, - uint64_t size) { - if (!src->IsValid()) - return false; - - if (!Init(ext)) - return false; - - std::vector buf(size); - int len = src->Read(offset, buf.data(), buf.size()); - if (len != static_cast(size)) - return false; - - base::File dest(path_, base::File::FLAG_OPEN | base::File::FLAG_WRITE); - if (!dest.IsValid()) - return false; - - return dest.WriteAtCurrentPos(buf.data(), buf.size()) == - static_cast(size); -} - } // namespace asar diff --git a/shell/common/asar/scoped_temporary_file.h b/shell/common/asar/scoped_temporary_file.h index e42a244e1220..15f65290a926 100644 --- a/shell/common/asar/scoped_temporary_file.h +++ b/shell/common/asar/scoped_temporary_file.h @@ -27,12 +27,6 @@ class ScopedTemporaryFile { // Init an empty temporary file with a certain extension. bool Init(const base::FilePath::StringType& ext); - // Init an temporary file and fill it with content of |path|. - bool InitFromFile(base::File* src, - const base::FilePath::StringType& ext, - uint64_t offset, - uint64_t size); - base::FilePath path() const { return path_; } private: diff --git a/typings/internal-ambient.d.ts b/typings/internal-ambient.d.ts index 58b76dbcc3d6..103c940ee7b5 100644 --- a/typings/internal-ambient.d.ts +++ b/typings/internal-ambient.d.ts @@ -68,7 +68,8 @@ declare namespace NodeJS { readdir(path: string): string[] | false; realpath(path: string): string | false; copyFileOut(path: string): string | false; - getFd(): number | -1; + read(offset: number, size: number): Promise; + readSync(offset: number, size: number): ArrayBuffer; } interface AsarBinding {