Commit 8f0dff8d authored by Philip Chimento's avatar Philip Chimento 🚮

byteArray: Add compatibility toString property

This overrides, on each Uint8Array returned from an introspected function
or from ByteArray.fromString() or ByteArray.fromGBytes(), the toString()
property with a compatibility shim that preserves the old behaviour of
ByteArray.prototype.toString() while logging a compatibility warning
asking people to fix their code.

This ByteArray.toString() -> Uint8Array.toString() change has had more
fallout in application code than I expected, so it seems better to
preserve backwards compatibility. (The old behaviour was to decode the
byte array into a string with default encoding UTF-8, and the default
behaviour of Uint8Array is to return a string with the decimal digits of
each byte joined with commas. So the effect was that strings like
"97,98,99,100" would show up in UIs where previously "abcd" would have
been printed.

This is only on specific instances, so Uint8Array.prototype.toString()
remains untouched.
parent f55c0123
Pipeline #25820 passed with stages
in 42 minutes and 7 seconds
......@@ -25,6 +25,7 @@
#include "byteArray.h"
#include "gi/boxed.h"
#include "gjs/deprecation.h"
#include "jsapi-util-args.h"
#include "jsapi-wrapper.h"
......@@ -45,22 +46,11 @@ static void bytes_unref_arraybuffer(void* contents, void* user_data) {
}
/* implement toString() with an optional encoding arg */
static bool
to_string_func(JSContext *context,
unsigned argc,
JS::Value *vp)
{
JS::CallArgs argv = JS::CallArgsFromVp(argc, vp);
GjsAutoJSChar encoding;
JS::RootedObject byte_array(context);
static bool to_string_impl(JSContext* context, JS::HandleObject byte_array,
const char* encoding, JS::MutableHandleValue rval) {
bool encoding_is_utf8;
uint8_t* data;
if (!gjs_parse_call_args(context, "toString", argv, "o|s",
"byteArray", &byte_array,
"encoding", &encoding))
return false;
if (encoding) {
/* maybe we should be smarter about utf8 synonyms here.
* doesn't matter much though. encoding_is_utf8 is
......@@ -80,7 +70,7 @@ to_string_func(JSContext *context,
* libmozjs hardwired utf8-to-utf16
*/
return gjs_string_from_utf8_n(context, reinterpret_cast<char*>(data),
len, argv.rval());
len, rval);
} else {
bool ok = false;
gsize bytes_written;
......@@ -109,7 +99,7 @@ to_string_func(JSContext *context,
s = JS_NewUCStringCopyN(context, u16_out, bytes_written / 2);
if (s != NULL) {
ok = true;
argv.rval().setString(s);
rval.setString(s);
}
g_free(u16_str);
......@@ -118,6 +108,35 @@ to_string_func(JSContext *context,
}
}
static bool to_string_func(JSContext* cx, unsigned argc, JS::Value* vp) {
JS::CallArgs args = JS::CallArgsFromVp(argc, vp);
GjsAutoJSChar encoding;
JS::RootedObject byte_array(cx);
if (!gjs_parse_call_args(cx, "toString", args, "o|s", "byteArray",
&byte_array, "encoding", &encoding))
return false;
return to_string_impl(cx, byte_array, encoding, args.rval());
}
/* Workaround to keep existing code compatible. This function is tacked onto
* any Uint8Array instances created in situations where previously a ByteArray
* would have been created. It logs a compatibility warning. */
static bool instance_to_string_func(JSContext* cx, unsigned argc,
JS::Value* vp) {
GJS_GET_THIS(cx, argc, vp, args, this_obj);
GjsAutoJSChar encoding;
_gjs_warn_deprecated_once_per_callsite(
cx, GjsDeprecationMessageId::ByteArrayInstanceToString);
if (!gjs_parse_call_args(cx, "toString", args, "|s", "encoding", &encoding))
return false;
return to_string_impl(cx, this_obj, encoding, args.rval());
}
static bool
to_gbytes_func(JSContext *context,
unsigned argc,
......@@ -227,6 +246,7 @@ from_string_func(JSContext *context,
if (!array_buffer)
return false;
obj = JS_NewUint8ArrayWithBuffer(context, array_buffer, 0, -1);
JS_DefineFunction(context, obj, "toString", instance_to_string_func, 1, 0);
argv.rval().setObject(*obj);
return true;
}
......@@ -264,6 +284,7 @@ from_gbytes_func(JSContext *context,
context, JS_NewUint8ArrayWithBuffer(context, array_buffer, 0, -1));
if (!obj)
return false;
JS_DefineFunction(context, obj, "toString", instance_to_string_func, 1, 0);
argv.rval().setObject(*obj);
return true;
......@@ -275,7 +296,10 @@ JSObject* gjs_byte_array_from_data(JSContext* cx, size_t nbytes, void* data) {
if (!array_buffer)
return nullptr;
return JS_NewUint8ArrayWithBuffer(cx, array_buffer, 0, -1);
JS::RootedObject array(cx,
JS_NewUint8ArrayWithBuffer(cx, array_buffer, 0, -1));
JS_DefineFunction(cx, array, "toString", instance_to_string_func, 1, 0);
return array;
}
JSObject* gjs_byte_array_from_byte_array(JSContext* cx, GByteArray* array) {
......
......@@ -32,6 +32,15 @@
const char* messages[] = {
// None:
"(invalid message)",
// ByteArrayInstanceToString:
"Some code called array.toString() on a Uint8Array instance. Previously "
"this would have interpreted the bytes of the array as a string, but that "
"is nonstandard. In the future this will return the bytes as "
"comma-separated digits. For the time being, the old behavior has been "
"preserved, but please fix your code anyway to explicitly call ByteArray"
".toString(array).\n"
"(Note that array.toString() may have been called implicitly.)",
};
struct DeprecationEntry {
......
......@@ -28,6 +28,7 @@
enum GjsDeprecationMessageId {
None,
ByteArrayInstanceToString,
};
void _gjs_warn_deprecated_once_per_callsite(JSContext* cx,
......
const ByteArray = imports.byteArray;
const {GIMarshallingTests, GLib} = imports.gi;
describe('Byte array', function () {
it('can be created from a string', function () {
......@@ -35,4 +36,27 @@ describe('Byte array', function () {
expect(s.length).toEqual(4);
expect(s).toEqual('abcd');
});
describe('legacy toString() behavior', function () {
beforeEach(function () {
GLib.test_expect_message('Gjs', GLib.LogLevelFlags.LEVEL_WARNING,
'Some code called array.toString()*');
});
it('is preserved when created from a string', function () {
let a = ByteArray.fromString('');
expect(a.toString()).toEqual('');
});
it('is preserved when marshalled from GI', function () {
let a = GIMarshallingTests.bytearray_full_return();
expect(() => a.toString()).toThrowError(TypeError,
/malformed UTF-8 character sequence/);
});
afterEach(function () {
GLib.test_assert_expected_messages_internal('Gjs',
'testByteArray.js', 0, 'testToStringCompatibility');
});
});
});
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment