[ CLI ] Don't resolve paths as URIs for known SDK artifacts

Handles cases where the SDK is installed on a path which includes
percent encoded characters. The OS doesn't decode these characters, so
decoding them before trying to load a snapshot with said characters in
their path will result in files not being found.

Fixes https://github.com/dart-lang/sdk/issues/46364

TEST=pkg/dartdev/test/regress_46364_test.dart

Change-Id: I5f78e8a2049cc0c83555528fcc8f41be946141f5
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/218082
Reviewed-by: Ryan Macnak <rmacnak@google.com>
This commit is contained in:
Ben Konyi 2021-10-27 16:34:00 +00:00
parent 90f4b41eb0
commit 4de9e805ed
7 changed files with 82 additions and 23 deletions

View file

@ -34,6 +34,8 @@ dependencies:
usage: any
dev_dependencies:
expect:
path: ../expect
lints: any
pub_semver: any
test: any

View file

@ -0,0 +1,40 @@
// Copyright (c) 2021, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
import 'dart:io';
import 'package:expect/expect.dart';
import 'package:path/path.dart' as p;
// Copied from package:io
Future<void> copyPath(String from, String to) async {
await Directory(to).create(recursive: true);
await for (final file in Directory(from).list(recursive: true)) {
final copyTo = p.join(to, p.relative(file.path, from: from));
if (file is Directory) {
await Directory(copyTo).create(recursive: true);
} else if (file is File) {
await File(file.path).copy(copyTo);
} else if (file is Link) {
await Link(copyTo).create(await file.target(), recursive: true);
}
}
}
Future<void> main() async {
final exePath = Platform.resolvedExecutable;
final sdkDir = p.dirname(p.dirname(exePath));
// Try to run the VM located on a path with % encoded characters. The VM
// should not try and resolve the path as a URI for SDK artifacts (e.g.,
// dartdev.dart.snapshot).
final d = Directory.systemTemp.createTempSync('dart_symlink%3A');
try {
await copyPath(sdkDir, d.path);
final path = '${d.path}/bin/dart';
final result = await Process.run(path, ['help']);
Expect.equals(result.exitCode, 0);
} finally {
await d.delete(recursive: true);
}
}

View file

@ -231,9 +231,11 @@ void DFE::CompileAndReadScript(const char* script_uri,
void DFE::ReadScript(const char* script_uri,
uint8_t** kernel_buffer,
intptr_t* kernel_buffer_size) const {
intptr_t* kernel_buffer_size,
bool decode_uri) const {
int64_t start = Dart_TimelineGetMicros();
if (!TryReadKernelFile(script_uri, kernel_buffer, kernel_buffer_size)) {
if (!TryReadKernelFile(script_uri, kernel_buffer, kernel_buffer_size,
decode_uri)) {
return;
}
if (!Dart_IsKernel(*kernel_buffer, *kernel_buffer_size)) {
@ -274,9 +276,12 @@ static bool TryReadSimpleKernelBuffer(uint8_t* buffer,
///
/// If successful, newly allocated buffer with file contents is returned in
/// [buffer], file contents byte count - in [size].
static bool TryReadFile(const char* script_uri, uint8_t** buffer,
intptr_t* size) {
void* script_file = DartUtils::OpenFileUri(script_uri, false);
static bool TryReadFile(const char* script_uri,
uint8_t** buffer,
intptr_t* size,
bool decode_uri = true) {
void* script_file = decode_uri ? DartUtils::OpenFileUri(script_uri, false)
: DartUtils::OpenFile(script_uri, false);
if (script_file == nullptr) {
return false;
}
@ -417,12 +422,13 @@ static bool TryReadKernelListBuffer(const char* script_uri,
bool DFE::TryReadKernelFile(const char* script_uri,
uint8_t** kernel_ir,
intptr_t* kernel_ir_size) {
intptr_t* kernel_ir_size,
bool decode_uri) {
*kernel_ir = nullptr;
*kernel_ir_size = -1;
uint8_t* buffer;
if (!TryReadFile(script_uri, &buffer, kernel_ir_size)) {
if (!TryReadFile(script_uri, &buffer, kernel_ir_size, decode_uri)) {
return false;
}

View file

@ -92,7 +92,8 @@ class DFE {
// valid kernel file, false otherwise.
void ReadScript(const char* script_uri,
uint8_t** kernel_buffer,
intptr_t* kernel_buffer_size) const;
intptr_t* kernel_buffer_size,
bool decode_uri = true) const;
bool KernelServiceDillAvailable() const;
@ -103,7 +104,8 @@ class DFE {
// was returned.
static bool TryReadKernelFile(const char* script_uri,
uint8_t** kernel_buffer,
intptr_t* kernel_buffer_size);
intptr_t* kernel_buffer_size,
bool decode_uri = true);
// We distinguish between "intent to use Dart frontend" vs "can actually
// use Dart frontend". The method UseDartFrontend tells us about the

View file

@ -431,8 +431,9 @@ static Dart_Isolate CreateAndSetupKernelIsolate(const char* script_uri,
AppSnapshot* app_snapshot = NULL;
// Kernel isolate uses an app snapshot or uses the dill file.
if ((kernel_snapshot_uri != NULL) &&
(app_snapshot = Snapshot::TryReadAppSnapshot(kernel_snapshot_uri)) !=
NULL) {
(app_snapshot = Snapshot::TryReadAppSnapshot(
kernel_snapshot_uri, /*force_load_elf_from_memory=*/false,
/*decode_uri=*/false)) != nullptr) {
const uint8_t* isolate_snapshot_data = NULL;
const uint8_t* isolate_snapshot_instructions = NULL;
const uint8_t* ignore_vm_snapshot_data;
@ -603,8 +604,9 @@ static Dart_Isolate CreateAndSetupDartDevIsolate(const char* script_uri,
AppSnapshot* app_snapshot = nullptr;
bool isolate_run_app_snapshot = true;
if (dartdev_path.get() != nullptr &&
(app_snapshot = Snapshot::TryReadAppSnapshot(dartdev_path.get())) !=
nullptr) {
(app_snapshot = Snapshot::TryReadAppSnapshot(
dartdev_path.get(), /*force_load_elf_from_memory=*/false,
/*decode_uri=*/false)) != nullptr) {
const uint8_t* isolate_snapshot_data = NULL;
const uint8_t* isolate_snapshot_instructions = NULL;
const uint8_t* ignore_vm_snapshot_data;
@ -613,8 +615,8 @@ static Dart_Isolate CreateAndSetupDartDevIsolate(const char* script_uri,
&ignore_vm_snapshot_data, &ignore_vm_snapshot_instructions,
&isolate_snapshot_data, &isolate_snapshot_instructions);
isolate_group_data =
new IsolateGroupData(dartdev_path.get(), packages_config, app_snapshot,
isolate_run_app_snapshot);
new IsolateGroupData(DART_DEV_ISOLATE_NAME, packages_config,
app_snapshot, isolate_run_app_snapshot);
isolate_data = new IsolateData(isolate_group_data);
isolate = Dart_CreateIsolateGroup(
DART_DEV_ISOLATE_NAME, DART_DEV_ISOLATE_NAME, isolate_snapshot_data,
@ -642,7 +644,7 @@ static Dart_Isolate CreateAndSetupDartDevIsolate(const char* script_uri,
uint8_t* application_kernel_buffer = NULL;
intptr_t application_kernel_buffer_size = 0;
dfe.ReadScript(dartdev_path.get(), &application_kernel_buffer,
&application_kernel_buffer_size);
&application_kernel_buffer_size, /*decode_uri=*/false);
isolate_group_data->SetKernelBufferNewlyOwned(
application_kernel_buffer, application_kernel_buffer_size);

View file

@ -330,13 +330,19 @@ static AppSnapshot* TryReadAppSnapshotDynamicLibrary(const char* script_name) {
#endif // defined(DART_PRECOMPILED_RUNTIME)
AppSnapshot* Snapshot::TryReadAppSnapshot(const char* script_uri,
bool force_load_elf_from_memory) {
auto decoded_path = File::UriToPath(script_uri);
if (decoded_path == nullptr) {
return nullptr;
bool force_load_elf_from_memory,
bool decode_uri) {
Utils::CStringUniquePtr decoded_path(nullptr, std::free);
const char* script_name = nullptr;
if (decode_uri) {
decoded_path = File::UriToPath(script_uri);
if (decoded_path == nullptr) {
return nullptr;
}
script_name = decoded_path.get();
} else {
script_name = script_uri;
}
const char* script_name = decoded_path.get();
if (File::GetType(nullptr, script_name, true) != File::kIsFile) {
// If 'script_name' refers to a pipe, don't read to check for an app
// snapshot since we cannot rewind if it isn't (and couldn't mmap it in

View file

@ -41,7 +41,8 @@ class Snapshot {
static AppSnapshot* TryReadAppendedAppSnapshotElf(const char* container_path);
static AppSnapshot* TryReadAppSnapshot(
const char* script_uri,
bool force_load_elf_from_memory = false);
bool force_load_elf_from_memory = false,
bool decode_uri = true);
static void WriteAppSnapshot(const char* filename,
uint8_t* vm_data_buffer,
intptr_t vm_data_size,