Commit 11d96aa9 authored by Philip Chimento's avatar Philip Chimento 🚮 Committed by Philip Chimento

jsapi-util: Rooting-safe gjs_parse_call_args()

This removes the old unused gjs_parse_args(), and moves
gjs_parse_call_args() into new files jsapi-util-args.cpp and
jsapi-util-args.h.

In order to ensure that JSObjects unpacked from gjs_parse_call_args()
land in GC roots, we need to add some type safety to its variable
arguments. Instead of accepting a JSObject** pointer for the "o" format
character, it must accept a JS::MutableHandleObject.

We can do this (and make the whole thing type safe as well) by using C++
templates instead of C varargs. Now we can issue a compile-time error
when an unknown type is passed in as a return location for an argument,
in particular JSObject**.

This also fixes some undefined behaviour: the old signature of
gjs_parse_call_args() placed the varargs parameter right after the
JS::CallArgs& parameter, and using a reference parameter in va_start() is
undefined. Here's a good explanation of what can go wrong:
http://stackoverflow.com/a/222288/172999

https://bugzilla.gnome.org/show_bug.cgi?id=742249
parent 16c6e144
......@@ -92,6 +92,7 @@ gjs_tests_LDADD = \
gjs_tests_SOURCES = \
test/gjs-tests.cpp \
test/gjs-tests-add-funcs.h \
test/gjs-test-call-args.cpp \
test/gjs-test-coverage.cpp \
mock-js-resources.c \
$(NULL)
......
......@@ -82,6 +82,7 @@ libgjs_la_SOURCES = \
gjs/jsapi-private.h \
gjs/jsapi-util.cpp \
gjs/jsapi-dynamic-class.cpp \
gjs/jsapi-util-args.h \
gjs/jsapi-util-error.cpp \
gjs/jsapi-util-string.cpp \
gjs/mem.cpp \
......
......@@ -30,6 +30,7 @@
#include "object.h"
#include "gtype.h"
#include "interface.h"
#include "gjs/jsapi-util-args.h"
#include "arg.h"
#include "repo.h"
#include "gtype.h"
......@@ -2232,8 +2233,7 @@ gjs_hook_up_vfunc(JSContext *cx,
{
JS::CallArgs argv = JS::CallArgsFromVp (argc, vp);
gchar *name;
JS::RootedObject object(cx);
JSObject *function;
JS::RootedObject object(cx), function(cx);
ObjectInstance *priv;
GType gtype, info_gtype;
GIObjectInfo *info;
......@@ -2241,11 +2241,10 @@ gjs_hook_up_vfunc(JSContext *cx,
gpointer implementor_vtable;
GIFieldInfo *field_info;
if (!gjs_parse_call_args(cx, "hook_up_vfunc",
"oso", argv,
"object", object.address(),
"name", &name,
"function", &function))
if (!gjs_parse_call_args(cx, "hook_up_vfunc", argv, "oso",
"object", &object,
"name", &name,
"function", &function))
return false;
if (!do_base_typecheck(cx, object, true))
......@@ -2490,9 +2489,9 @@ gjs_override_property(JSContext *cx,
GParamSpec *new_pspec;
GType gtype;
if (!gjs_parse_call_args(cx, "override_property", "so", args,
if (!gjs_parse_call_args(cx, "override_property", args, "so",
"name", &name,
"type", type.address()))
"type", &type))
return false;
if ((gtype = gjs_gtype_get_actual_gtype(cx, type)) == G_TYPE_INVALID) {
......@@ -2747,7 +2746,8 @@ gjs_register_interface(JSContext *cx,
{
JS::CallArgs args = JS::CallArgsFromVp(argc, vp);
char *name = NULL;
JSObject *constructor, *interfaces, *properties, *module;
JS::RootedObject interfaces(cx), properties(cx);
JSObject *constructor, *module;
guint32 i, n_interfaces, n_properties;
GType *iface_types;
GType interface_type;
......@@ -2767,7 +2767,7 @@ gjs_register_interface(JSContext *cx,
NULL, /* instance_init */
};
if (!gjs_parse_call_args(cx, "register_interface", "soo", args,
if (!gjs_parse_call_args(cx, "register_interface", args, "soo",
"name", &name,
"interfaces", &interfaces,
"properties", &properties))
......@@ -2826,8 +2826,8 @@ gjs_register_type(JSContext *cx,
{
JS::CallArgs argv = JS::CallArgsFromVp (argc, vp);
gchar *name;
JS::RootedObject parent(cx);
JSObject *constructor, *interfaces, *properties, *module;
JS::RootedObject parent(cx), interfaces(cx), properties(cx);
JSObject *constructor, *module;
GType instance_type, parent_type;
GTypeQuery query;
GTypeModule *type_module;
......@@ -2852,12 +2852,11 @@ gjs_register_type(JSContext *cx,
JS_BeginRequest(cx);
if (!gjs_parse_call_args(cx, "register_type",
"osoo", argv,
"parent", parent.address(),
"name", &name,
"interfaces", &interfaces,
"properties", &properties))
if (!gjs_parse_call_args(cx, "register_type", argv, "osoo",
"parent", &parent,
"name", &name,
"interfaces", &interfaces,
"properties", &properties))
goto out;
if (!parent)
......
......@@ -27,6 +27,7 @@
#include "byteArray.h"
#include "gi/boxed.h"
#include "compat.h"
#include "jsapi-util-args.h"
#include <girepository.h>
#include <util/log.h>
......@@ -712,8 +713,8 @@ from_gbytes_func(JSContext *context,
GBytes *gbytes;
ByteArrayInstance *priv;
if (!gjs_parse_call_args(context, "overrides_gbytes_to_array", "o", argv,
"bytes", bytes_obj.address()))
if (!gjs_parse_call_args(context, "overrides_gbytes_to_array", argv, "o",
"bytes", &bytes_obj))
return false;
if (!gjs_typecheck_boxed(context, bytes_obj, NULL, G_TYPE_BYTES, true))
......
......@@ -26,6 +26,7 @@
#include "coverage.h"
#include "coverage-internal.h"
#include "importer.h"
#include "jsapi-util-args.h"
#include "util/error.h"
struct _GjsCoveragePrivate {
......@@ -1383,7 +1384,7 @@ get_filename_from_filename_as_js_string(JSContext *context,
JS::CallArgs &args) {
char *filename = NULL;
if (!gjs_parse_call_args(context, "getFileContents", "s", args,
if (!gjs_parse_call_args(context, "getFileContents", args, "s",
"filename", &filename))
return NULL;
......@@ -1482,7 +1483,7 @@ coverage_get_file_contents(JSContext *context,
JSString *script_jsstr;
GError *error = NULL;
if (!gjs_parse_call_args(context, "getFileContents", "s", args,
if (!gjs_parse_call_args(context, "getFileContents", args, "s",
"filename", &filename))
goto out;
......
/* -*- mode: C; c-basic-offset: 4; indent-tabs-mode: nil; -*- */
/*
* Copyright © 2016 Endless Mobile, Inc.
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to
* deal in the Software without restriction, including without limitation the
* rights to use, copy, modify, merge, publish, distribute, sublicense, and/or
* sell copies of the Software, and to permit persons to whom the Software is
* furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in
* all copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
* FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
* IN THE SOFTWARE.
*
* Authored by: Philip Chimento <philip@endlessm.com>
*/
#include <type_traits>
#include <glib.h>
#include "compat.h"
static inline bool
check_nullable(const char*& fchar,
const char*& fmt_string)
{
if (*fchar != '?')
return false;
fchar++;
fmt_string++;
g_assert(((void) "Invalid format string, parameter required after '?'",
*fchar != '\0'));
return true;
}
/* This preserves the previous behaviour of gjs_parse_args(), but maybe we want
* to use JS::ToBoolean instead? */
static inline void
assign(JSContext *cx,
char c,
bool nullable,
JS::HandleValue value,
bool *ref)
{
if (c != 'b')
throw g_strdup_printf("Wrong type for %c, got bool*", c);
if (!value.isBoolean())
throw g_strdup("Not a boolean");
if (nullable)
throw g_strdup("Invalid format string combination ?b");
*ref = value.toBoolean();
}
/* This preserves the previous behaviour of gjs_parse_args(), but maybe we want
* to box primitive types instead of throwing? */
static inline void
assign(JSContext *cx,
char c,
bool nullable,
JS::HandleValue value,
JS::MutableHandleObject ref)
{
if (c != 'o')
throw g_strdup_printf("Wrong type for %c, got JS::MutableHandleObject", c);
if (nullable && value.isNull()) {
ref.set(NULL);
return;
}
if (!value.isObject())
throw g_strdup("Not an object");
ref.set(&value.toObject());
}
static inline void
assign(JSContext *cx,
char c,
bool nullable,
JS::HandleValue value,
char **ref)
{
if (nullable && (c == 's' || c == 'F') && value.isNull()) {
*ref = NULL;
return;
}
if (c == 's') {
if (!gjs_string_to_utf8(cx, value, ref))
throw g_strdup("Couldn't convert to string");
} else if (c == 'F') {
if (!gjs_string_to_filename(cx, value, ref))
throw g_strdup("Couldn't convert to filename");
} else {
throw g_strdup_printf("Wrong type for %c, got char**", c);
}
}
static inline void
assign(JSContext *cx,
char c,
bool nullable,
JS::HandleValue value,
int32_t *ref)
{
if (c != 'i')
throw g_strdup_printf("Wrong type for %c, got int32_t*", c);
if (nullable)
throw g_strdup("Invalid format string combination ?i");
if (!JS::ToInt32(cx, value, ref))
throw g_strdup("Couldn't convert to integer");
}
static inline void
assign(JSContext *cx,
char c,
bool nullable,
JS::HandleValue value,
uint32_t *ref)
{
double num;
if (c != 'u')
throw g_strdup_printf("Wrong type for %c, got uint32_t*", c);
if (nullable)
throw g_strdup("Invalid format string combination ?u");
if (!value.isNumber() || !JS::ToNumber(cx, value, &num))
throw g_strdup("Couldn't convert to unsigned integer");
if (num > G_MAXUINT32 || num < 0)
throw g_strdup_printf("Value %f is out of range", num);
*ref = num;
}
static inline void
assign(JSContext *cx,
char c,
bool nullable,
JS::HandleValue value,
int64_t *ref)
{
if (c != 't')
throw g_strdup_printf("Wrong type for %c, got int64_t*", c);
if (nullable)
throw g_strdup("Invalid format string combination ?t");
if (!JS::ToInt64(cx, value, ref))
throw g_strdup("Couldn't convert to 64-bit integer");
}
static inline void
assign(JSContext *cx,
char c,
bool nullable,
JS::HandleValue value,
double *ref)
{
if (c != 'f')
throw g_strdup_printf("Wrong type for %c, got double*", c);
if (nullable)
throw g_strdup("Invalid format string combination ?f");
if (!JS::ToNumber(cx, value, ref))
throw g_strdup("Couldn't convert to double");
}
/* Special case: treat pointer-to-enum as pointer-to-int, but use enable_if to
* prevent instantiation for any other types besides pointer-to-enum */
template<typename T,
typename std::enable_if<std::is_enum<T>::value, int>::type = 0>
static inline void
assign(JSContext *cx,
char c,
bool nullable,
JS::HandleValue value,
T *ref)
{
/* Sadly, we cannot use std::underlying_type<T> here; the underlying type of
* an enum is implementation-defined, so it would not be clear what letter
* to use in the format string. For the same reason, we can only support
* enum types that are the same width as int.
* Additionally, it would be nice to be able to check whether the resulting
* value was in range for the enum, but that is not possible (yet?) */
static_assert(sizeof(T) == sizeof(int),
"Short or wide enum types not supported");
assign(cx, c, nullable, value, (int *)ref);
}
/* Force JS::RootedObject * to be converted to JS::MutableHandleObject,
* see overload in jsapi-util-args.cpp */
template<typename T,
typename std::enable_if<!std::is_same<T, JS::RootedObject *>::value, int>::type = 0>
static inline void
free_if_necessary(T param_ref) {}
static inline void
free_if_necessary(JS::MutableHandleObject param_ref)
{
/* This is not exactly right, since before we consumed a JS::ObjectValue
* there may have been something different inside the handle. But it has
* already been clobbered at this point anyhow */
param_ref.set(NULL);
}
static inline void
free_if_necessary(char **param_ref)
{
g_free(*param_ref);
}
template<typename T>
static bool
parse_call_args_helper(JSContext *cx,
const char *function_name,
JS::CallArgs& args,
bool ignore_trailing_args,
const char*& fmt_required,
const char*& fmt_optional,
unsigned param_ix,
const char *param_name,
T param_ref)
{
bool nullable = false;
const char *fchar = fmt_required;
g_return_val_if_fail (param_name != NULL, false);
if (*fchar != '\0') {
nullable = check_nullable(fchar, fmt_required);
fmt_required++;
} else {
/* No more args passed in JS, only optional formats left */
if (args.length() <= param_ix)
return true;
fchar = fmt_optional;
g_assert(((void) "Wrong number of parameters passed to gjs_parse_call_args()",
*fchar != '\0'));
nullable = check_nullable(fchar, fmt_optional);
fmt_optional++;
}
try {
/* COMPAT: JS::CallArgs::operator[] will yield Handle in mozjs31 */
assign(cx, *fchar, nullable, args.handleOrUndefinedAt(param_ix), param_ref);
} catch (char *message) {
/* Our error messages are going to be more useful than whatever was
* thrown by the various conversion functions */
JS_ClearPendingException(cx);
gjs_throw(cx, "Error invoking %s, at argument %d (%s): %s",
function_name, param_ix, param_name, message);
g_free(message);
return false;
}
return true;
}
template<typename T, typename... Args>
static bool
parse_call_args_helper(JSContext *cx,
const char *function_name,
JS::CallArgs& args,
bool ignore_trailing_args,
const char*& fmt_required,
const char*& fmt_optional,
unsigned param_ix,
const char *param_name,
T param_ref,
Args ...params)
{
bool retval;
if (!parse_call_args_helper(cx, function_name, args, ignore_trailing_args,
fmt_required, fmt_optional, param_ix,
param_name, param_ref))
return false;
retval = parse_call_args_helper(cx, function_name, args,
ignore_trailing_args,
fmt_required, fmt_optional, ++param_ix,
params...);
/* We still own the strings in the error case, free any we converted */
if (!retval)
free_if_necessary(param_ref);
return retval;
}
/* Empty-args version of the template */
G_GNUC_UNUSED
static bool
gjs_parse_call_args(JSContext *cx,
const char *function_name,
JS::CallArgs& args,
const char *format)
{
bool ignore_trailing_args = false;
if (*format == '!') {
ignore_trailing_args = true;
format++;
}
g_assert(((void) "Wrong number of parameters passed to gjs_parse_call_args()",
*format == '\0'));
if (!ignore_trailing_args && args.length() > 0) {
JSAutoRequest ar(cx);
gjs_throw(cx, "Error invoking %s: Expected 0 arguments, got %d",
function_name, args.length());
return false;
}
return true;
}
/**
* gjs_parse_call_args:
* @context:
* @function_name: The name of the function being called
* @format: Printf-like format specifier containing the expected arguments
* @args: #JS::CallArgs from #JSNative function
* @params: for each character in @format, a pair of const char * which is the
* name of the argument, and a location to store the value. The type of
* location argument depends on the format character, as described below.
*
* This function is inspired by Python's PyArg_ParseTuple for those
* familiar with it. It takes a format specifier which gives the
* types of the expected arguments, and a list of argument names and
* value location pairs. The currently accepted format specifiers are:
*
* b: A boolean (pass a bool *)
* s: A string, converted into UTF-8 (pass a char **)
* F: A string, converted into "filename encoding" (i.e. active locale) (pass
* a char **)
* i: A number, will be converted to a 32-bit int (pass an int32_t * or a
* pointer to an enum type)
* u: A number, converted into a 32-bit unsigned int (pass a uint32_t *)
* t: A 64-bit number, converted into a 64-bit int (pass an int64_t *)
* f: A number, will be converted into a double (pass a double *)
* o: A JavaScript object (pass a JS::MutableHandleObject)
*
* If the first character in the format string is a '!', then JS is allowed
* to pass extra arguments that are ignored, to the function.
*
* The '|' character introduces optional arguments. All format specifiers
* after a '|' when not specified, do not cause any changes in the C
* value location.
*
* A prefix character '?' in front of 's', 'F', or 'o' means that the next value
* may be null. For 's' or 'F' a null pointer is returned, for 'o' the handle is
* set to null.
*/
template<typename... Args>
static bool
gjs_parse_call_args(JSContext *cx,
const char *function_name,
JS::CallArgs& args,
const char *format,
Args ...params)
{
const char *fmt_iter, *fmt_required, *fmt_optional;
unsigned n_required = 0, n_total = 0;
bool ignore_trailing_args = false, retval;
char **parts;
if (*format == '!') {
ignore_trailing_args = true;
format++;
}
for (fmt_iter = format; *fmt_iter; fmt_iter++) {
switch (*fmt_iter) {
case '|':
n_required = n_total;
continue;
case '?':
continue;
default:
n_total++;
}
}
if (n_required == 0)
n_required = n_total;
g_assert(((void) "Wrong number of parameters passed to gjs_parse_call_args()",
sizeof...(Args) / 2 == n_total));
JSAutoRequest ar(cx);
/* COMPAT: In future, use args.requireAtLeast() */
if (args.length() < n_required ||
(args.length() > n_total && !ignore_trailing_args)) {
if (n_required == n_total) {
gjs_throw(cx, "Error invoking %s: Expected %d arguments, got %d",
function_name, n_required, args.length());
} else {
gjs_throw(cx,
"Error invoking %s: Expected minimum %d arguments (and %d optional), got %d",
function_name, n_required, n_total - n_required,
args.length());
}
return false;
}
parts = g_strsplit(format, "|", 2);
fmt_required = parts[0];
fmt_optional = parts[1]; /* may be NULL */
retval = parse_call_args_helper(cx, function_name, args,
ignore_trailing_args, fmt_required,
fmt_optional, 0, params...);
g_strfreev(parts);
return retval;
}
......@@ -814,273 +814,6 @@ gjs_get_type_name(JS::Value value)
}
}
static bool
gjs_parse_args_valist (JSContext *context,
const char *function_name,
const char *format,
unsigned argc,
JS::Value *argv,
va_list args)
{
guint i;
const char *fmt_iter;
guint n_unwind = 0;
#define MAX_UNWIND_STRINGS 16
gpointer unwind_strings[MAX_UNWIND_STRINGS];
bool ignore_trailing_args = false;
guint n_required = 0;
guint n_total = 0;
guint consumed_args;
JS_BeginRequest(context);
if (*format == '!') {
ignore_trailing_args = true;
format++;
}
for (fmt_iter = format; *fmt_iter; fmt_iter++) {
switch (*fmt_iter) {
case '|':
n_required = n_total;
continue;
case '?':
continue;
default:
break;
}
n_total++;
}
if (n_required == 0)
n_required = n_total;
if (argc < n_required || (argc > n_total && !ignore_trailing_args)) {
if (n_required == n_total) {
gjs_throw(context, "Error invoking %s: Expected %d arguments, got %d", function_name,
n_required, argc);
} else {
gjs_throw(context, "Error invoking %s: Expected minimum %d arguments (and %d optional), got %d", function_name,
n_required, n_total - n_required, argc);
}
goto error_unwind;
}
/* We have 3 iteration variables here.
* @i: The current integer position in fmt_args
* @fmt_iter: A pointer to the character in fmt_args
* @consumed_args: How many arguments we've taken from argv
*
* consumed_args can currently be different from 'i' because of the '|' character.
*/
for (i = 0, consumed_args = 0, fmt_iter = format; *fmt_iter; fmt_iter++, i++) {
const char *argname;
gpointer arg_location;
JS::Value js_value;
const char *arg_error_message = NULL;
if (*fmt_iter == '|')
continue;
if (consumed_args == argc) {
break;
}
argname = va_arg (args, char *);
arg_location = va_arg (args, gpointer);
g_return_val_if_fail (argname != NULL, false);
g_return_val_if_fail (arg_location != NULL, false);
js_value = argv[consumed_args];
if (*fmt_iter == '?') {
fmt_iter++;
if (js_value.isNull()) {
gpointer *arg = (gpointer*) arg_location;
*arg = NULL;
goto got_value;
}
}
switch (*fmt_iter) {
case 'b': {
if (!js_value.isBoolean()) {
arg_error_message = "Not a boolean";
} else {
bool *arg = (bool *) arg_location;
*arg = js_value.toBoolean();
}
}
break;
case 'o': {
if (!js_value.isObject()) {
arg_error_message = "Not an object";
} else {
JSObject **arg = (JSObject**) arg_location;
*arg = &js_value.toObject();
}
}
break;
case 's': {
char **arg = (char**) arg_location;
if (gjs_string_to_utf8 (context, js_value, arg)) {
unwind_strings[n_unwind++] = *arg;
g_assert(n_unwind < MAX_UNWIND_STRINGS);
} else {
/* Our error message is going to be more useful */
JS_ClearPendingException(context);
arg_error_message = "Couldn't convert to string";
}
}
break;
case 'F': {
char **arg = (char**) arg_location;
if (gjs_string_to_filename (context, js_value, arg)) {
unwind_strings[n_unwind++] = *arg;
g_assert(n_unwind < MAX_UNWIND_STRINGS);
} else {
/* Our error message is going to be more useful */
JS_ClearPendingException(context);
arg_error_message = "Couldn't convert to filename";
}
}
break;
case 'i': {
if (!JS::ToInt32(context, js_value, (int32_t *) arg_location)) {
/* Our error message is going to be more useful */
JS_ClearPendingException(context);
arg_error_message = "Couldn't convert to integer";
}
}
break;
case 'u': {
gdouble num;
if (!js_value.isNumber() || !JS::ToNumber(context, js_value, &num)) {
/* Our error message is going to be more useful */
JS_ClearPendingException(context);
arg_error_message = "Couldn't convert to unsigned integer";
} else if (num > G_MAXUINT32 || num < 0) {
arg_error_message = "Value is out of range";
} else {
*((guint32*) arg_location) = num;