Skip to content

[v6.x] src: some refactoring of node_url.cc #18324

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
addaleax wants to merge 6 commits into from
Closed

[v6.x] src: some refactoring of node_url.cc #18324

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
0cdcb67
src: minor cleanups to node_url.cc
addaleax Dec 5, 2017
08b2482
src: move url internals into anonymous namespace
addaleax Dec 5, 2017
2de0731
src: make url host a proper C++ class
addaleax Dec 5, 2017
b70a4b4
src: use correct OOB check for IPv6 parsing
addaleax Dec 5, 2017
92aa327
src: dumb down code by removing std::move
addaleax Jan 23, 2018
71b5a61
[squash] make linter happy
addaleax Jan 24, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
src: minor cleanups to node_url.cc 
- Remove pointless pointers
- Make `WriteHost` take a const argument so that it’s functionality
  is clear from the signature
- Make `FindLongestZeroSequence` templated to accommodate the constness
  in `WriteHost` and because using `uint16_t` is an articifial,
  unnecessary restriction
- Remove string copying when no copies are needed
- Make `PercentDecode` just return its return value
- Make `ParseHost` (string-only version) take its constant argument
  as a constant reference

PR-URL: #17470
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
  • Loading branch information
@addaleax
addaleax committed Jan 23, 2018
commit 0cdcb675fa370a512927e84f9e5160f90dd1fcc4
61 changes: 29 additions & 32 deletions src/node_url.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -506,12 +506,11 @@ static inline unsigned hex2bin(const T ch) {
return static_cast<unsigned>(-1);
}

static inline void PercentDecode(const char* input,
size_t len,
std::string* dest) {
inline std::string PercentDecode(const char* input, size_t len) {
std::string dest;
if (len == 0)
return;
dest->reserve(len);
return dest;
dest.reserve(len);
const char* pointer = input;
const char* end = input + len;

Expand All @@ -522,17 +521,18 @@ static inline void PercentDecode(const char* input,
(ch == '%' &&
(!IsASCIIHexDigit(pointer[1]) ||
!IsASCIIHexDigit(pointer[2])))) {
*dest += ch;
dest += ch;
pointer++;
continue;
} else {
unsigned a = hex2bin(pointer[1]);
unsigned b = hex2bin(pointer[2]);
char c = static_cast<char>(a * 16 + b);
*dest += c;
dest += c;
pointer += 3;
}
}
return dest;
}

#define SPECIALS(XX) \
Expand Down Expand Up @@ -860,7 +860,7 @@ static url_host_type ParseHost(url_host* host,
return ParseOpaqueHost(host, input, length);

// First, we have to percent decode
PercentDecode(input, length, &decoded);
decoded = PercentDecode(input, length);

// Then we have to punycode toASCII
if (!ToASCII(decoded, &decoded))
Expand Down Expand Up @@ -894,13 +894,13 @@ static url_host_type ParseHost(url_host* host,

// Locates the longest sequence of 0 segments in an IPv6 address
// in order to use the :: compression when serializing
static inline uint16_t* FindLongestZeroSequence(uint16_t* values,
size_t len) {
uint16_t* start = values;
uint16_t* end = start + len;
uint16_t* result = nullptr;
template<typename T>
static inline T* FindLongestZeroSequence(T* values, size_t len) {
T* start = values;
T* end = start + len;
T* result = nullptr;

uint16_t* current = nullptr;
T* current = nullptr;
unsigned counter = 0, longest = 1;

while (start < end) {
Expand All @@ -923,7 +923,7 @@ static inline uint16_t* FindLongestZeroSequence(uint16_t* values,
return result;
}

static url_host_type WriteHost(url_host* host, std::string* dest) {
static url_host_type WriteHost(const url_host* host, std::string* dest) {
dest->clear();
switch (host->type) {
case HOST_TYPE_DOMAIN:
Expand All @@ -934,8 +934,7 @@ static url_host_type WriteHost(url_host* host, std::string* dest) {
uint32_t value = host->value.ipv4;
for (int n = 0; n < 4; n++) {
char buf[4];
char* buffer = buf;
snprintf(buffer, sizeof(buf), "%d", value % 256);
snprintf(buf, sizeof(buf), "%d", value % 256);
dest->insert(0, buf);
if (n < 3)
dest->insert(0, 1, '.');
Expand All @@ -946,12 +945,12 @@ static url_host_type WriteHost(url_host* host, std::string* dest) {
case HOST_TYPE_IPV6: {
dest->reserve(41);
*dest+= '[';
uint16_t* start = &host->value.ipv6[0];
uint16_t* compress_pointer =
const uint16_t* start = &host->value.ipv6[0];
const uint16_t* compress_pointer =
FindLongestZeroSequence(start, 8);
bool ignore0 = false;
for (int n = 0; n <= 7; n++) {
uint16_t* piece = &host->value.ipv6[n];
const uint16_t* piece = &host->value.ipv6[n];
if (ignore0 && *piece == 0)
continue;
else if (ignore0)
Expand All @@ -962,8 +961,7 @@ static url_host_type WriteHost(url_host* host, std::string* dest) {
continue;
}
char buf[5];
char* buffer = buf;
snprintf(buffer, sizeof(buf), "%x", *piece);
snprintf(buf, sizeof(buf), "%x", *piece);
*dest += buf;
if (n < 7)
*dest += ':';
Expand All @@ -980,16 +978,16 @@ static url_host_type WriteHost(url_host* host, std::string* dest) {
return host->type;
}

static bool ParseHost(std::string* input,
static bool ParseHost(const std::string& input,
std::string* output,
bool is_special,
bool unicode = false) {
if (input->length() == 0) {
if (input.length() == 0) {
output->clear();
return true;
}
url_host host{{""}, HOST_TYPE_DOMAIN};
ParseHost(&host, input->c_str(), input->length(), is_special, unicode);
ParseHost(&host, input.c_str(), input.length(), is_special, unicode);
if (host.type == HOST_TYPE_FAILED)
return false;
WriteHost(&host, output);
Expand Down Expand Up @@ -1092,7 +1090,7 @@ static inline void HarvestContext(Environment* env,
}

// Single dot segment can be ".", "%2e", or "%2E"
static inline bool IsSingleDotSegment(std::string str) {
static inline bool IsSingleDotSegment(const std::string& str) {
switch (str.size()) {
case 1:
return str == ".";
Expand All @@ -1108,7 +1106,7 @@ static inline bool IsSingleDotSegment(std::string str) {
// Double dot segment can be:
// "..", ".%2e", ".%2E", "%2e.", "%2E.",
// "%2e%2e", "%2E%2E", "%2e%2E", or "%2E%2e"
static inline bool IsDoubleDotSegment(std::string str) {
static inline bool IsDoubleDotSegment(const std::string& str) {
switch (str.size()) {
case 2:
return str == "..";
Expand Down Expand Up @@ -1542,7 +1540,7 @@ void URL::Parse(const char* input,
return;
}
url->flags |= URL_FLAGS_HAS_HOST;
if (!ParseHost(&buffer, &url->host, special)) {
if (!ParseHost(buffer, &url->host, special)) {
url->flags |= URL_FLAGS_FAILED;
return;
}
Expand All @@ -1569,7 +1567,7 @@ void URL::Parse(const char* input,
return;
}
url->flags |= URL_FLAGS_HAS_HOST;
if (!ParseHost(&buffer, &url->host, special)) {
if (!ParseHost(buffer, &url->host, special)) {
url->flags |= URL_FLAGS_FAILED;
return;
}
Expand Down Expand Up @@ -1741,7 +1739,7 @@ void URL::Parse(const char* input,
state = kPathStart;
} else {
std::string host;
if (!ParseHost(&buffer, &host, special)) {
if (!ParseHost(buffer, &host, special)) {
url->flags |= URL_FLAGS_FAILED;
return;
}
Expand Down Expand Up @@ -2104,8 +2102,7 @@ std::string URL::ToFilePath() const {
#endif
std::string decoded_path;
for (const std::string& part : context_.path) {
std::string decoded;
PercentDecode(part.c_str(), part.length(), &decoded);
std::string decoded = PercentDecode(part.c_str(), part.length());
for (char& ch : decoded) {
if (is_slash(ch)) {
return "";
Expand Down