src: make multiple improvements to node_url_pattern · nodejs/node@f8910e3
@@ -165,6 +165,10 @@ void URLPattern::New(const FunctionCallbackInfo<Value>& args) {
165165 input = input_buffer.ToString();
166166 } else if (args[0]->IsObject()) {
167167 init = URLPatternInit::FromJsObject(env, args[0].As<Object>());
168+// If init does not have a value here, the implication is that an
169+// error was thrown. Let's allow that to be handled now by returning
170+// early. If we don't, the error thrown will be swallowed.
171+if (!init) return;
168172 } else {
169173THROW_ERR_INVALID_ARG_TYPE(env, "Input must be an object or a string");
170174return;
@@ -180,7 +184,9 @@ void URLPattern::New(const FunctionCallbackInfo<Value>& args) {
180184CHECK(!options.has_value());
181185 options = URLPatternOptions::FromJsObject(env, args[1].As<Object>());
182186if (!options) {
183-THROW_ERR_INVALID_ARG_TYPE(env, "options.ignoreCase must be a boolean");
187+// If options does not have a value, we assume an error was
188+// thrown and scheduled on the isolate. Return early to
189+// propagate it.
184190return;
185191 }
186192 } else {
@@ -197,7 +203,9 @@ void URLPattern::New(const FunctionCallbackInfo<Value>& args) {
197203CHECK(!options.has_value());
198204 options = URLPatternOptions::FromJsObject(env, args[2].As<Object>());
199205if (!options) {
200-THROW_ERR_INVALID_ARG_TYPE(env, "options.ignoreCase must be a boolean");
206+// If options does not have a value, we assume an error was
207+// thrown and scheduled on the isolate. Return early to
208+// propagate it.
201209return;
202210 }
203211 }
@@ -234,73 +242,29 @@ MaybeLocal<Value> URLPattern::URLPatternInit::ToJsObject(
234242auto isolate = env->isolate();
235243auto context = env->context();
236244auto result = Object::New(isolate);
237-if (init.protocol) {
238- Local<Value> protocol;
239-if (!ToV8Value(context, *init.protocol).ToLocal(&protocol) ||
240- result->Set(context, env->protocol_string(), protocol).IsNothing()) {
241-return {};
242- }
243- }
244-if (init.username) {
245- Local<Value> username;
246-if (!ToV8Value(context, *init.username).ToLocal(&username) ||
247- result->Set(context, env->username_string(), username).IsNothing()) {
248-return {};
249- }
250- }
251-if (init.password) {
252- Local<Value> password;
253-if (!ToV8Value(context, *init.password).ToLocal(&password) ||
254- result->Set(context, env->password_string(), password).IsNothing()) {
255-return {};
256- }
257- }
258-if (init.hostname) {
259- Local<Value> hostname;
260-if (!ToV8Value(context, *init.hostname).ToLocal(&hostname) ||
261- result->Set(context, env->hostname_string(), hostname).IsNothing()) {
262-return {};
263- }
264- }
265-if (init.port) {
266- Local<Value> port;
267-if (!ToV8Value(context, *init.port).ToLocal(&port) ||
268- result->Set(context, env->port_string(), port).IsNothing()) {
269-return {};
270- }
271- }
272-if (init.pathname) {
273- Local<Value> pathname;
274-if (!ToV8Value(context, *init.pathname).ToLocal(&pathname) ||
275- result->Set(context, env->pathname_string(), pathname).IsNothing()) {
276-return {};
277- }
278- }
279-if (init.search) {
280- Local<Value> search;
281-if (!ToV8Value(context, *init.search).ToLocal(&search) ||
282- result->Set(context, env->search_string(), search).IsNothing()) {
283-return {};
284- }
285- }
286-if (init.hash) {
287- Local<Value> hash;
288-if (!ToV8Value(context, *init.hash).ToLocal(&hash) ||
289- result->Set(context, env->hash_string(), hash).IsNothing()) {
290-return {};
291- }
292- }
293-if (init.base_url) {
294- Local<Value> base_url;
295-if (!ToV8Value(context, *init.base_url).ToLocal(&base_url) ||
296- result->Set(context, env->base_url_string(), base_url).IsNothing()) {
297-return {};
298- }
245+246+const auto trySet = [&](auto name, const std::optional<std::string>& val) {
247+if (!val) return true;
248+ Local<Value> temp;
249+return ToV8Value(context, *val).ToLocal(&temp) &&
250+ result->Set(context, name, temp).IsJust();
251+ };
252+253+if (!trySet(env->protocol_string(), init.protocol) ||
254+ !trySet(env->username_string(), init.username) ||
255+ !trySet(env->password_string(), init.password) ||
256+ !trySet(env->hostname_string(), init.hostname) ||
257+ !trySet(env->port_string(), init.port) ||
258+ !trySet(env->pathname_string(), init.pathname) ||
259+ !trySet(env->search_string(), init.search) ||
260+ !trySet(env->hash_string(), init.hash) ||
261+ !trySet(env->base_url_string(), init.base_url)) {
262+return {};
299263 }
300264return result;
301265}
302266303-ada::url_pattern_init URLPattern::URLPatternInit::FromJsObject(
267+std::optional<ada::url_pattern_init> URLPattern::URLPatternInit::FromJsObject(
304268 Environment* env, Local<Object> obj) {
305269 ada::url_pattern_init init{};
306270 Local<String> components[] = {
@@ -344,6 +308,10 @@ ada::url_pattern_init URLPattern::URLPatternInit::FromJsObject(
344308 Utf8Value utf8_value(isolate, value);
345309set_parameter(key.ToStringView(), utf8_value.ToStringView());
346310 }
311+ } else {
312+// If ToLocal failed then we assume an error occurred,
313+// bail out early to propagate the error.
314+return std::nullopt;
347315 }
348316 }
349317return init;
@@ -355,12 +323,8 @@ MaybeLocal<Object> URLPattern::URLPatternComponentResult::ToJSObject(
355323auto context = env->context();
356324auto parsed_group = Object::New(isolate);
357325for (const auto& [group_key, group_value] : result.groups) {
358- Local<String> key;
359-if (!String::NewFromUtf8(isolate,
360- group_key.c_str(),
361- NewStringType::kNormal,
362- group_key.size())
363- .ToLocal(&key)) {
326+ Local<Value> key;
327+if (!ToV8Value(context, group_key).ToLocal(&key)) {
364328return {};
365329 }
366330 Local<Value> value;
@@ -371,7 +335,9 @@ MaybeLocal<Object> URLPattern::URLPatternComponentResult::ToJSObject(
371335 } else {
372336 value = Undefined(isolate);
373337 }
374-USE(parsed_group->Set(context, key, value));
338+if (parsed_group->Set(context, key, value).IsNothing()) {
339+return {};
340+ }
375341 }
376342 Local<Value> input;
377343if (!ToV8Value(env->context(), result.input).ToLocal(&input)) {
@@ -418,34 +384,20 @@ MaybeLocal<Value> URLPattern::URLPatternResult::ToJSValue(
418384 LocalVector<Value> values(isolate, arraysize(names));
419385 values[0] = Array::New(isolate, inputs.data(), inputs.size());
420386if (!URLPatternComponentResult::ToJSObject(env, result.protocol)
421- .ToLocal(&values[1])) {
422-return {};
423- }
424-if (!URLPatternComponentResult::ToJSObject(env, result.username)
425- .ToLocal(&values[2])) {
426-return {};
427- }
428-if (!URLPatternComponentResult::ToJSObject(env, result.password)
429- .ToLocal(&values[3])) {
430-return {};
431- }
432-if (!URLPatternComponentResult::ToJSObject(env, result.hostname)
433- .ToLocal(&values[4])) {
434-return {};
435- }
436-if (!URLPatternComponentResult::ToJSObject(env, result.port)
437- .ToLocal(&values[5])) {
438-return {};
439- }
440-if (!URLPatternComponentResult::ToJSObject(env, result.pathname)
441- .ToLocal(&values[6])) {
442-return {};
443- }
444-if (!URLPatternComponentResult::ToJSObject(env, result.search)
445- .ToLocal(&values[7])) {
446-return {};
447- }
448-if (!URLPatternComponentResult::ToJSObject(env, result.hash)
387+ .ToLocal(&values[1]) ||
388+ !URLPatternComponentResult::ToJSObject(env, result.username)
389+ .ToLocal(&values[2]) ||
390+ !URLPatternComponentResult::ToJSObject(env, result.password)
391+ .ToLocal(&values[3]) ||
392+ !URLPatternComponentResult::ToJSObject(env, result.hostname)
393+ .ToLocal(&values[4]) ||
394+ !URLPatternComponentResult::ToJSObject(env, result.port)
395+ .ToLocal(&values[5]) ||
396+ !URLPatternComponentResult::ToJSObject(env, result.pathname)
397+ .ToLocal(&values[6]) ||
398+ !URLPatternComponentResult::ToJSObject(env, result.search)
399+ .ToLocal(&values[7]) ||
400+ !URLPatternComponentResult::ToJSObject(env, result.hash)
449401 .ToLocal(&values[8])) {
450402return {};
451403 }
@@ -461,9 +413,15 @@ URLPattern::URLPatternOptions::FromJsObject(Environment* env,
461413if (obj->Get(env->context(), env->ignore_case_string())
462414 .ToLocal(&ignore_case)) {
463415if (!ignore_case->IsBoolean()) {
416+THROW_ERR_INVALID_ARG_TYPE(env, "options.ignoreCase must be a boolean");
464417return std::nullopt;
465418 }
466419 options.ignore_case = ignore_case->IsTrue();
420+ } else {
421+// If ToLocal returns false, the assumption is that getting the
422+// ignore_case_string threw an error, let's propagate that now
423+// by returning std::nullopt.
424+return std::nullopt;
467425 }
468426return options;
469427}
@@ -553,7 +511,9 @@ void URLPattern::Exec(const FunctionCallbackInfo<Value>& args) {
553511 input_base = input_value.ToString();
554512 input = std::string_view(input_base);
555513 } else if (args[0]->IsObject()) {
556- input = URLPatternInit::FromJsObject(env, args[0].As<Object>());
514+auto maybeInput = URLPatternInit::FromJsObject(env, args[0].As<Object>());
515+if (!maybeInput.has_value()) return;
516+ input = std::move(*maybeInput);
557517 } else {
558518THROW_ERR_INVALID_ARG_TYPE(
559519 env, "URLPattern input needs to be a string or an object");
@@ -594,7 +554,9 @@ void URLPattern::Test(const FunctionCallbackInfo<Value>& args) {
594554 input_base = input_value.ToString();
595555 input = std::string_view(input_base);
596556 } else if (args[0]->IsObject()) {
597- input = URLPatternInit::FromJsObject(env, args[0].As<Object>());
557+auto maybeInput = URLPatternInit::FromJsObject(env, args[0].As<Object>());
558+if (!maybeInput.has_value()) return;
559+ input = std::move(*maybeInput);
598560 } else {
599561THROW_ERR_INVALID_ARG_TYPE(
600562 env, "URLPattern input needs to be a string or an object");