Skip to content

Commit 8e35998

Browse files
committed
fix(runtime): zero the whole return slot for empty callback results
ArgConverter::SetValue wrote a single ffi_arg when a JS callback returned null, undefined or a revoked Proxy, so struct-valued returns handed native code stale bytes past the first word. Callers now pass cif->rtype->size and the empty branch clears the full slot.
1 parent c4df088 commit 8e35998

5 files changed

Lines changed: 29 additions & 9 deletions

File tree

‎NativeScript/runtime/ArgConverter.h‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,8 +70,10 @@ class ArgConverter {
7070
static const ProtocolMeta* FindProtocolMeta(Protocol* protocol);
7171
static void MethodCallback(ffi_cif* cif, void* retValue, void** argValues,
7272
void* userData);
73+
// returnSize is the ABI size of the return slot (cif->rtype->size); an
74+
// empty, null or undefined value zeroes all of it.
7375
static void SetValue(v8::Local<v8::Context> context, void* retValue,
74-
v8::Local<v8::Value> value,
76+
size_t returnSize, v8::Local<v8::Value> value,
7577
const TypeEncoding* typeEncoding);
7678
// Returns (lazily creating) the per-isolate interop.escapeException brand
7779
// private stored in Caches. Empty handle if the isolate cache is invalid.

‎NativeScript/runtime/ArgConverter.mm‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
#include "ArgConverter.h"
22
#include <Foundation/Foundation.h>
3+
#include <algorithm>
34
#include <sstream>
45
#include "DictionaryAdapter.h"
56
#include "Helpers.h"
@@ -434,7 +435,7 @@
434435
if (!success) {
435436
memset(retValue, 0, cif->rtype->size);
436437
} else {
437-
ArgConverter::SetValue(context, retValue, result, data->typeEncoding_);
438+
ArgConverter::SetValue(context, retValue, cif->rtype->size, result, data->typeEncoding_);
438439
}
439440
} else {
440441
memset(retValue, 0, cif->rtype->size);
@@ -446,18 +447,18 @@
446447
}
447448
}
448449

449-
void ArgConverter::SetValue(Local<Context> context, void* retValue, Local<Value> value,
450-
const TypeEncoding* typeEncoding) {
450+
void ArgConverter::SetValue(Local<Context> context, void* retValue, size_t returnSize,
451+
Local<Value> value, const TypeEncoding* typeEncoding) {
451452
if (typeEncoding->type == BinaryTypeEncodingType::VoidEncoding) {
452453
return;
453454
}
454455

455456
// Runs inside an ffi closure, where a C++ throw cannot propagate; a revoked
456-
// proxy returns nil.
457+
// proxy returns nil. libffi sizes the return slot to at least ffi_arg even
458+
// for narrower types, and the full slot must be written.
457459
value = tns::UnwrapProxy(value);
458460
if (value.IsEmpty() || value->IsNullOrUndefined()) {
459-
void* nullPtr = nullptr;
460-
*(ffi_arg*)retValue = (unsigned long)nullPtr;
461+
memset(retValue, 0, std::max(returnSize, sizeof(ffi_arg)));
461462
return;
462463
}
463464

‎NativeScript/runtime/ClassBuilder.mm‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -931,7 +931,7 @@ void ScopeClassNameToIsolate(std::string& name, int isolateId) {
931931
memset(retValue, 0, cif->rtype->size);
932932
} else {
933933
const TypeEncoding* typeEncoding = context->meta_->getter()->encodings()->first();
934-
ArgConverter::SetValue(v8Context, retValue, res, typeEncoding);
934+
ArgConverter::SetValue(v8Context, retValue, cif->rtype->size, res, typeEncoding);
935935
}
936936
}
937937
if (pendingThrow != nil) {

‎NativeScript/runtime/MetadataBuilder.mm‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1220,7 +1220,7 @@ CMethodCall methodCall(context, item->userData_, typeEncoding, args,
12201220
memset(retValue, 0, cif->rtype->size);
12211221
} else {
12221222
const TypeEncoding* typeEncoding = context->meta_->getter()->encodings()->first();
1223-
ArgConverter::SetValue(v8Context, retValue, res, typeEncoding);
1223+
ArgConverter::SetValue(v8Context, retValue, cif->rtype->size, res, typeEncoding);
12241224
}
12251225
}
12261226
if (pendingThrow != nil) {

‎TestRunner/app/tests/ProxyReceiverTests.js‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,23 @@ describe(module.id, function () {
145145
}).toThrowError(TypeError, /not a native object/);
146146
});
147147

148+
it("zeroes a struct return when a callback returns a revoked proxy", function () {
149+
var revocable = Proxy.revocable(CGRectMake(1, 2, 3, 4), {});
150+
revocable.revoke();
151+
152+
[revocable.proxy, null].forEach(function (returned) {
153+
var rect = getStructFromCallback(new interop.FunctionReference(function () {
154+
return returned;
155+
}));
156+
157+
expect(rect instanceof CGRect).toBe(true);
158+
expect(rect.origin.x).toBe(0);
159+
expect(rect.origin.y).toBe(0);
160+
expect(rect.size.width).toBe(0);
161+
expect(rect.size.height).toBe(0);
162+
});
163+
});
164+
148165
it("releases the native counterpart of the proxy target", function () {
149166
var target = NSMutableString.alloc().init();
150167
var proxy = new Proxy(target, {});

0 commit comments

Comments
 (0)