permission: support URL and Uint8Array as has()/drop() reference · nodejs/node@72717a5 · GitHub
Skip to content

Commit 72717a5

Browse files
nhjbest22nhjbest22
committed
permission: support URL and Uint8Array as has()/drop() reference
normalizeReference() adds URL and Uint8Array support to has()/drop(); the existing string/Buffer behavior is unchanged. BufferValue replaces Utf8Value so a Buffer/TypedArray reference is copied as raw bytes instead of a lossy UTF-8 conversion. Signed-off-by: seungmin Nam <nhjbest22@g.skku.edu>
1 parent b2b2b41 commit 72717a5

4 files changed

Lines changed: 145 additions & 34 deletions

File tree

‎lib/internal/process/permission.js‎

Lines changed: 32 additions & 22 deletions

‎src/permission/permission.cc‎

Lines changed: 24 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -90,9 +90,12 @@ static void Drop(const FunctionCallbackInfo<Value>& args) {
9090
}
9191

9292
if (args.Length() > 1 && !args[1]->IsUndefined()) {
93-
Utf8Value utf8_arg(env->isolate(), args[1]);
94-
if (utf8_arg.length() > 0) {
95-
env->permission()->Drop(env, scope, utf8_arg.ToStringView());
93+
// BufferValue copies raw bytes out of a Buffer/TypedArray as-is instead
94+
// of forcing a (potentially lossy) UTF-8 string conversion, since paths
95+
// are not guaranteed to be valid UTF-8.
96+
BufferValue resource(env->isolate(), args[1]);
97+
if (resource.length() > 0) {
98+
env->permission()->Drop(env, scope, resource.ToStringView());
9699
return;
97100
}
98101
}
@@ -113,13 +116,16 @@ static void Has(const FunctionCallbackInfo<Value>& args) {
113116
}
114117

115118
if (args.Length() > 1 && !args[1]->IsUndefined()) {
116-
Utf8Value utf8_arg(env->isolate(), args[1]);
117-
if (utf8_arg.length() == 0) {
119+
// BufferValue copies raw bytes out of a Buffer/TypedArray as-is instead
120+
// of forcing a (potentially lossy) UTF-8 string conversion, since paths
121+
// are not guaranteed to be valid UTF-8.
122+
BufferValue resource(env->isolate(), args[1]);
123+
if (resource.length() == 0) {
118124
args.GetReturnValue().Set(false);
119125
return;
120126
}
121127
return args.GetReturnValue().Set(
122-
env->permission()->is_granted(env, scope, utf8_arg.ToStringView()));
128+
env->permission()->is_granted(env, scope, resource.ToStringView()));
123129
}
124130

125131
return args.GetReturnValue().Set(env->permission()->is_granted(env, scope));
@@ -174,16 +180,22 @@ static bool FastHasResource(
174180
return false;
175181
}
176182

177-
Local<String> res_str;
178-
if (!resource_arg->ToString(context).ToLocal(&res_str)) {
179-
return false;
183+
// The JS wrapper always calls this with 2 arguments, passing undefined
184+
// when no resource was given, so this path (unlike Has()) has to check
185+
// for that explicitly and fall back to the scope-only check.
186+
if (resource_arg->IsUndefined()) {
187+
return env->permission()->is_granted(env, scope);
180188
}
181-
Utf8Value utf8_res(isolate, res_str);
182-
if (utf8_res.length() == 0) {
189+
190+
// BufferValue is constructed directly from resource_arg (not from a
191+
// pre-converted String) so that a Buffer/TypedArray's raw bytes are
192+
// copied as-is instead of going through a lossy UTF-8 string conversion.
193+
BufferValue resource(isolate, resource_arg);
194+
if (resource.length() == 0) {
183195
return false;
184196
}
185197

186-
return env->permission()->is_granted(env, scope, utf8_res.ToStringView());
198+
return env->permission()->is_granted(env, scope, resource.ToStringView());
187199
}
188200

189201
static CFunction fast_has_methods_[] = {CFunction::Make(FastHas),

‎test/fixtures/permission/has-reference-types.js‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
'use strict';
2+
3+
const assert = require('assert');
4+
const { pathToFileURL } = require('url');
5+
6+
const allowedDir = process.env.ALLOWED_DIR;
7+
const allowedFile = process.env.ALLOWED_FILE;
8+
const deniedFile = process.env.DENIED_FILE;
9+
10+
// file: URL
11+
{
12+
assert.strictEqual(process.permission.has('fs.read', pathToFileURL(allowedFile)), true);
13+
assert.strictEqual(process.permission.has('fs.read', pathToFileURL(deniedFile)), false);
14+
assert.throws(() => {
15+
process.permission.has('fs.read', new URL('https://example.com'));
16+
}, { code: 'ERR_INVALID_URL_SCHEME' });
17+
}
18+
19+
// Non-fs scopes like net ignore the reference, so a non-file: URL must not be rejected here.
20+
{
21+
assert.strictEqual(process.permission.has('net', new URL('https://example.com')), false);
22+
}
23+
24+
// Uint8Array
25+
{
26+
const allowedBytes = new Uint8Array(Buffer.from(allowedFile));
27+
const deniedBytes = new Uint8Array(Buffer.from(deniedFile));
28+
assert.strictEqual(process.permission.has('fs.read', allowedBytes), true);
29+
assert.strictEqual(process.permission.has('fs.read', deniedBytes), false);
30+
31+
// Must respect byteOffset/byteLength, not read from the start of the buffer.
32+
const padded = Buffer.concat([Buffer.from('padding-'), Buffer.from(allowedFile)]);
33+
const view = new Uint8Array(padded.buffer, padded.byteOffset + 8, allowedFile.length);
34+
assert.strictEqual(process.permission.has('fs.read', view), true);
35+
}
36+
37+
// drop() only matches the exact granted path (the directory), not a file inside it.
38+
{
39+
process.permission.drop('fs.read', pathToFileURL(allowedDir));
40+
assert.strictEqual(process.permission.has('fs.read', pathToFileURL(allowedFile)), false);
41+
}

‎test/parallel/test-permission-has-reference-types.js‎

Lines changed: 48 additions & 0 deletions

0 commit comments

Comments
 (0)