Skip to content

Commit

Permalink
src: don't use locale-sensitive strcasecmp()
Browse files Browse the repository at this point in the history
strcasecmp() is affected by the current locale as configured through
e.g. the LC_ALL environment variable and the setlocale() libc function.

It can result in unpredictable results across systems so replace it with
a function that isn't susceptible to that.

PR-URL: #6582
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
bnoordhuis authored and Myles Borins committed Jul 12, 2016
1 parent 8a13abf commit f9677bb
Show file tree
Hide file tree
Showing 5 changed files with 50 additions and 19 deletions.
27 changes: 13 additions & 14 deletions src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@
#if defined(_MSC_VER)
#include <direct.h>
#include <io.h>
#define strcasecmp _stricmp
#define getpid GetCurrentProcessId
#define umask _umask
typedef int mode_t;
Expand Down Expand Up @@ -1350,35 +1349,35 @@ enum encoding ParseEncoding(const char* encoding,
break;
}

if (strcasecmp(encoding, "utf8") == 0) {
if (StringEqualNoCase(encoding, "utf8")) {
return UTF8;
} else if (strcasecmp(encoding, "utf-8") == 0) {
} else if (StringEqualNoCase(encoding, "utf-8")) {
return UTF8;
} else if (strcasecmp(encoding, "ascii") == 0) {
} else if (StringEqualNoCase(encoding, "ascii")) {
return ASCII;
} else if (strcasecmp(encoding, "base64") == 0) {
} else if (StringEqualNoCase(encoding, "base64")) {
return BASE64;
} else if (strcasecmp(encoding, "ucs2") == 0) {
} else if (StringEqualNoCase(encoding, "ucs2")) {
return UCS2;
} else if (strcasecmp(encoding, "ucs-2") == 0) {
} else if (StringEqualNoCase(encoding, "ucs-2")) {
return UCS2;
} else if (strcasecmp(encoding, "utf16le") == 0) {
} else if (StringEqualNoCase(encoding, "utf16le")) {
return UCS2;
} else if (strcasecmp(encoding, "utf-16le") == 0) {
} else if (StringEqualNoCase(encoding, "utf-16le")) {
return UCS2;
} else if (strcasecmp(encoding, "binary") == 0) {
} else if (StringEqualNoCase(encoding, "binary")) {
return BINARY;
} else if (strcasecmp(encoding, "buffer") == 0) {
} else if (StringEqualNoCase(encoding, "buffer")) {
return BUFFER;
} else if (strcasecmp(encoding, "hex") == 0) {
} else if (StringEqualNoCase(encoding, "hex")) {
return HEX;
} else if (strcasecmp(encoding, "raw") == 0) {
} else if (StringEqualNoCase(encoding, "raw") == 0) {
if (!no_deprecation) {
fprintf(stderr, "'raw' (array of integers) has been removed. "
"Use 'binary'.\n");
}
return BINARY;
} else if (strcasecmp(encoding, "raws") == 0) {
} else if (StringEqualNoCase(encoding, "raws") == 0) {
if (!no_deprecation) {
fprintf(stderr, "'raws' encoding has been renamed to 'binary'. "
"Please update your code.\n");
Expand Down
6 changes: 1 addition & 5 deletions src/node_crypto.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,6 @@
#include <stdlib.h>
#include <string.h>

#if defined(_MSC_VER)
#define strcasecmp _stricmp
#endif

#if OPENSSL_VERSION_NUMBER >= 0x10000000L
#define OPENSSL_CONST const
#else
Expand Down Expand Up @@ -4232,7 +4228,7 @@ void DiffieHellman::DiffieHellmanGroup(
for (size_t i = 0; i < arraysize(modp_groups); ++i) {
const modp_group* it = modp_groups + i;

if (strcasecmp(*group_name, it->name) != 0)
if (!StringEqualNoCase(*group_name, it->name))
continue;

initialized = diffieHellman->Init(it->prime,
Expand Down
12 changes: 12 additions & 0 deletions src/util-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,19 @@ void SwapBytes(uint16_t* dst, const uint16_t* src, size_t buflen) {
dst[i] = (src[i] << 8) | (src[i] >> 8);
}

char ToLower(char c) {
return c >= 'A' && c <= 'Z' ? c + ('a' - 'A') : c;
}

bool StringEqualNoCase(const char* a, const char* b) {
do {
if (*a == '\0')
return *b == '\0';
if (*b == '\0')
return *a == '\0';
} while (ToLower(*a++) == ToLower(*b++));
return false;
}

} // namespace node

Expand Down
6 changes: 6 additions & 0 deletions src/util.h
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,12 @@ inline TypeName* Unwrap(v8::Local<v8::Object> object);

inline void SwapBytes(uint16_t* dst, const uint16_t* src, size_t buflen);

// tolower() is locale-sensitive. Use ToLower() instead.
inline char ToLower(char c);

// strcasecmp() is locale-sensitive. Use StringEqualNoCase() instead.
inline bool StringEqualNoCase(const char* a, const char* b);

class Utf8Value {
public:
explicit Utf8Value(v8::Isolate* isolate, v8::Local<v8::Value> value);
Expand Down
18 changes: 18 additions & 0 deletions test/cctest/util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -56,3 +56,21 @@ TEST(UtilTest, ListHead) {
EXPECT_TRUE(list.IsEmpty());
EXPECT_FALSE(list.begin() != list.end());
}

TEST(UtilTest, StringEqualNoCase) {
using node::StringEqualNoCase;
EXPECT_FALSE(StringEqualNoCase("a", "b"));
EXPECT_TRUE(StringEqualNoCase("", ""));
EXPECT_TRUE(StringEqualNoCase("equal", "equal"));
EXPECT_TRUE(StringEqualNoCase("equal", "EQUAL"));
EXPECT_TRUE(StringEqualNoCase("EQUAL", "EQUAL"));
EXPECT_FALSE(StringEqualNoCase("equal", "equals"));
EXPECT_FALSE(StringEqualNoCase("equals", "equal"));
}

TEST(UtilTest, ToLower) {
using node::ToLower;
EXPECT_EQ('0', ToLower('0'));
EXPECT_EQ('a', ToLower('a'));
EXPECT_EQ('a', ToLower('A'));
}

0 comments on commit f9677bb

Please sign in to comment.