Skip to content

Commit 078316c

Browse files
TimothyGuevanlucas
TimothyGu
authored and
evanlucas
committed
src: WHATWG URL C++ parser cleanup
- Clarify port state - Remove scheme flag - Clarify URL_FLAG_TERMINATED PR-URL: #12507 Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent c7de98a commit 078316c

File tree

2 files changed

+37
-30
lines changed

2 files changed

+37
-30
lines changed

‎src/node_url.cc‎

Lines changed: 31 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -494,7 +494,9 @@ namespace url {
494494
if (flags->IsInt32())
495495
base->flags = flags->Int32Value(context).FromJust();
496496

497-
GET_AND_SET(env, base_obj, scheme, base, URL_FLAGS_HAS_SCHEME);
497+
Local<Value> scheme = GET(env, base_obj, "scheme");
498+
base->scheme = Utf8Value(env->isolate(), scheme).out();
499+
498500
GET_AND_SET(env, base_obj, username, base, URL_FLAGS_HAS_USERNAME);
499501
GET_AND_SET(env, base_obj, password, base, URL_FLAGS_HAS_PASSWORD);
500502
GET_AND_SET(env, base_obj, host, base, URL_FLAGS_HAS_HOST);
@@ -644,7 +646,7 @@ namespace url {
644646
state = kNoScheme;
645647
continue;
646648
} else {
647-
url->flags |= URL_FLAGS_TERMINATED;
649+
url->flags |= URL_FLAGS_FAILED;
648650
return;
649651
}
650652
break;
@@ -654,10 +656,12 @@ namespace url {
654656
p++;
655657
continue;
656658
} else if (ch == ':' || (has_state_override && ch == kEOL)) {
657-
buffer += ':';
658659
if (buffer.size() > 0) {
659-
url->flags |= URL_FLAGS_HAS_SCHEME;
660+
buffer += ':';
660661
url->scheme = buffer;
662+
} else if (has_state_override) {
663+
url->flags |= URL_FLAGS_TERMINATED;
664+
return;
661665
}
662666
if (IsSpecial(url->scheme)) {
663667
url->flags |= URL_FLAGS_SPECIAL;
@@ -672,7 +676,6 @@ namespace url {
672676
state = kFile;
673677
} else if (special &&
674678
has_base &&
675-
base->flags & URL_FLAGS_HAS_SCHEME &&
676679
url->scheme == base->scheme) {
677680
state = kSpecialRelativeOrAuthority;
678681
} else if (special) {
@@ -692,7 +695,7 @@ namespace url {
692695
p = input;
693696
continue;
694697
} else {
695-
url->flags |= URL_FLAGS_TERMINATED;
698+
url->flags |= URL_FLAGS_FAILED;
696699
return;
697700
}
698701
break;
@@ -702,7 +705,6 @@ namespace url {
702705
url->flags |= URL_FLAGS_FAILED;
703706
return;
704707
} else if (cannot_be_base && ch == '#') {
705-
url->flags |= URL_FLAGS_HAS_SCHEME;
706708
url->scheme = base->scheme;
707709
if (IsSpecial(url->scheme)) {
708710
url->flags |= URL_FLAGS_SPECIAL;
@@ -725,12 +727,10 @@ namespace url {
725727
url->flags |= URL_FLAGS_CANNOT_BE_BASE;
726728
state = kFragment;
727729
} else if (has_base &&
728-
base->flags & URL_FLAGS_HAS_SCHEME &&
729730
base->scheme != "file:") {
730731
state = kRelative;
731732
continue;
732733
} else {
733-
url->flags |= URL_FLAGS_HAS_SCHEME;
734734
url->scheme = "file:";
735735
url->flags |= URL_FLAGS_SPECIAL;
736736
special = true;
@@ -756,7 +756,6 @@ namespace url {
756756
}
757757
break;
758758
case kRelative:
759-
url->flags |= URL_FLAGS_HAS_SCHEME;
760759
url->scheme = base->scheme;
761760
if (IsSpecial(url->scheme)) {
762761
url->flags |= URL_FLAGS_SPECIAL;
@@ -951,7 +950,6 @@ namespace url {
951950
buffer.clear();
952951
state = kPort;
953952
if (state_override == kHostname) {
954-
url->flags |= URL_FLAGS_TERMINATED;
955953
return;
956954
}
957955
} else if (ch == kEOL ||
@@ -972,7 +970,6 @@ namespace url {
972970
buffer.clear();
973971
state = kPathStart;
974972
if (has_state_override) {
975-
url->flags |= URL_FLAGS_TERMINATED;
976973
return;
977974
}
978975
} else {
@@ -996,13 +993,26 @@ namespace url {
996993
int port = 0;
997994
for (size_t i = 0; i < buffer.size(); i++)
998995
port = port * 10 + buffer[i] - '0';
999-
if (port >= 0 && port <= 0xffff) {
1000-
url->port = NormalizePort(url->scheme, port);
1001-
} else if (!has_state_override) {
1002-
url->flags |= URL_FLAGS_FAILED;
996+
if (port < 0 || port > 0xffff) {
997+
// TODO(TimothyGu): This hack is currently needed for the host
998+
// setter since it needs access to hostname if it is valid, and
999+
// if the FAILED flag is set the entire response to JS layer
1000+
// will be empty.
1001+
if (state_override == kHost)
1002+
url->port = -1;
1003+
else
1004+
url->flags |= URL_FLAGS_FAILED;
10031005
return;
10041006
}
1007+
url->port = NormalizePort(url->scheme, port);
10051008
buffer.clear();
1009+
} else if (has_state_override) {
1010+
// TODO(TimothyGu): Similar case as above.
1011+
if (state_override == kHost)
1012+
url->port = -1;
1013+
else
1014+
url->flags |= URL_FLAGS_TERMINATED;
1015+
return;
10061016
}
10071017
state = kPathStart;
10081018
continue;
@@ -1014,7 +1024,6 @@ namespace url {
10141024
case kFile:
10151025
base_is_file = (
10161026
has_base &&
1017-
base->flags & URL_FLAGS_HAS_SCHEME &&
10181027
base->scheme == "file:");
10191028
switch (ch) {
10201029
case kEOL:
@@ -1097,7 +1106,6 @@ namespace url {
10971106
state = kFileHost;
10981107
} else {
10991108
if (has_base &&
1100-
base->flags & URL_FLAGS_HAS_SCHEME &&
11011109
base->scheme == "file:" &&
11021110
base->flags & URL_FLAGS_HAS_PATH &&
11031111
base->path.size() > 0 &&
@@ -1158,8 +1166,7 @@ namespace url {
11581166
url->path.push_back("");
11591167
}
11601168
} else {
1161-
if (url->flags & URL_FLAGS_HAS_SCHEME &&
1162-
url->scheme == "file:" &&
1169+
if (url->scheme == "file:" &&
11631170
url->path.empty() &&
11641171
buffer.size() == 2 &&
11651172
WINDOWS_DRIVE_LETTER(buffer[0], buffer[1])) {
@@ -1233,8 +1240,7 @@ namespace url {
12331240
const struct url_data* url) {
12341241
Isolate* isolate = env->isolate();
12351242
argv[ARG_FLAGS] = Integer::NewFromUnsigned(isolate, url->flags);
1236-
if (url->flags & URL_FLAGS_HAS_SCHEME)
1237-
argv[ARG_PROTOCOL] = OneByteString(isolate, url->scheme.c_str());
1243+
argv[ARG_PROTOCOL] = OneByteString(isolate, url->scheme.c_str());
12381244
if (url->flags & URL_FLAGS_HAS_USERNAME)
12391245
argv[ARG_USERNAME] = UTF8STRING(isolate, url->username);
12401246
if (url->flags & URL_FLAGS_HAS_PASSWORD)
@@ -1275,7 +1281,9 @@ namespace url {
12751281
HarvestBase(env, &base, base_obj.As<Object>());
12761282

12771283
URL::Parse(input, len, state_override, &url, &base, has_base);
1278-
if (url.flags & URL_FLAGS_INVALID_PARSE_STATE)
1284+
if ((url.flags & URL_FLAGS_INVALID_PARSE_STATE) ||
1285+
((state_override != kUnknownState) &&
1286+
(url.flags & URL_FLAGS_TERMINATED)))
12791287
return;
12801288

12811289
// Define the return value placeholders

‎src/node_url.h‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -451,13 +451,12 @@ static inline void PercentDecode(const char* input,
451451
XX(URL_FLAGS_INVALID_PARSE_STATE, 0x04) \
452452
XX(URL_FLAGS_TERMINATED, 0x08) \
453453
XX(URL_FLAGS_SPECIAL, 0x10) \
454-
XX(URL_FLAGS_HAS_SCHEME, 0x20) \
455-
XX(URL_FLAGS_HAS_USERNAME, 0x40) \
456-
XX(URL_FLAGS_HAS_PASSWORD, 0x80) \
457-
XX(URL_FLAGS_HAS_HOST, 0x100) \
458-
XX(URL_FLAGS_HAS_PATH, 0x200) \
459-
XX(URL_FLAGS_HAS_QUERY, 0x400) \
460-
XX(URL_FLAGS_HAS_FRAGMENT, 0x800)
454+
XX(URL_FLAGS_HAS_USERNAME, 0x20) \
455+
XX(URL_FLAGS_HAS_PASSWORD, 0x40) \
456+
XX(URL_FLAGS_HAS_HOST, 0x80) \
457+
XX(URL_FLAGS_HAS_PATH, 0x100) \
458+
XX(URL_FLAGS_HAS_QUERY, 0x200) \
459+
XX(URL_FLAGS_HAS_FRAGMENT, 0x400)
461460

462461
#define ARGS(XX) \
463462
XX(ARG_FLAGS) \

0 commit comments

Comments
 (0)