Skip to content

Commit 01a2e23

Browse files
nornagonnornagon
authored
refactor: mmap asar files (electron#24470)
1 parent 15ee34a commit 01a2e23

7 files changed

Lines changed: 133 additions & 114 deletions

File tree

‎lib/asar/fs-wrapper.ts‎

Lines changed: 29 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -529,17 +529,15 @@ export const wrapFsWithAsar = (fs: Record<string, any>) => {
529529
return fs.readFile(realPath, options, callback);
530530
}
531531

532-
const buffer = Buffer.alloc(info.size);
533-
const fd = archive.getFd();
534-
if (!(fd >= 0)) {
535-
const error = createError(AsarError.NOT_FOUND, { asarPath, filePath });
536-
nextTick(callback, [error]);
537-
return;
538-
}
539-
540532
logASARAccess(asarPath, filePath, info.offset);
541-
fs.read(fd, buffer, 0, info.size, info.offset, (error: Error) => {
542-
callback(error, encoding ? buffer.toString(encoding) : buffer);
533+
archive.read(info.offset, info.size).then((buf) => {
534+
const buffer = Buffer.from(buf);
535+
callback(null, encoding ? buffer.toString(encoding) : buffer);
536+
}, (err) => {
537+
const error: AsarErrorObject = new Error(`EINVAL, ${err.message} while reading ${filePath} in ${asarPath}`);
538+
error.code = 'EINVAL';
539+
error.errno = -22;
540+
callback(error);
543541
});
544542
};
545543

@@ -572,13 +570,19 @@ export const wrapFsWithAsar = (fs: Record<string, any>) => {
572570
}
573571

574572
const { encoding } = options;
575-
const buffer = Buffer.alloc(info.size);
576-
const fd = archive.getFd();
577-
if (!(fd >= 0)) throw createError(AsarError.NOT_FOUND, { asarPath, filePath });
578573

579574
logASARAccess(asarPath, filePath, info.offset);
580-
fs.readSync(fd, buffer, 0, info.size, info.offset);
581-
return (encoding) ? buffer.toString(encoding) : buffer;
575+
let arrayBuffer: ArrayBuffer;
576+
try {
577+
arrayBuffer = archive.readSync(info.offset, info.size);
578+
} catch (err) {
579+
const error: AsarErrorObject = new Error(`EINVAL, ${err.message} while reading ${filePath} in ${asarPath}`);
580+
error.code = 'EINVAL';
581+
error.errno = -22;
582+
throw error;
583+
}
584+
const buffer = Buffer.from(arrayBuffer);
585+
return encoding ? buffer.toString(encoding) : buffer;
582586
};
583587

584588
const { readdir } = fs;
@@ -685,12 +689,17 @@ export const wrapFsWithAsar = (fs: Record<string, any>) => {
685689
return fs.readFileSync(realPath, { encoding: 'utf8' });
686690
}
687691

688-
const buffer = Buffer.alloc(info.size);
689-
const fd = archive.getFd();
690-
if (!(fd >= 0)) return [];
691-
692692
logASARAccess(asarPath, filePath, info.offset);
693-
fs.readSync(fd, buffer, 0, info.size, info.offset);
693+
let arrayBuffer: ArrayBuffer;
694+
try {
695+
arrayBuffer = archive.readSync(info.offset, info.size);
696+
} catch (err) {
697+
const error: AsarErrorObject = new Error(`EINVAL, ${err.message} while reading ${filePath} in ${asarPath}`);
698+
error.code = 'EINVAL';
699+
error.errno = -22;
700+
throw error;
701+
}
702+
const buffer = Buffer.from(arrayBuffer);
694703
const str = buffer.toString('utf8');
695704
return [str, str.length > 0];
696705
};

‎shell/common/api/electron_api_asar.cc‎

Lines changed: 65 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
#include <vector>
66

7+
#include "base/numerics/safe_math.h"
78
#include "gin/handle.h"
89
#include "gin/object_template_builder.h"
910
#include "gin/wrappable.h"
@@ -12,6 +13,9 @@
1213
#include "shell/common/gin_converters/callback_converter.h"
1314
#include "shell/common/gin_converters/file_path_converter.h"
1415
#include "shell/common/gin_helper/dictionary.h"
16+
#include "shell/common/gin_helper/error_thrower.h"
17+
#include "shell/common/gin_helper/function_template_extensions.h"
18+
#include "shell/common/gin_helper/promise.h"
1519
#include "shell/common/node_includes.h"
1620
#include "shell/common/node_util.h"
1721

@@ -38,7 +42,8 @@ class Archive : public gin::Wrappable<Archive> {
3842
.SetMethod("readdir", &Archive::Readdir)
3943
.SetMethod("realpath", &Archive::Realpath)
4044
.SetMethod("copyFileOut", &Archive::CopyFileOut)
41-
.SetMethod("getFd", &Archive::GetFD);
45+
.SetMethod("read", &Archive::Read)
46+
.SetMethod("readSync", &Archive::ReadSync);
4247
}
4348

4449
const char* GetTypeName() override { return "Archive"; }
@@ -104,15 +109,68 @@ class Archive : public gin::Wrappable<Archive> {
104109
return gin::ConvertToV8(isolate, new_path);
105110
}
106111

107-
// Return the file descriptor.
108-
int GetFD() const {
109-
if (!archive_)
110-
return -1;
111-
return archive_->GetFD();
112+
v8::Local<v8::ArrayBuffer> ReadSync(gin_helper::ErrorThrower thrower,
113+
uint64_t offset,
114+
uint64_t length) {
115+
base::CheckedNumeric<uint64_t> safe_offset(offset);
116+
base::CheckedNumeric<uint64_t> safe_end = safe_offset + length;
117+
if (!safe_end.IsValid() ||
118+
safe_end.ValueOrDie() > archive_->file()->length()) {
119+
thrower.ThrowError("Out of bounds read");
120+
return v8::Local<v8::ArrayBuffer>();
121+
}
122+
auto array_buffer = v8::ArrayBuffer::New(thrower.isolate(), length);
123+
auto backing_store = array_buffer->GetBackingStore();
124+
memcpy(backing_store->Data(), archive_->file()->data() + offset, length);
125+
return array_buffer;
126+
}
127+
128+
v8::Local<v8::Promise> Read(v8::Isolate* isolate,
129+
uint64_t offset,
130+
uint64_t length) {
131+
gin_helper::Promise<v8::Local<v8::ArrayBuffer>> promise(isolate);
132+
v8::Local<v8::Promise> handle = promise.GetHandle();
133+
134+
base::CheckedNumeric<uint64_t> safe_offset(offset);
135+
base::CheckedNumeric<uint64_t> safe_end = safe_offset + length;
136+
if (!safe_end.IsValid() ||
137+
safe_end.ValueOrDie() > archive_->file()->length()) {
138+
promise.RejectWithErrorMessage("Out of bounds read");
139+
return handle;
140+
}
141+
142+
auto backing_store = v8::ArrayBuffer::NewBackingStore(isolate, length);
143+
base::ThreadPool::PostTaskAndReplyWithResult(
144+
FROM_HERE, {base::MayBlock(), base::TaskPriority::USER_VISIBLE},
145+
base::BindOnce(&Archive::ReadOnIO, isolate, archive_,
146+
std::move(backing_store), offset, length),
147+
base::BindOnce(&Archive::ResolveReadOnUI, std::move(promise)));
148+
149+
return handle;
112150
}
113151

114152
private:
115-
std::unique_ptr<asar::Archive> archive_;
153+
static std::unique_ptr<v8::BackingStore> ReadOnIO(
154+
v8::Isolate* isolate,
155+
std::shared_ptr<asar::Archive> archive,
156+
std::unique_ptr<v8::BackingStore> backing_store,
157+
uint64_t offset,
158+
uint64_t length) {
159+
memcpy(backing_store->Data(), archive->file()->data() + offset, length);
160+
return backing_store;
161+
}
162+
163+
static void ResolveReadOnUI(
164+
gin_helper::Promise<v8::Local<v8::ArrayBuffer>> promise,
165+
std::unique_ptr<v8::BackingStore> backing_store) {
166+
v8::HandleScope scope(promise.isolate());
167+
v8::Context::Scope context_scope(promise.GetContext());
168+
auto array_buffer =
169+
v8::ArrayBuffer::New(promise.isolate(), std::move(backing_store));
170+
promise.Resolve(array_buffer);
171+
}
172+
173+
std::shared_ptr<asar::Archive> archive_;
116174

117175
DISALLOW_COPY_AND_ASSIGN(Archive);
118176
};

‎shell/common/asar/archive.cc‎

Lines changed: 34 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -117,78 +117,51 @@ bool FillFileInfoWithNode(Archive::FileInfo* info,
117117

118118
} // namespace
119119

120-
Archive::Archive(const base::FilePath& path)
121-
: path_(path), file_(base::File::FILE_OK) {
120+
Archive::Archive(const base::FilePath& path) : path_(path) {
122121
base::ThreadRestrictions::ScopedAllowIO allow_io;
123-
file_.Initialize(path_, base::File::FLAG_OPEN | base::File::FLAG_READ);
124-
#if defined(OS_WIN)
125-
fd_ = _open_osfhandle(reinterpret_cast<intptr_t>(file_.GetPlatformFile()), 0);
126-
#elif defined(OS_POSIX)
127-
fd_ = file_.GetPlatformFile();
128-
#endif
129-
}
130-
131-
Archive::~Archive() {
132-
#if defined(OS_WIN)
133-
if (fd_ != -1) {
134-
_close(fd_);
135-
// Don't close the handle since we already closed the fd.
136-
file_.TakePlatformFile();
122+
if (!file_.Initialize(path_)) {
123+
LOG(ERROR) << "Failed to open ASAR archive at '" << path_.value() << "'";
137124
}
138-
#endif
139-
base::ThreadRestrictions::ScopedAllowIO allow_io;
140-
file_.Close();
141125
}
142126

127+
Archive::~Archive() {}
128+
143129
bool Archive::Init() {
144130
if (!file_.IsValid()) {
145-
if (file_.error_details() != base::File::FILE_ERROR_NOT_FOUND) {
146-
LOG(WARNING) << "Opening " << path_.value() << ": "
147-
<< base::File::ErrorToString(file_.error_details());
148-
}
149131
return false;
150132
}
151133

152-
std::vector<char> buf;
153-
int len;
154-
155-
buf.resize(8);
156-
{
157-
base::ThreadRestrictions::ScopedAllowIO allow_io;
158-
len = file_.ReadAtCurrentPos(buf.data(), buf.size());
159-
}
160-
if (len != static_cast<int>(buf.size())) {
161-
PLOG(ERROR) << "Failed to read header size from " << path_.value();
134+
if (file_.length() < 8) {
135+
LOG(ERROR) << "Malformed ASAR file at '" << path_.value()
136+
<< "' (too short)";
162137
return false;
163138
}
164139

165140
uint32_t size;
166-
if (!base::PickleIterator(base::Pickle(buf.data(), buf.size()))
167-
.ReadUInt32(&size)) {
168-
LOG(ERROR) << "Failed to parse header size from " << path_.value();
141+
base::PickleIterator size_pickle(
142+
base::Pickle(reinterpret_cast<const char*>(file_.data()), 8));
143+
if (!size_pickle.ReadUInt32(&size)) {
144+
LOG(ERROR) << "Failed to read header size at '" << path_.value() << "'";
169145
return false;
170146
}
171147

172-
buf.resize(size);
173-
{
174-
base::ThreadRestrictions::ScopedAllowIO allow_io;
175-
len = file_.ReadAtCurrentPos(buf.data(), buf.size());
176-
}
177-
if (len != static_cast<int>(buf.size())) {
178-
PLOG(ERROR) << "Failed to read header from " << path_.value();
148+
if (file_.length() - 8 < size) {
149+
LOG(ERROR) << "Malformed ASAR file at '" << path_.value()
150+
<< "' (incorrect header)";
179151
return false;
180152
}
181153

154+
base::PickleIterator header_pickle(
155+
base::Pickle(reinterpret_cast<const char*>(file_.data() + 8), size));
182156
std::string header;
183-
if (!base::PickleIterator(base::Pickle(buf.data(), buf.size()))
184-
.ReadString(&header)) {
185-
LOG(ERROR) << "Failed to parse header from " << path_.value();
157+
if (!header_pickle.ReadString(&header)) {
158+
LOG(ERROR) << "Failed to read header string at '" << path_.value() << "'";
186159
return false;
187160
}
188161

189162
base::Optional<base::Value> value = base::JSONReader::Read(header);
190163
if (!value || !value->is_dict()) {
191-
LOG(ERROR) << "Failed to parse header";
164+
LOG(ERROR) << "Header was not valid JSON at '" << path_.value() << "'";
192165
return false;
193166
}
194167

@@ -291,11 +264,24 @@ bool Archive::CopyFileOut(const base::FilePath& path, base::FilePath* out) {
291264
return true;
292265
}
293266

267+
base::CheckedNumeric<uint64_t> safe_offset(info.offset);
268+
auto safe_end = safe_offset + info.size;
269+
if (!safe_end.IsValid() || safe_end.ValueOrDie() > file_.length())
270+
return false;
271+
294272
auto temp_file = std::make_unique<ScopedTemporaryFile>();
295273
base::FilePath::StringType ext = path.Extension();
296-
if (!temp_file->InitFromFile(&file_, ext, info.offset, info.size))
274+
if (!temp_file->Init(ext))
297275
return false;
298276

277+
base::File dest(temp_file->path(),
278+
base::File::FLAG_OPEN | base::File::FLAG_WRITE);
279+
if (!dest.IsValid())
280+
return false;
281+
282+
dest.WriteAtCurrentPos(
283+
reinterpret_cast<const char*>(file_.data() + info.offset), info.size);
284+
299285
#if defined(OS_POSIX)
300286
if (info.executable) {
301287
// chmod a+x temp_file;
@@ -308,8 +294,4 @@ bool Archive::CopyFileOut(const base::FilePath& path, base::FilePath* out) {
308294
return true;
309295
}
310296

311-
int Archive::GetFD() const {
312-
return fd_;
313-
}
314-
315297
} // namespace asar

‎shell/common/asar/archive.h‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
#include "base/files/file.h"
1313
#include "base/files/file_path.h"
14+
#include "base/files/memory_mapped_file.h"
1415

1516
namespace base {
1617
class DictionaryValue;
@@ -61,16 +62,13 @@ class Archive {
6162
// For unpacked file, this method will return its real path.
6263
bool CopyFileOut(const base::FilePath& path, base::FilePath* out);
6364

64-
// Returns the file's fd.
65-
int GetFD() const;
66-
65+
base::MemoryMappedFile* file() { return &file_; }
6766
base::FilePath path() const { return path_; }
6867
base::DictionaryValue* header() const { return header_.get(); }
6968

7069
private:
7170
base::FilePath path_;
72-
base::File file_;
73-
int fd_ = -1;
71+
base::MemoryMappedFile file_;
7472
uint32_t header_size_ = 0;
7573
std::unique_ptr<base::DictionaryValue> header_;
7674

‎shell/common/asar/scoped_temporary_file.cc‎

Lines changed: 0 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -48,27 +48,4 @@ bool ScopedTemporaryFile::Init(const base::FilePath::StringType& ext) {
4848
return true;
4949
}
5050

51-
bool ScopedTemporaryFile::InitFromFile(base::File* src,
52-
const base::FilePath::StringType& ext,
53-
uint64_t offset,
54-
uint64_t size) {
55-
if (!src->IsValid())
56-
return false;
57-
58-
if (!Init(ext))
59-
return false;
60-
61-
std::vector<char> buf(size);
62-
int len = src->Read(offset, buf.data(), buf.size());
63-
if (len != static_cast<int>(size))
64-
return false;
65-
66-
base::File dest(path_, base::File::FLAG_OPEN | base::File::FLAG_WRITE);
67-
if (!dest.IsValid())
68-
return false;
69-
70-
return dest.WriteAtCurrentPos(buf.data(), buf.size()) ==
71-
static_cast<int>(size);
72-
}
73-
7451
} // namespace asar

‎shell/common/asar/scoped_temporary_file.h‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -27,12 +27,6 @@ class ScopedTemporaryFile {
2727
// Init an empty temporary file with a certain extension.
2828
bool Init(const base::FilePath::StringType& ext);
2929

30-
// Init an temporary file and fill it with content of |path|.
31-
bool InitFromFile(base::File* src,
32-
const base::FilePath::StringType& ext,
33-
uint64_t offset,
34-
uint64_t size);
35-
3630
base::FilePath path() const { return path_; }
3731

3832
private:

‎typings/internal-ambient.d.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,8 @@ declare namespace NodeJS {
6868
readdir(path: string): string[] | false;
6969
realpath(path: string): string | false;
7070
copyFileOut(path: string): string | false;
71-
getFd(): number | -1;
71+
read(offset: number, size: number): Promise<ArrayBuffer>;
72+
readSync(offset: number, size: number): ArrayBuffer;
7273
}
7374

7475
interface AsarBinding {

0 commit comments

Comments
 (0)