Bug 105865 - Port JavaScript authority to mozjs52
Summary: Port JavaScript authority to mozjs52
Status: RESOLVED FIXED
Alias: None
Product: PolicyKit
Classification: Unclassified
Component: daemon (show other bugs)
Version: unspecified
Hardware: All All
: medium normal
Assignee: David Zeuthen (not reading bugmail)
QA Contact: David Zeuthen (not reading bugmail)
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-04-03 14:49 UTC by Ray Strode [halfline]
Modified: 2018-04-03 19:49 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
configure: bump mozjs requirement to 52 (2.58 KB, patch)
2018-04-03 14:50 UTC, Ray Strode [halfline]
Details | Splinter Review
jsauthority: fix how classes are defined (5.65 KB, patch)
2018-04-03 14:50 UTC, Ray Strode [halfline]
Details | Splinter Review
jsauthority: use JS_FN instead of JS_FS (2.87 KB, patch)
2018-04-03 14:50 UTC, Ray Strode [halfline]
Details | Splinter Review
jsauthority: get rid of JSRuntime (9.41 KB, patch)
2018-04-03 14:50 UTC, Ray Strode [halfline]
Details | Splinter Review
jsauthority: change how setVersion is called (3.07 KB, patch)
2018-04-03 14:50 UTC, Ray Strode [halfline]
Details | Splinter Review
jsauthority: call JS_Init (5.09 KB, patch)
2018-04-03 14:50 UTC, Ray Strode [halfline]
Details | Splinter Review
jsauthority: call JS_InitSelfHostedCode (2.80 KB, patch)
2018-04-03 14:50 UTC, Ray Strode [halfline]
Details | Splinter Review
jsauthority: change how JIT is disabled (3.24 KB, patch)
2018-04-03 14:50 UTC, Ray Strode [halfline]
Details | Splinter Review
jsauthority: JS::SetWarningReporter instead of JS_SetErrorReporter (5.38 KB, patch)
2018-04-03 14:50 UTC, Ray Strode [halfline]
Details | Splinter Review
jsauthority: add UTF8 suffix to renamed functions (6.56 KB, patch)
2018-04-03 14:50 UTC, Ray Strode [halfline]
Details | Splinter Review
jsauthority: pass "%s" format string to report functions (2.86 KB, patch)
2018-04-03 14:50 UTC, Ray Strode [halfline]
Details | Splinter Review
jsauthority: s/JSBool/bool/ (21.35 KB, patch)
2018-04-03 14:50 UTC, Ray Strode [halfline]
Details | Splinter Review
jsauthority: s/jsval/JS::Value/ (30.72 KB, patch)
2018-04-03 14:50 UTC, Ray Strode [halfline]
Details | Splinter Review
jsauthority: s/JSVAL_NULL/JS::NullValue()/ (8.60 KB, patch)
2018-04-03 14:51 UTC, Ray Strode [halfline]
Details | Splinter Review
jsauthority: s/JSVAL_VOID/JS::UndefinedValue()/ (2.42 KB, patch)
2018-04-03 14:51 UTC, Ray Strode [halfline]
Details | Splinter Review
jsauthority: s/OBJECT_TO_JSVAL/JS::ObjectValue/ (2.92 KB, patch)
2018-04-03 14:51 UTC, Ray Strode [halfline]
Details | Splinter Review
jsauthority: s/STRING_TO_JSVAL/JS::StringValue/ (8.26 KB, patch)
2018-04-03 14:51 UTC, Ray Strode [halfline]
Details | Splinter Review
jsauthority: s/BOOLEAN_TO_JSVAL/JS::BooleanValue/ (2.27 KB, patch)
2018-04-03 14:51 UTC, Ray Strode [halfline]
Details | Splinter Review
jsauthority: JSVAL_TO_OBJECT (o) to o.toObjectOrNull() (4.54 KB, patch)
2018-04-03 14:51 UTC, Ray Strode [halfline]
Details | Splinter Review
jsauthority: JSVAL_TO_STRING (s) to s.toString() (6.53 KB, patch)
2018-04-03 14:51 UTC, Ray Strode [halfline]
Details | Splinter Review
jsauthority: JSVAL_IS_STRING (s) to s.isString() (6.56 KB, patch)
2018-04-03 14:51 UTC, Ray Strode [halfline]
Details | Splinter Review
jsauthority: JSVAL_IS_NULL (o) to o.isNull() (2.73 KB, patch)
2018-04-03 14:51 UTC, Ray Strode [halfline]
Details | Splinter Review
jsauthority: Fix up JS_CallFunctionName invocations (16.32 KB, patch)
2018-04-03 14:51 UTC, Ray Strode [halfline]
Details | Splinter Review
jsauthority: use InterruptCallback api instead of OperationCallback (5.20 KB, patch)
2018-04-03 14:51 UTC, Ray Strode [halfline]
Details | Splinter Review
jsauthority: redo how global objects are set up (14.14 KB, patch)
2018-04-03 14:51 UTC, Ray Strode [halfline]
Details | Splinter Review
jsauthority: root some locals to the context (9.07 KB, patch)
2018-04-03 14:51 UTC, Ray Strode [halfline]
Details | Splinter Review
jsauthority: adapt arguments for new JS::Compile API (3.30 KB, patch)
2018-04-03 14:51 UTC, Ray Strode [halfline]
Details | Splinter Review
jsauthority: adapt arguments for new JS_ExecuteScript API (3.05 KB, patch)
2018-04-03 14:51 UTC, Ray Strode [halfline]
Details | Splinter Review
jsauthority: use JS::Evaluate instead of JS_EvaluateScript (10.63 KB, patch)
2018-04-03 14:51 UTC, Ray Strode [halfline]
Details | Splinter Review
jsauthority: fix up set_property methods (7.04 KB, patch)
2018-04-03 14:51 UTC, Ray Strode [halfline]
Details | Splinter Review
jsauthority: stop using JS_GetStringCharsZ (9.43 KB, patch)
2018-04-03 14:51 UTC, Ray Strode [halfline]
Details | Splinter Review
jsauthority: switch from JS_ConvertArguments to JS::CallArgsFromVp (8.42 KB, patch)
2018-04-03 14:51 UTC, Ray Strode [halfline]
Details | Splinter Review
jsauthority: re-enable JIT (2.97 KB, patch)
2018-04-03 14:51 UTC, Ray Strode [halfline]
Details | Splinter Review
test: Add a test case to handle actions without explicit rules (2.74 KB, patch)
2018-04-03 14:51 UTC, Ray Strode [halfline]
Details | Splinter Review

Description Ray Strode [halfline] 2018-04-03 14:49:08 UTC
Currently polkit depends on mozjs24 to provide the JavaScript
support for the JavaScript authority.  The problem is, mozjs24
is quite old at this point.  Most other parts of the desktop
have moved on.

This patchset updates polkit to target mozjs52, instead.

As a side benefit, we can re-enable the JIT, since it no longer
seems to conflict with the watchdog thread used to detect when
a javascript script is caught in an infinite loop.
Comment 1 Ray Strode [halfline] 2018-04-03 14:50:25 UTC
Created attachment 138515 [details] [review]
configure: bump mozjs requirement to 52

This is going to briefly break the build.

Signed-off-by: Ray Strode <rstrode@redhat.com>
Comment 2 Ray Strode [halfline] 2018-04-03 14:50:28 UTC
Created attachment 138516 [details] [review]
jsauthority: fix how classes are defined

mozjs no longer has public stub functions that implementers of
JSClass objects are supposed to use.  Instead NULL means
to use the default stub implementations.

Furthermore, the structure has been broken out into a JSClassOps
sub structure now.

This commit adapts the code to the new layout.

Signed-off-by: Ray Strode <rstrode@redhat.com>
Comment 3 Ray Strode [halfline] 2018-04-03 14:50:30 UTC
Created attachment 138517 [details] [review]
jsauthority: use JS_FN instead of JS_FS

since it doesn't crash if i do that

Signed-off-by: Ray Strode <rstrode@redhat.com>
Comment 4 Ray Strode [halfline] 2018-04-03 14:50:33 UTC
Created attachment 138518 [details] [review]
jsauthority: get rid of JSRuntime

Seems like JSContext is the only thing that matters now.

Signed-off-by: Ray Strode <rstrode@redhat.com>
Comment 5 Ray Strode [halfline] 2018-04-03 14:50:35 UTC
Created attachment 138519 [details] [review]
jsauthority: change how setVersion is called

it's now part of a behaviors method in CompartmentOptions

Signed-off-by: Ray Strode <rstrode@redhat.com>
Comment 6 Ray Strode [halfline] 2018-04-03 14:50:38 UTC
Created attachment 138520 [details] [review]
jsauthority: call JS_Init

This is now required

Signed-off-by: Ray Strode <rstrode@redhat.com>
Comment 7 Ray Strode [halfline] 2018-04-03 14:50:41 UTC
Created attachment 138521 [details] [review]
jsauthority: call JS_InitSelfHostedCode

This is now required

Signed-off-by: Ray Strode <rstrode@redhat.com>
Comment 8 Ray Strode [halfline] 2018-04-03 14:50:44 UTC
Created attachment 138522 [details] [review]
jsauthority: change how JIT is disabled

JS_SetOptions seems to be replaced with JS::ContextOptionsRef now.
Also, disabling the JIT seems to be three options now instead of just
one.

Signed-off-by: Ray Strode <rstrode@redhat.com>
Comment 9 Ray Strode [halfline] 2018-04-03 14:50:47 UTC
Created attachment 138523 [details] [review]
jsauthority: JS::SetWarningReporter instead of JS_SetErrorReporter

This commit changes the code to use JS::SetWarningReporter instead
of JS_SetErrorReporter.  The latter, as far as I can tell, is
just a slightly renamed version of the former with the args moved
around a little bit.

Signed-off-by: Ray Strode <rstrode@redhat.com>
Comment 10 Ray Strode [halfline] 2018-04-03 14:50:49 UTC
Created attachment 138524 [details] [review]
jsauthority: add UTF8 suffix to renamed functions

Signed-off-by: Ray Strode <rstrode@redhat.com>
Comment 11 Ray Strode [halfline] 2018-04-03 14:50:52 UTC
Created attachment 138525 [details] [review]
jsauthority: pass "%s" format string to report functions

This just avoids the potential for security problems down the line.

Signed-off-by: Ray Strode <rstrode@redhat.com>
Comment 12 Ray Strode [halfline] 2018-04-03 14:50:54 UTC
Created attachment 138526 [details] [review]
jsauthority: s/JSBool/bool/

It's been gone since mozjs31

Signed-off-by: Ray Strode <rstrode@redhat.com>
Comment 13 Ray Strode [halfline] 2018-04-03 14:50:57 UTC
Created attachment 138527 [details] [review]
jsauthority: s/jsval/JS::Value/

The API got renamed in mozjs31.

Signed-off-by: Ray Strode <rstrode@redhat.com>
Comment 14 Ray Strode [halfline] 2018-04-03 14:51:01 UTC
Created attachment 138528 [details] [review]
jsauthority: s/JSVAL_NULL/JS::NullValue()/

This commit does a global search and replace
for JSVAL_NULL to JS::NullValue()

Signed-off-by: Ray Strode <rstrode@redhat.com>
Comment 15 Ray Strode [halfline] 2018-04-03 14:51:04 UTC
Created attachment 138529 [details] [review]
jsauthority: s/JSVAL_VOID/JS::UndefinedValue()/

This commit does a global search and replace
for JSVAL_VOID to JS::UndefinedValue()

Signed-off-by: Ray Strode <rstrode@redhat.com>
Comment 16 Ray Strode [halfline] 2018-04-03 14:51:07 UTC
Created attachment 138530 [details] [review]
jsauthority: s/OBJECT_TO_JSVAL/JS::ObjectValue/

This commit does a global search and replace
for OBJECT_TO_JSVAL to JS::ObjectValue()

Signed-off-by: Ray Strode <rstrode@redhat.com>
Comment 17 Ray Strode [halfline] 2018-04-03 14:51:11 UTC
Created attachment 138531 [details] [review]
jsauthority: s/STRING_TO_JSVAL/JS::StringValue/

Signed-off-by: Ray Strode <rstrode@redhat.com>
Comment 18 Ray Strode [halfline] 2018-04-03 14:51:14 UTC
Created attachment 138532 [details] [review]
jsauthority: s/BOOLEAN_TO_JSVAL/JS::BooleanValue/

Signed-off-by: Ray Strode <rstrode@redhat.com>
Comment 19 Ray Strode [halfline] 2018-04-03 14:51:16 UTC
Created attachment 138533 [details] [review]
jsauthority: JSVAL_TO_OBJECT (o) to o.toObjectOrNull()

Signed-off-by: Ray Strode <rstrode@redhat.com>
Comment 20 Ray Strode [halfline] 2018-04-03 14:51:19 UTC
Created attachment 138534 [details] [review]
jsauthority: JSVAL_TO_STRING (s) to s.toString()

Signed-off-by: Ray Strode <rstrode@redhat.com>
Comment 21 Ray Strode [halfline] 2018-04-03 14:51:21 UTC
Created attachment 138535 [details] [review]
jsauthority: JSVAL_IS_STRING (s) to s.isString()

Signed-off-by: Ray Strode <rstrode@redhat.com>
Comment 22 Ray Strode [halfline] 2018-04-03 14:51:24 UTC
Created attachment 138536 [details] [review]
jsauthority: JSVAL_IS_NULL (o) to o.isNull()

Signed-off-by: Ray Strode <rstrode@redhat.com>
Comment 23 Ray Strode [halfline] 2018-04-03 14:51:27 UTC
Created attachment 138537 [details] [review]
jsauthority: Fix up JS_CallFunctionName invocations

The way args are passed in changed.

Signed-off-by: Ray Strode <rstrode@redhat.com>
Comment 24 Ray Strode [halfline] 2018-04-03 14:51:29 UTC
Created attachment 138538 [details] [review]
jsauthority: use InterruptCallback api instead of OperationCallback

seems like it got renamed.

Signed-off-by: Ray Strode <rstrode@redhat.com>
Comment 25 Ray Strode [halfline] 2018-04-03 14:51:33 UTC
Created attachment 138539 [details] [review]
jsauthority: redo how global objects are set up

This commit drops usage of JS_AddObjectRoot and switches
the global object over to being wrapped in a JS::Heap
pointer. It stops using JS_DefineObject which no longer
seems to be available, and adds a new JS::FireOnNewGlobalHook
which seems to be required.

Signed-off-by: Ray Strode <rstrode@redhat.com>
Comment 26 Ray Strode [halfline] 2018-04-03 14:51:37 UTC
Created attachment 138540 [details] [review]
jsauthority: root some locals to the context

Signed-off-by: Ray Strode <rstrode@redhat.com>
Comment 27 Ray Strode [halfline] 2018-04-03 14:51:40 UTC
Created attachment 138541 [details] [review]
jsauthority: adapt arguments for new JS::Compile API

The global object is implicit now and the result is an
out arg.

This commit adapts to the new api.

Signed-off-by: Ray Strode <rstrode@redhat.com>
Comment 28 Ray Strode [halfline] 2018-04-03 14:51:43 UTC
Created attachment 138542 [details] [review]
jsauthority: adapt arguments for new JS_ExecuteScript API

JS_ExecuteScript no longer takes a global argument.

Signed-off-by: Ray Strode <rstrode@redhat.com>
Comment 29 Ray Strode [halfline] 2018-04-03 14:51:45 UTC
Created attachment 138543 [details] [review]
jsauthority: use JS::Evaluate instead of JS_EvaluateScript

JS_EvaluateScript is no longer in the API set, so use
JS::Evaluate instead.

Signed-off-by: Ray Strode <rstrode@redhat.com>
Comment 30 Ray Strode [halfline] 2018-04-03 14:51:48 UTC
Created attachment 138544 [details] [review]
jsauthority: fix up set_property methods

Signed-off-by: Ray Strode <rstrode@redhat.com>
Comment 31 Ray Strode [halfline] 2018-04-03 14:51:51 UTC
Created attachment 138545 [details] [review]
jsauthority: stop using JS_GetStringCharsZ

it's not around anymore.

Signed-off-by: Ray Strode <rstrode@redhat.com>
Comment 32 Ray Strode [halfline] 2018-04-03 14:51:54 UTC
Created attachment 138546 [details] [review]
jsauthority: switch from JS_ConvertArguments to JS::CallArgsFromVp

Signed-off-by: Ray Strode <rstrode@redhat.com>
Comment 33 Ray Strode [halfline] 2018-04-03 14:51:57 UTC
Created attachment 138547 [details] [review]
jsauthority: re-enable JIT

seems to work with mozjs52

Signed-off-by: Ray Strode <rstrode@redhat.com>
Comment 34 Ray Strode [halfline] 2018-04-03 14:51:59 UTC
Created attachment 138548 [details] [review]
test: Add a test case to handle actions without explicit rules

An implicit authorization parameter is provided to
polkit_backend_js_authority_check_authorization_sync() for actions
without corresponding explicit rules. Assure that is honored rather
than simply being denied.

Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
Comment 35 Ray Strode [halfline] 2018-04-03 18:34:17 UTC
I'm going to push this out now, without much in the way of
review (unfortunately). But I'm trying to get a release out
since the release is overdue, and this is a pre-req for the
release.

The patchset does seem to work okay with some testing.

Attachment 138515 [details] pushed as 81b92d6 - configure: bump mozjs requirement to 52
Attachment 138516 [details] pushed as 0b6db90 - jsauthority: fix how classes are defined
Attachment 138517 [details] pushed as 237f21f - jsauthority: use JS_FN instead of JS_FS
Attachment 138518 [details] pushed as d633aa5 - jsauthority: get rid of JSRuntime
Attachment 138519 [details] pushed as 8f60e85 - jsauthority: change how setVersion is called
Attachment 138520 [details] pushed as a1a66f2 - jsauthority: call JS_Init
Attachment 138521 [details] pushed as c348eef - jsauthority: call JS_InitSelfHostedCode
Attachment 138522 [details] pushed as fa27682 - jsauthority: change how JIT is disabled
Attachment 138523 [details] pushed as b6686d0 - jsauthority: JS::SetWarningReporter instead of JS_SetErrorReporter
Attachment 138524 [details] pushed as 9ae27de - jsauthority: add UTF8 suffix to renamed functions
Attachment 138525 [details] pushed as 00adeee - jsauthority: pass "%s" format string to report functions
Attachment 138526 [details] pushed as 7ecd59f - jsauthority: s/JSBool/bool/
Attachment 138527 [details] pushed as aacc152 - jsauthority: s/jsval/JS::Value/
Attachment 138528 [details] pushed as d395bab - jsauthority: s/JSVAL_NULL/JS::NullValue()/
Attachment 138529 [details] pushed as fcbb4a7 - jsauthority: s/JSVAL_VOID/JS::UndefinedValue()/
Attachment 138530 [details] pushed as 766f9d8 - jsauthority: s/OBJECT_TO_JSVAL/JS::ObjectValue/
Attachment 138531 [details] pushed as a4d815c - jsauthority: s/STRING_TO_JSVAL/JS::StringValue/
Attachment 138532 [details] pushed as f99f2bf - jsauthority: s/BOOLEAN_TO_JSVAL/JS::BooleanValue/
Attachment 138533 [details] pushed as 17a8cef - jsauthority: JSVAL_TO_OBJECT (o) to o.toObjectOrNull()
Attachment 138534 [details] pushed as f9ce9b7 - jsauthority: JSVAL_TO_STRING (s) to s.toString()
Attachment 138535 [details] pushed as e81601f - jsauthority: JSVAL_IS_STRING (s) to s.isString()
Attachment 138536 [details] pushed as a235091 - jsauthority: JSVAL_IS_NULL (o) to o.isNull()
Attachment 138537 [details] pushed as ba48e52 - jsauthority: Fix up JS_CallFunctionName invocations
Attachment 138538 [details] pushed as b69e2f7 - jsauthority: use InterruptCallback api instead of OperationCallback
Attachment 138539 [details] pushed as 84bfad0 - jsauthority: redo how global objects are set up
Attachment 138540 [details] pushed as a4a5511 - jsauthority: root some locals to the context
Attachment 138541 [details] pushed as 0ebf6c4 - jsauthority: adapt arguments for new JS::Compile API
Attachment 138542 [details] pushed as 9997181 - jsauthority: adapt arguments for new JS_ExecuteScript API
Attachment 138543 [details] pushed as e8c411c - jsauthority: use JS::Evaluate instead of JS_EvaluateScript
Attachment 138544 [details] pushed as 92bde56 - jsauthority: fix up set_property methods
Attachment 138545 [details] pushed as c556351 - jsauthority: stop using JS_GetStringCharsZ
Attachment 138546 [details] pushed as ed23dac - jsauthority: switch from JS_ConvertArguments to JS::CallArgsFromVp
Attachment 138547 [details] pushed as e155a9c - jsauthority: re-enable JIT
Attachment 138548 [details] pushed as ca50ffa - test: Add a test case to handle actions without explicit rules
Comment 36 Ray Strode [halfline] 2018-04-03 19:49:12 UTC
(In reply to Ray Strode [halfline] from comment #11)
> Created attachment 138525 [details] [review] [review]
> jsauthority: pass "%s" format string to report functions
> 
> This just avoids the potential for security problems down the line.
> 
> Signed-off-by: Ray Strode <rstrode@redhat.com>

follow up fix here:

https://cgit.freedesktop.org/polkit/commit/?id=373705b35e7f6c7dc83de5e0a3ce11ecd15d0409


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.