Skip to content

Commit

Permalink
src: fix erroneous fallthrough in ParseEncoding()
Browse files Browse the repository at this point in the history
A missing 'break' statement unintentionally allowed "linary"
and "luffer" as alternatives for "binary" and "buffer".

Regression introduced in commit 54cc721 ("buffer: introduce latin1
encoding term".)

PR-URL: #7262
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
  • Loading branch information
bnoordhuis authored and addaleax committed Aug 8, 2016
1 parent a059aea commit 7ba0f86
Show file tree
Hide file tree
Showing 5 changed files with 66 additions and 1 deletion.
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ ADDONS_BINDING_GYPS := \

ADDONS_BINDING_SOURCES := \
$(filter-out test/addons/??_*/*.cc, $(wildcard test/addons/*/*.cc)) \
$(filter-out test/addons/??_*/*.h, $(wildcard test/addons/*/*.h)) \
$(filter-out test/addons/??_*/*.h, $(wildcard test/addons/*/*.h))

# Implicitly depends on $(NODE_EXE), see the build-addons rule for rationale.
# Depends on node-gyp package.json so that build-addons is (re)executed when
Expand Down
1 change: 1 addition & 0 deletions src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1416,6 +1416,7 @@ enum encoding ParseEncoding(const char* encoding,
if (strncmp(encoding + 2, "tin1", 4) == 0)
return LATIN1;
}
break;
case 'b':
// binary
if (encoding[1] == 'i') {
Expand Down
36 changes: 36 additions & 0 deletions test/addons/parse-encoding/binding.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
#include "node.h"
#include "v8.h"

namespace {

#define ENCODING_MAP(V) \
V(ASCII) \
V(BASE64) \
V(BUFFER) \
V(HEX) \
V(LATIN1) \
V(UCS2) \
V(UTF8) \

void ParseEncoding(const v8::FunctionCallbackInfo<v8::Value>& args) {
const node::encoding encoding =
node::ParseEncoding(args.GetIsolate(), args[0],
static_cast<node::encoding>(-1));
const char* encoding_name = "UNKNOWN";
#define V(name) if (encoding == node::name) encoding_name = #name;
ENCODING_MAP(V)
#undef V
auto encoding_string =
v8::String::NewFromUtf8(args.GetIsolate(), encoding_name,
v8::NewStringType::kNormal)
.ToLocalChecked();
args.GetReturnValue().Set(encoding_string);
}

void Initialize(v8::Local<v8::Object> target) {
NODE_SET_METHOD(target, "parseEncoding", ParseEncoding);
}

} // anonymous namespace

NODE_MODULE(binding, Initialize);
9 changes: 9 additions & 0 deletions test/addons/parse-encoding/binding.gyp
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
'targets': [
{
'target_name': 'binding',
'defines': [ 'V8_DEPRECATION_WARNINGS=1' ],
'sources': [ 'binding.cc' ]
}
]
}
19 changes: 19 additions & 0 deletions test/addons/parse-encoding/test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
'use strict';

require('../../common');
const assert = require('assert');
const { parseEncoding } = require('./build/Release/binding');

assert.strictEqual(parseEncoding(''), 'UNKNOWN');

assert.strictEqual(parseEncoding('ascii'), 'ASCII');
assert.strictEqual(parseEncoding('base64'), 'BASE64');
assert.strictEqual(parseEncoding('binary'), 'LATIN1');
assert.strictEqual(parseEncoding('buffer'), 'BUFFER');
assert.strictEqual(parseEncoding('hex'), 'HEX');
assert.strictEqual(parseEncoding('latin1'), 'LATIN1');
assert.strictEqual(parseEncoding('ucs2'), 'UCS2');
assert.strictEqual(parseEncoding('utf8'), 'UTF8');

assert.strictEqual(parseEncoding('linary'), 'UNKNOWN');
assert.strictEqual(parseEncoding('luffer'), 'UNKNOWN');

1 comment on commit 7ba0f86

@Fishrock123
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR-URL: #8022

Please sign in to comment.