Skip to content
Permalink
Browse files

Auto merge of #23158 - BartGitHub:promise-constructor, r=jdm

Promise constructor

In this PR, measures are taken that prevent the ```Promise::new``` constructor from being used outside a compartment.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #22982 (GitHub issue number if applicable)

<!-- Either: -->
- [x] These changes do not require tests because no new functionality is added.

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/23158)
<!-- Reviewable:end -->
  • Loading branch information...
bors-servo committed Apr 6, 2019
2 parents 967efc7 + 60ba3d2 commit c87b4aab876d4dcb8bab5c1524f81df5375de181
@@ -49,8 +49,9 @@ pub enum FetchedData {

// https://fetch.spec.whatwg.org/#concept-body-consume-body
#[allow(unrooted_must_root)]
#[allow(unsafe_code)]
pub fn consume_body<T: BodyOperations + DomObject>(object: &T, body_type: BodyType) -> Rc<Promise> {
let promise = Promise::new(&object.global());
let promise = unsafe { Promise::new_in_current_compartment(&object.global()) };

// Step 1
if object.get_body_used() || object.is_locked() {
@@ -107,9 +107,10 @@ impl AudioContextMethods for AudioContext {
}

// https://webaudio.github.io/web-audio-api/#dom-audiocontext-suspend
#[allow(unsafe_code)]
fn Suspend(&self) -> Rc<Promise> {
// Step 1.
let promise = Promise::new(&self.global());
let promise = unsafe { Promise::new_in_current_compartment(&self.global()) };

// Step 2.
if self.context.control_thread_state() == ProcessingState::Closed {
@@ -168,9 +169,10 @@ impl AudioContextMethods for AudioContext {
}

// https://webaudio.github.io/web-audio-api/#dom-audiocontext-close
#[allow(unsafe_code)]
fn Close(&self) -> Rc<Promise> {
// Step 1.
let promise = Promise::new(&self.global());
let promise = unsafe { Promise::new_in_current_compartment(&self.global()) };

// Step 2.
if self.context.control_thread_state() == ProcessingState::Closed {
@@ -273,9 +273,10 @@ impl BaseAudioContextMethods for BaseAudioContext {
}

/// https://webaudio.github.io/web-audio-api/#dom-baseaudiocontext-resume
#[allow(unsafe_code)]
fn Resume(&self) -> Rc<Promise> {
// Step 1.
let promise = Promise::new(&self.global());
let promise = unsafe { Promise::new_in_current_compartment(&self.global()) };

// Step 2.
if self.audio_context_impl.state() == ProcessingState::Closed {
@@ -405,14 +406,15 @@ impl BaseAudioContextMethods for BaseAudioContext {
}

// https://webaudio.github.io/web-audio-api/#dom-baseaudiocontext-decodeaudiodata
#[allow(unsafe_code)]
fn DecodeAudioData(
&self,
audio_data: CustomAutoRooterGuard<ArrayBuffer>,
decode_success_callback: Option<Rc<DecodeSuccessCallback>>,
decode_error_callback: Option<Rc<DecodeErrorCallback>>,
) -> Rc<Promise> {
// Step 1.
let promise = Promise::new(&self.global());
let promise = unsafe { Promise::new_in_current_compartment(&self.global()) };
let global = self.global();
let window = global.as_window();

@@ -278,6 +278,7 @@ pub fn response_async<T: AsyncBluetoothListener + DomObject + 'static>(
}

// https://webbluetoothcg.github.io/web-bluetooth/#getgattchildren
#[allow(unsafe_code)]
pub fn get_gatt_children<T, F>(
attribute: &T,
single: bool,
@@ -291,7 +292,7 @@ where
T: AsyncBluetoothListener + DomObject + 'static,
F: FnOnce(StringOrUnsignedLong) -> Fallible<UUID>,
{
let p = Promise::new(&attribute.global());
let p = unsafe { Promise::new_in_current_compartment(&attribute.global()) };

let result_uuid = if let Some(u) = uuid {
// Step 1.
@@ -530,8 +531,9 @@ impl From<BluetoothError> for Error {

impl BluetoothMethods for Bluetooth {
// https://webbluetoothcg.github.io/web-bluetooth/#dom-bluetooth-requestdevice
#[allow(unsafe_code)]
fn RequestDevice(&self, option: &RequestDeviceOptions) -> Rc<Promise> {
let p = Promise::new(&self.global());
let p = unsafe { Promise::new_in_current_compartment(&self.global()) };
// Step 1.
if (option.filters.is_some() && option.acceptAllDevices) ||
(option.filters.is_none() && !option.acceptAllDevices)
@@ -548,8 +550,9 @@ impl BluetoothMethods for Bluetooth {
}

// https://webbluetoothcg.github.io/web-bluetooth/#dom-bluetooth-getavailability
#[allow(unsafe_code)]
fn GetAvailability(&self) -> Rc<Promise> {
let p = Promise::new(&self.global());
let p = unsafe { Promise::new_in_current_compartment(&self.global()) };
// Step 1. We did not override the method
// Step 2 - 3. in handle_response
let sender = response_async(&p, self);
@@ -277,8 +277,9 @@ impl BluetoothDeviceMethods for BluetoothDevice {
}

// https://webbluetoothcg.github.io/web-bluetooth/#dom-bluetoothdevice-watchadvertisements
#[allow(unsafe_code)]
fn WatchAdvertisements(&self) -> Rc<Promise> {
let p = Promise::new(&self.global());
let p = unsafe { Promise::new_in_current_compartment(&self.global()) };
let sender = response_async(&p, self);
// TODO: Step 1.
// Note: Steps 2 - 3 are implemented in components/bluetooth/lib.rs in watch_advertisements function
@@ -134,8 +134,9 @@ impl BluetoothRemoteGATTCharacteristicMethods for BluetoothRemoteGATTCharacteris
}

// https://webbluetoothcg.github.io/web-bluetooth/#dom-bluetoothremotegattcharacteristic-readvalue
#[allow(unsafe_code)]
fn ReadValue(&self) -> Rc<Promise> {
let p = Promise::new(&self.global());
let p = unsafe { Promise::new_in_current_compartment(&self.global()) };

// Step 1.
if uuid_is_blocklisted(self.uuid.as_ref(), Blocklist::Reads) {
@@ -167,8 +168,9 @@ impl BluetoothRemoteGATTCharacteristicMethods for BluetoothRemoteGATTCharacteris
}

// https://webbluetoothcg.github.io/web-bluetooth/#dom-bluetoothremotegattcharacteristic-writevalue
#[allow(unsafe_code)]
fn WriteValue(&self, value: ArrayBufferViewOrArrayBuffer) -> Rc<Promise> {
let p = Promise::new(&self.global());
let p = unsafe { Promise::new_in_current_compartment(&self.global()) };

// Step 1.
if uuid_is_blocklisted(self.uuid.as_ref(), Blocklist::Writes) {
@@ -218,8 +220,9 @@ impl BluetoothRemoteGATTCharacteristicMethods for BluetoothRemoteGATTCharacteris
}

// https://webbluetoothcg.github.io/web-bluetooth/#dom-bluetoothremotegattcharacteristic-startnotifications
#[allow(unsafe_code)]
fn StartNotifications(&self) -> Rc<Promise> {
let p = Promise::new(&self.global());
let p = unsafe { Promise::new_in_current_compartment(&self.global()) };

// Step 1.
if uuid_is_blocklisted(self.uuid.as_ref(), Blocklist::Reads) {
@@ -255,8 +258,9 @@ impl BluetoothRemoteGATTCharacteristicMethods for BluetoothRemoteGATTCharacteris
}

// https://webbluetoothcg.github.io/web-bluetooth/#dom-bluetoothremotegattcharacteristic-stopnotifications
#[allow(unsafe_code)]
fn StopNotifications(&self) -> Rc<Promise> {
let p = Promise::new(&self.global());
let p = unsafe { Promise::new_in_current_compartment(&self.global()) };
let sender = response_async(&p, self);

// TODO: Step 3 - 4: Implement `active notification context set` for BluetoothRemoteGATTCharacteristic,
@@ -93,8 +93,9 @@ impl BluetoothRemoteGATTDescriptorMethods for BluetoothRemoteGATTDescriptor {
}

// https://webbluetoothcg.github.io/web-bluetooth/#dom-bluetoothremotegattdescriptor-readvalue
#[allow(unsafe_code)]
fn ReadValue(&self) -> Rc<Promise> {
let p = Promise::new(&self.global());
let p = unsafe { Promise::new_in_current_compartment(&self.global()) };

// Step 1.
if uuid_is_blocklisted(self.uuid.as_ref(), Blocklist::Reads) {
@@ -125,8 +126,9 @@ impl BluetoothRemoteGATTDescriptorMethods for BluetoothRemoteGATTDescriptor {
}

// https://webbluetoothcg.github.io/web-bluetooth/#dom-bluetoothremotegattdescriptor-writevalue
#[allow(unsafe_code)]
fn WriteValue(&self, value: ArrayBufferViewOrArrayBuffer) -> Rc<Promise> {
let p = Promise::new(&self.global());
let p = unsafe { Promise::new_in_current_compartment(&self.global()) };

// Step 1.
if uuid_is_blocklisted(self.uuid.as_ref(), Blocklist::Writes) {
@@ -69,9 +69,10 @@ impl BluetoothRemoteGATTServerMethods for BluetoothRemoteGATTServer {
}

// https://webbluetoothcg.github.io/web-bluetooth/#dom-bluetoothremotegattserver-connect
#[allow(unsafe_code)]
fn Connect(&self) -> Rc<Promise> {
// Step 1.
let p = Promise::new(&self.global());
let p = unsafe { Promise::new_in_current_compartment(&self.global()) };
let sender = response_async(&p, self);

// TODO: Step 3: Check if the UA is currently using the Bluetooth system.
@@ -399,20 +399,21 @@ impl CustomElementRegistryMethods for CustomElementRegistry {
}

/// <https://html.spec.whatwg.org/multipage/#dom-customelementregistry-whendefined>
#[allow(unsafe_code)]
fn WhenDefined(&self, name: DOMString) -> Rc<Promise> {
let global_scope = self.window.upcast::<GlobalScope>();
let name = LocalName::from(&*name);

// Step 1
if !is_valid_custom_element_name(&name) {
let promise = Promise::new(global_scope);
let promise = unsafe { Promise::new_in_current_compartment(global_scope) };
promise.reject_native(&DOMException::new(global_scope, DOMErrorName::SyntaxError));
return promise;
}

// Step 2
if self.definitions.borrow().contains_key(&name) {
let promise = Promise::new(global_scope);
let promise = unsafe { Promise::new_in_current_compartment(global_scope) };
promise.resolve_native(&UndefinedValue());
return promise;
}
@@ -422,7 +423,7 @@ impl CustomElementRegistryMethods for CustomElementRegistry {

// Steps 4, 5
let promise = map.get(&name).cloned().unwrap_or_else(|| {
let promise = Promise::new(global_scope);
let promise = unsafe { Promise::new_in_current_compartment(global_scope) };
map.insert(name, promise.clone());
promise
});
@@ -3129,9 +3129,10 @@ impl Document {
}

// https://fullscreen.spec.whatwg.org/#dom-element-requestfullscreen
#[allow(unsafe_code)]
pub fn enter_fullscreen(&self, pending: &Element) -> Rc<Promise> {
// Step 1
let promise = Promise::new(&self.global());
let promise = unsafe { Promise::new_in_current_compartment(&self.global()) };
let mut error = false;

// Step 4
@@ -3195,10 +3196,11 @@ impl Document {
}

// https://fullscreen.spec.whatwg.org/#exit-fullscreen
#[allow(unsafe_code)]
pub fn exit_fullscreen(&self) -> Rc<Promise> {
let global = self.global();
// Step 1
let promise = Promise::new(&global);
let promise = unsafe { Promise::new_in_current_compartment(&global) };
// Step 2
if self.fullscreen_element.get().is_none() {
promise.reject_error(Error::Type(String::from("fullscreen is null")));
@@ -1651,8 +1651,9 @@ impl HTMLMediaElementMethods for HTMLMediaElement {
}

// https://html.spec.whatwg.org/multipage/#dom-media-play
#[allow(unsafe_code)]
fn Play(&self) -> Rc<Promise> {
let promise = Promise::new(&self.global());
let promise = unsafe { Promise::new_in_current_compartment(&self.global()) };
// Step 1.
// FIXME(nox): Reject promise if not allowed to play.

@@ -44,8 +44,9 @@ impl MediaDevices {

impl MediaDevicesMethods for MediaDevices {
/// https://w3c.github.io/mediacapture-main/#dom-mediadevices-getusermedia
#[allow(unsafe_code)]
fn GetUserMedia(&self, constraints: &MediaStreamConstraints) -> Rc<Promise> {
let p = Promise::new(&self.global());
let p = unsafe { Promise::new_in_current_compartment(&self.global()) };
let media = ServoMedia::get().unwrap();
let mut tracks = vec![];
if let Some(constraints) = convert_constraints(&constraints.audio) {
@@ -43,8 +43,9 @@ impl NavigationPreloadManager {

impl NavigationPreloadManagerMethods for NavigationPreloadManager {
// https://w3c.github.io/ServiceWorker/#navigation-preload-manager-enable
#[allow(unsafe_code)]
fn Enable(&self) -> Rc<Promise> {
let promise = Promise::new(&*self.global());
let promise = unsafe { Promise::new_in_current_compartment(&*self.global()) };

// 2.
if self.serviceworker_registration.active().is_none() {
@@ -65,8 +66,9 @@ impl NavigationPreloadManagerMethods for NavigationPreloadManager {
}

// https://w3c.github.io/ServiceWorker/#navigation-preload-manager-disable
#[allow(unsafe_code)]
fn Disable(&self) -> Rc<Promise> {
let promise = Promise::new(&*self.global());
let promise = unsafe { Promise::new_in_current_compartment(&*self.global()) };

// 2.
if self.serviceworker_registration.active().is_none() {
@@ -87,8 +89,9 @@ impl NavigationPreloadManagerMethods for NavigationPreloadManager {
}

// https://w3c.github.io/ServiceWorker/#navigation-preload-manager-setheadervalue
#[allow(unsafe_code)]
fn SetHeaderValue(&self, value: ByteString) -> Rc<Promise> {
let promise = Promise::new(&*self.global());
let promise = unsafe { Promise::new_in_current_compartment(&*self.global()) };

// 2.
if self.serviceworker_registration.active().is_none() {
@@ -109,8 +112,9 @@ impl NavigationPreloadManagerMethods for NavigationPreloadManager {
}

// https://w3c.github.io/ServiceWorker/#navigation-preload-manager-getstate
#[allow(unsafe_code)]
fn GetState(&self) -> Rc<Promise> {
let promise = Promise::new(&*self.global());
let promise = unsafe { Promise::new_in_current_compartment(&*self.global()) };
// 2.
let mut state = NavigationPreloadState::empty();

@@ -150,8 +150,9 @@ impl NavigatorMethods for Navigator {
}

// https://w3c.github.io/webvr/spec/1.1/#navigator-getvrdisplays-attribute
#[allow(unsafe_code)]
fn GetVRDisplays(&self) -> Rc<Promise> {
let promise = Promise::new(&self.global());
let promise = unsafe { Promise::new_in_current_compartment(&self.global()) };
let displays = self.Xr().get_displays();
match displays {
Ok(displays) => promise.resolve_native(&displays),
@@ -113,8 +113,9 @@ impl OfflineAudioContextMethods for OfflineAudioContext {
}

// https://webaudio.github.io/web-audio-api/#dom-offlineaudiocontext-startrendering
#[allow(unsafe_code)]
fn StartRendering(&self) -> Rc<Promise> {
let promise = Promise::new(&self.global());
let promise = unsafe { Promise::new_in_current_compartment(&self.global()) };
if self.rendering_started.get() {
promise.reject_error(Error::InvalidState);
return promise;
@@ -87,6 +87,7 @@ impl Permissions {
// https://w3c.github.io/permissions/#dom-permissions-query
// https://w3c.github.io/permissions/#dom-permissions-request
// https://w3c.github.io/permissions/#dom-permissions-revoke
#[allow(unsafe_code)]
fn manipulate(
&self,
op: Operation,
@@ -97,7 +98,7 @@ impl Permissions {
// (Query, Request) Step 3.
let p = match promise {
Some(promise) => promise,
None => Promise::new(&self.global()),
None => unsafe { Promise::new_in_current_compartment(&self.global()) },
};

// (Query, Request, Revoke) Step 1.
@@ -80,13 +80,16 @@ impl Drop for Promise {

impl Promise {
#[allow(unsafe_code)]
pub fn new(global: &GlobalScope) -> Rc<Promise> {
pub fn new(global: &GlobalScope, _comp: &JSAutoCompartment) -> Rc<Promise> {
unsafe { Promise::new_in_current_compartment(global) }
}

#[allow(unsafe_code)]
pub unsafe fn new_in_current_compartment(global: &GlobalScope) -> Rc<Promise> {
let cx = global.get_cx();
rooted!(in(cx) let mut obj = ptr::null_mut::<JSObject>());
unsafe {
Promise::create_js_promise(cx, HandleObject::null(), obj.handle_mut());
Promise::new_with_js_promise(obj.handle(), cx)
}
Promise::create_js_promise(cx, HandleObject::null(), obj.handle_mut());
Promise::new_with_js_promise(obj.handle(), cx)
}

#[allow(unsafe_code)]

1 comment on commit c87b4aa

@taskcluster

This comment has been minimized.

Copy link

commented on c87b4aa Apr 6, 2019

Submitting the task to Taskcluster failed. Details

InterpreterError at template.tasks[0]: object keys must be strings

Please sign in to comment.
You can’t perform that action at this time.