Skip to content

Commit

Permalink
src: minor cleanups to node_url.cc
Browse files Browse the repository at this point in the history 
- 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

Backport-PR-URL: #18324
PR-URL: #17470
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
  • Loading branch information
@addaleax @MylesBorins
addaleax authored and MylesBorins committed Feb 13, 2018
1 parent 1b68986 commit d66f469
Showing 1 changed file with 29 additions and 32 deletions.
61 changes: 29 additions & 32 deletions src/node_url.cc
View file
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

0 comments on commit d66f469

Please sign in to comment.