Skip to content

Commit de7fe08

Browse files
anonrigUlisesGascon
anonrig
authored and
UlisesGascon
committed
fs,url: refactor FileURLToPath method
PR-URL: #50090 Reviewed-By: Geoffrey Booth <[email protected]> Reviewed-By: Vinícius Lourenço Claro Cardoso <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Stephen Belanger <[email protected]>
1 parent 186e6e0 commit de7fe08

File tree

3 files changed

+62
-85
lines changed

3 files changed

+62
-85
lines changed

‎src/node_file.cc‎

Lines changed: 37 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -2904,45 +2904,41 @@ void BindingData::LegacyMainResolve(const FunctionCallbackInfo<Value>& args) {
29042904
CHECK(args[0]->IsString());
29052905

29062906
Environment* env = Environment::GetCurrent(args);
2907+
auto isolate = env->isolate();
29072908

2908-
Utf8Value utf8_package_json_url(env->isolate(), args[0].As<String>());
2909+
Utf8Value utf8_package_json_url(isolate, args[0]);
29092910
auto package_json_url =
29102911
ada::parse<ada::url_aggregator>(utf8_package_json_url.ToStringView());
29112912

29122913
if (!package_json_url) {
2913-
env->isolate()->ThrowException(
2914-
ERR_INVALID_URL(env->isolate(), "Invalid URL"));
2915-
2914+
THROW_ERR_INVALID_URL(isolate, "Invalid URL");
29162915
return;
29172916
}
29182917

29192918
ada::result<ada::url_aggregator> file_path_url;
2920-
std::string initial_file_path;
2919+
std::optional<std::string> initial_file_path;
29212920
std::string file_path;
29222921

2923-
if (args.Length() >= 2 && !args[1]->IsNullOrUndefined() &&
2924-
args[1]->IsString()) {
2925-
std::string package_config_main =
2926-
Utf8Value(env->isolate(), args[1].As<String>()).ToString();
2922+
if (args.Length() >= 2 && args[1]->IsString()) {
2923+
auto package_config_main = Utf8Value(isolate, args[1]).ToString();
29272924

29282925
file_path_url = ada::parse<ada::url_aggregator>(
29292926
std::string("./") + package_config_main, &package_json_url.value());
29302927

29312928
if (!file_path_url) {
2932-
env->isolate()->ThrowException(
2933-
ERR_INVALID_URL(env->isolate(), "Invalid URL"));
2934-
2929+
THROW_ERR_INVALID_URL(isolate, "Invalid URL");
29352930
return;
29362931
}
29372932

2938-
if (!node::url::FileURLToPath(
2939-
env, file_path_url.value(), initial_file_path))
2933+
initial_file_path = node::url::FileURLToPath(env, *file_path_url);
2934+
if (!initial_file_path.has_value()) {
29402935
return;
2936+
}
29412937

2942-
FromNamespacedPath(&initial_file_path);
2938+
FromNamespacedPath(&initial_file_path.value());
29432939

29442940
for (int i = 0; i < legacy_main_extensions_with_main_end; i++) {
2945-
file_path = initial_file_path + std::string(legacy_main_extensions[i]);
2941+
file_path = *initial_file_path + std::string(legacy_main_extensions[i]);
29462942

29472943
switch (FilePathIsFile(env, file_path)) {
29482944
case BindingData::FilePathIsFileReturnType::kIsFile:
@@ -2965,21 +2961,21 @@ void BindingData::LegacyMainResolve(const FunctionCallbackInfo<Value>& args) {
29652961
ada::parse<ada::url_aggregator>("./index", &package_json_url.value());
29662962

29672963
if (!file_path_url) {
2968-
env->isolate()->ThrowException(
2969-
ERR_INVALID_URL(env->isolate(), "Invalid URL"));
2970-
2964+
THROW_ERR_INVALID_URL(isolate, "Invalid URL");
29712965
return;
29722966
}
29732967

2974-
if (!node::url::FileURLToPath(env, file_path_url.value(), initial_file_path))
2968+
initial_file_path = node::url::FileURLToPath(env, *file_path_url);
2969+
if (!initial_file_path.has_value()) {
29752970
return;
2971+
}
29762972

2977-
FromNamespacedPath(&initial_file_path);
2973+
FromNamespacedPath(&initial_file_path.value());
29782974

29792975
for (int i = legacy_main_extensions_with_main_end;
29802976
i < legacy_main_extensions_package_fallback_end;
29812977
i++) {
2982-
file_path = initial_file_path + std::string(legacy_main_extensions[i]);
2978+
file_path = *initial_file_path + std::string(legacy_main_extensions[i]);
29832979

29842980
switch (FilePathIsFile(env, file_path)) {
29852981
case BindingData::FilePathIsFileReturnType::kIsFile:
@@ -2996,39 +2992,39 @@ void BindingData::LegacyMainResolve(const FunctionCallbackInfo<Value>& args) {
29962992
}
29972993
}
29982994

2999-
std::string module_path;
3000-
std::string module_base;
2995+
std::optional<std::string> module_path =
2996+
node::url::FileURLToPath(env, *package_json_url);
2997+
std::optional<std::string> module_base;
30012998

3002-
if (!node::url::FileURLToPath(env, package_json_url.value(), module_path))
2999+
if (!module_path.has_value()) {
30033000
return;
3001+
}
30043002

3005-
if (args.Length() >= 3 && !args[2]->IsNullOrUndefined() &&
3006-
args[2]->IsString()) {
3007-
Utf8Value utf8_base_path(env->isolate(), args[2].As<String>());
3003+
if (args.Length() >= 3 && args[2]->IsString()) {
3004+
Utf8Value utf8_base_path(isolate, args[2]);
30083005
auto base_url =
30093006
ada::parse<ada::url_aggregator>(utf8_base_path.ToStringView());
30103007

30113008
if (!base_url) {
3012-
env->isolate()->ThrowException(
3013-
ERR_INVALID_URL(env->isolate(), "Invalid URL"));
3014-
3009+
THROW_ERR_INVALID_URL(isolate, "Invalid URL");
30153010
return;
30163011
}
30173012

3018-
if (!node::url::FileURLToPath(env, base_url.value(), module_base)) return;
3013+
module_base = node::url::FileURLToPath(env, *base_url);
3014+
if (!module_base.has_value()) {
3015+
return;
3016+
}
30193017
} else {
3020-
std::string err_arg_message =
3021-
"The \"base\" argument must be of type string or an instance of URL.";
3022-
env->isolate()->ThrowException(
3023-
ERR_INVALID_ARG_TYPE(env->isolate(), err_arg_message.c_str()));
3018+
THROW_ERR_INVALID_ARG_TYPE(
3019+
isolate,
3020+
"The \"base\" argument must be of type string or an instance of URL.");
30243021
return;
30253022
}
30263023

3027-
env->isolate()->ThrowException(
3028-
ERR_MODULE_NOT_FOUND(env->isolate(),
3029-
"Cannot find package '%s' imported from %s",
3030-
module_path,
3031-
module_base));
3024+
THROW_ERR_MODULE_NOT_FOUND(isolate,
3025+
"Cannot find package '%s' imported from %s",
3026+
*module_path,
3027+
*module_base);
30323028
}
30333029

30343030
void BindingData::MemoryInfo(MemoryTracker* tracker) const {

‎src/node_url.cc‎

Lines changed: 21 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -437,16 +437,11 @@ std::string FromFilePath(std::string_view file_path) {
437437
return ada::href_from_file(escaped_file_path);
438438
}
439439

440-
bool FileURLToPath(Environment* env,
441-
const ada::url_aggregator& file_url,
442-
/* The linter can't detect the assign for result_file_path
443-
So we need to ignore since it suggest to put const */
444-
// NOLINTNEXTLINE(runtime/references)
445-
std::string& result_file_path) {
440+
std::optional<std::string> FileURLToPath(Environment* env,
441+
const ada::url_aggregator& file_url) {
446442
if (file_url.type != ada::scheme::FILE) {
447-
env->isolate()->ThrowException(ERR_INVALID_URL_SCHEME(env->isolate()));
448-
449-
return false;
443+
THROW_ERR_INVALID_URL_SCHEME(env->isolate());
444+
return std::nullopt;
450445
}
451446

452447
std::string_view pathname = file_url.get_pathname();
@@ -479,11 +474,10 @@ bool FileURLToPath(Environment* env,
479474

480475
if (!is_slash && !is_forward_slash) continue;
481476

482-
env->isolate()->ThrowException(ERR_INVALID_FILE_URL_PATH(
477+
THROW_ERR_INVALID_FILE_URL_PATH(
483478
env->isolate(),
484-
"File URL path must not include encoded \\ or / characters"));
485-
486-
return false;
479+
"File URL path must not include encoded \\ or / characters");
480+
return std::nullopt;
487481
}
488482

489483
std::string_view hostname = file_url.get_hostname();
@@ -497,37 +491,29 @@ bool FileURLToPath(Environment* env,
497491
// about percent encoding because the URL parser will have
498492
// already taken care of that for us. Note that this only
499493
// causes IDNs with an appropriate `xn--` prefix to be decoded.
500-
result_file_path =
501-
"\\\\" + ada::unicode::to_unicode(hostname) + decoded_pathname;
502-
503-
return true;
494+
return "\\\\" + ada::unicode::to_unicode(hostname) + decoded_pathname;
504495
}
505496

506497
char letter = decoded_pathname[1] | 0x20;
507498
char sep = decoded_pathname[2];
508499

509500
// a..z A..Z
510501
if (letter < 'a' || letter > 'z' || sep != ':') {
511-
env->isolate()->ThrowException(ERR_INVALID_FILE_URL_PATH(
512-
env->isolate(), "File URL path must be absolute"));
513-
514-
return false;
502+
THROW_ERR_INVALID_FILE_URL_PATH(env->isolate(),
503+
"File URL path must be absolute");
504+
return std::nullopt;
515505
}
516506

517-
result_file_path = decoded_pathname.substr(1);
518-
519-
return true;
507+
return decoded_pathname.substr(1);
520508
#else // _WIN32
521509
std::string_view hostname = file_url.get_hostname();
522510

523511
if (hostname.size() > 0) {
524-
std::string error_message =
525-
std::string("File URL host must be \"localhost\" or empty on ") +
526-
std::string(per_process::metadata.platform);
527-
env->isolate()->ThrowException(
528-
ERR_INVALID_FILE_URL_HOST(env->isolate(), error_message.c_str()));
529-
530-
return false;
512+
THROW_ERR_INVALID_FILE_URL_HOST(
513+
env->isolate(),
514+
"File URL host must be \"localhost\" or empty on ",
515+
std::string(per_process::metadata.platform));
516+
return std::nullopt;
531517
}
532518

533519
size_t first_percent = std::string::npos;
@@ -539,17 +525,14 @@ bool FileURLToPath(Environment* env,
539525
}
540526

541527
if (pathname[i + 1] == '2' && (pathname[i + 2] | 0x20) == 102) {
542-
env->isolate()->ThrowException(ERR_INVALID_FILE_URL_PATH(
528+
THROW_ERR_INVALID_FILE_URL_PATH(
543529
env->isolate(),
544-
"File URL path must not include encoded / characters"));
545-
546-
return false;
530+
"File URL path must not include encoded / characters");
531+
return std::nullopt;
547532
}
548533
}
549534

550-
result_file_path = ada::unicode::percent_decode(pathname, first_percent);
551-
552-
return true;
535+
return ada::unicode::percent_decode(pathname, first_percent);
553536
#endif // _WIN32
554537
}
555538

‎src/node_url.h‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
#include "v8-fast-api-calls.h"
1313
#include "v8.h"
1414

15+
#include <optional>
1516
#include <string>
1617

1718
namespace node {
@@ -82,12 +83,9 @@ class BindingData : public SnapshotableObject {
8283
};
8384

8485
std::string FromFilePath(std::string_view file_path);
85-
bool FileURLToPath(Environment* env,
86-
const ada::url_aggregator& file_url,
87-
/* The linter can't detect the assign for result_file_path
88-
So we need to ignore since it suggest to put const */
89-
// NOLINTNEXTLINE(runtime/references)
90-
std::string& result_file_path);
86+
std::optional<std::string> FileURLToPath(Environment* env,
87+
const ada::url_aggregator& file_url);
88+
9189
} // namespace url
9290

9391
} // namespace node

0 commit comments

Comments
 (0)