From 4d32444bf731f75c29e6bad7777ae8eacb96bcdc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 4 Jan 2016 13:56:37 +0100 Subject: [PATCH 1/4] webgl: Unify accessing ArrayBufferView objects This fixes an invalid length being reported from float32_array_to_slice (which used the byte length), and also to generalize getting data from a JS array buffer view, to reduce code duplication. The pending type safety issues, like where we could send a UInt16Array where we expect a Float32 one, should be solved by IDL bindings in some cases, like uniform[n]fv or vertexAttrib[n]fv, and with extra checks in others, like in the pending texImage2D(..., ArrayBufferView). --- .../script/dom/webglrenderingcontext.rs | 41 ++++++------------- 1 file changed, 13 insertions(+), 28 deletions(-) diff --git a/components/script/dom/webglrenderingcontext.rs b/components/script/dom/webglrenderingcontext.rs index 25ec30657ea..3938f97eb6b 100644 --- a/components/script/dom/webglrenderingcontext.rs +++ b/components/script/dom/webglrenderingcontext.rs @@ -29,7 +29,7 @@ use dom::webgluniformlocation::WebGLUniformLocation; use euclid::size::Size2D; use ipc_channel::ipc::{self, IpcSender}; use js::jsapi::{JSContext, JSObject, RootedValue}; -use js::jsapi::{JS_GetFloat32ArrayData, JS_GetObjectAsArrayBufferView}; +use js::jsapi::{JS_GetObjectAsArrayBufferView}; use js::jsval::{BooleanValue, DoubleValue, Int32Value, JSVal, NullValue, UndefinedValue}; use net_traits::image::base::PixelFormat; use net_traits::image_cache_task::ImageResponse; @@ -37,7 +37,7 @@ use offscreen_gl_context::GLContextAttributes; use script_traits::ScriptMsg as ConstellationMsg; use std::cell::Cell; use std::sync::mpsc::channel; -use std::{ptr, slice}; +use std::{ptr, slice, mem}; use util::str::DOMString; use util::vec::byte_swap; @@ -167,31 +167,16 @@ impl WebGLRenderingContext { } #[allow(unsafe_code)] - fn float32_array_to_slice(&self, data: *mut JSObject) -> Option> { + fn array_buffer_view_to_vec(&self, data: *mut JSObject) -> Option> { unsafe { - let mut length = 0; + let mut byte_length = 0; let mut ptr = ptr::null_mut(); - let buffer_data = JS_GetObjectAsArrayBufferView(data, &mut length, &mut ptr); + let buffer_data = JS_GetObjectAsArrayBufferView(data, &mut byte_length, &mut ptr); if buffer_data.is_null() { self.webgl_error(InvalidValue); // https://github.com/servo/servo/issues/5014 return None; } - let data_f32 = JS_GetFloat32ArrayData(data, ptr::null()); - Some(slice::from_raw_parts(data_f32, length as usize).to_vec()) - } - } - - #[allow(unsafe_code)] - fn byte_array_to_slice(&self, data: *mut JSObject) -> Option> { - unsafe { - let mut length = 0; - let mut ptr = ptr::null_mut(); - let buffer_data = JS_GetObjectAsArrayBufferView(data, &mut length, &mut ptr); - if buffer_data.is_null() { - self.webgl_error(InvalidValue); // https://github.com/servo/servo/issues/5014 - return None; - } - Some(slice::from_raw_parts(ptr, length as usize).to_vec()) + Some(slice::from_raw_parts(ptr as *mut T, byte_length as usize / mem::size_of::()).to_vec()) } } @@ -471,7 +456,7 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { Some(data) => data, None => return self.webgl_error(InvalidValue), }; - if let Some(data_vec) = self.byte_array_to_slice(data) { + if let Some(data_vec) = self.array_buffer_view_to_vec::(data) { handle_potential_webgl_error!(self, bound_buffer.buffer_data(target, &data_vec, usage)); } } @@ -495,7 +480,7 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { if offset < 0 { return self.webgl_error(InvalidValue); } - if let Some(data_vec) = self.byte_array_to_slice(data) { + if let Some(data_vec) = self.array_buffer_view_to_vec::(data) { if (offset as usize) + data_vec.len() > bound_buffer.capacity() { return self.webgl_error(InvalidValue); } @@ -973,7 +958,7 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { None => return, }; - if let Some(data_vec) = self.float32_array_to_slice(data) { + if let Some(data_vec) = self.array_buffer_view_to_vec::(data) { if data_vec.len() < 4 { return self.webgl_error(InvalidOperation); } @@ -999,7 +984,7 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { #[allow(unsafe_code)] // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.10 fn VertexAttrib1fv(&self, _cx: *mut JSContext, indx: u32, data: *mut JSObject) { - if let Some(data_vec) = self.float32_array_to_slice(data) { + if let Some(data_vec) = self.array_buffer_view_to_vec::(data) { if data_vec.len() < 4 { return self.webgl_error(InvalidOperation); } @@ -1015,7 +1000,7 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { #[allow(unsafe_code)] // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.10 fn VertexAttrib2fv(&self, _cx: *mut JSContext, indx: u32, data: *mut JSObject) { - if let Some(data_vec) = self.float32_array_to_slice(data) { + if let Some(data_vec) = self.array_buffer_view_to_vec::(data) { if data_vec.len() < 2 { return self.webgl_error(InvalidOperation); } @@ -1031,7 +1016,7 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { #[allow(unsafe_code)] // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.10 fn VertexAttrib3fv(&self, _cx: *mut JSContext, indx: u32, data: *mut JSObject) { - if let Some(data_vec) = self.float32_array_to_slice(data) { + if let Some(data_vec) = self.array_buffer_view_to_vec::(data) { if data_vec.len() < 3 { return self.webgl_error(InvalidOperation); } @@ -1047,7 +1032,7 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { #[allow(unsafe_code)] // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.10 fn VertexAttrib4fv(&self, _cx: *mut JSContext, indx: u32, data: *mut JSObject) { - if let Some(data_vec) = self.float32_array_to_slice(data) { + if let Some(data_vec) = self.array_buffer_view_to_vec::(data) { if data_vec.len() < 4 { return self.webgl_error(InvalidOperation); } From 9ad49c8aa1524f597707aaa4ea937a48534b2415 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 4 Jan 2016 14:58:57 +0100 Subject: [PATCH 2/4] conversion: Extrapolate array_buffer_view_data And use it instead of the raw jsapi calls. --- components/script/dom/bindings/conversions.rs | 32 ++++++++++++- components/script/dom/crypto.rs | 25 +++++----- components/script/dom/textdecoder.rs | 20 ++++---- .../script/dom/webglrenderingcontext.rs | 48 +++++++++---------- 4 files changed, 76 insertions(+), 49 deletions(-) diff --git a/components/script/dom/bindings/conversions.rs b/components/script/dom/bindings/conversions.rs index b887fd27d76..63af3f13f93 100644 --- a/components/script/dom/bindings/conversions.rs +++ b/components/script/dom/bindings/conversions.rs @@ -48,11 +48,12 @@ use js::jsapi::{JSClass, JSContext, JSObject, MutableHandleValue}; use js::jsapi::{JS_GetLatin1StringCharsAndLength, JS_GetReservedSlot}; use js::jsapi::{JS_GetTwoByteStringCharsAndLength, JS_NewStringCopyN}; use js::jsapi::{JS_StringHasLatin1Chars, JS_WrapValue}; +use js::jsapi::{JS_GetObjectAsArrayBufferView}; use js::jsval::{ObjectValue, StringValue}; use js::rust::ToString; use libc; use num::Float; -use std::{ptr, slice}; +use std::{ptr, mem, slice}; use util::str::DOMString; pub use util::str::{StringificationBehavior, jsstring_to_str}; @@ -335,3 +336,32 @@ impl ToJSValConvertible for Root { self.reflector().to_jsval(cx, rval); } } + +/// A JS ArrayBufferView contents can only be viewed as the types marked with this trait +pub unsafe trait ArrayBufferViewContents: Clone {} +unsafe impl ArrayBufferViewContents for u8 {} +unsafe impl ArrayBufferViewContents for i8 {} +unsafe impl ArrayBufferViewContents for u16 {} +unsafe impl ArrayBufferViewContents for i16 {} +unsafe impl ArrayBufferViewContents for u32 {} +unsafe impl ArrayBufferViewContents for i32 {} +unsafe impl ArrayBufferViewContents for f32 {} +unsafe impl ArrayBufferViewContents for f64 {} + +/// Returns a mutable slice of the Array Buffer View data, viewed as T +pub unsafe fn array_buffer_view_data<'a, T: ArrayBufferViewContents>(abv: *mut JSObject) -> Option<&'a mut [T]> { + let mut byte_length = 0; + let mut ptr = ptr::null_mut(); + let ret = JS_GetObjectAsArrayBufferView(abv, &mut byte_length, &mut ptr); + if ret.is_null() { + return None; + } + Some(slice::from_raw_parts_mut(ptr as *mut T, byte_length as usize / mem::size_of::())) +} + +/// Returns a copy of the ArrayBufferView data +pub fn array_buffer_view_to_vec(abv: *mut JSObject) -> Option> { + unsafe { + array_buffer_view_data(abv).map(|data| data.to_vec()) + } +} diff --git a/components/script/dom/crypto.rs b/components/script/dom/crypto.rs index c0181a9a900..bd75b58db1b 100644 --- a/components/script/dom/crypto.rs +++ b/components/script/dom/crypto.rs @@ -3,6 +3,7 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ use dom::bindings::cell::DOMRefCell; +use dom::bindings::conversions::array_buffer_view_data; use dom::bindings::codegen::Bindings::CryptoBinding; use dom::bindings::codegen::Bindings::CryptoBinding::CryptoMethods; use dom::bindings::error::{Error, Fallible}; @@ -10,9 +11,8 @@ use dom::bindings::global::GlobalRef; use dom::bindings::js::Root; use dom::bindings::reflector::{Reflector, reflect_dom_object}; use js::jsapi::{JSContext, JSObject}; -use js::jsapi::{JS_GetArrayBufferViewType, JS_GetObjectAsArrayBufferView, Type}; +use js::jsapi::{JS_GetArrayBufferViewType, Type}; use rand::{OsRng, Rng}; -use std::{ptr, slice}; no_jsmanaged_fields!(OsRng); @@ -43,24 +43,23 @@ impl CryptoMethods for Crypto { _cx: *mut JSContext, input: *mut JSObject) -> Fallible<*mut JSObject> { - let mut length = 0; - let mut data = ptr::null_mut(); - if unsafe { JS_GetObjectAsArrayBufferView(input, &mut length, &mut data).is_null() } { - return Err(Error::Type("Argument to Crypto.getRandomValues is not an ArrayBufferView" + let mut data = match unsafe { array_buffer_view_data::(input) } { + Some(data) => data, + None => { + return Err(Error::Type("Argument to Crypto.getRandomValues is not an ArrayBufferView" .to_owned())); - } + } + }; + if !is_integer_buffer(input) { return Err(Error::TypeMismatch); } - if length > 65536 { + + if data.len() > 65536 { return Err(Error::QuotaExceeded); } - let mut buffer = unsafe { - slice::from_raw_parts_mut(data, length as usize) - }; - - self.rng.borrow_mut().fill_bytes(&mut buffer); + self.rng.borrow_mut().fill_bytes(&mut data); Ok(input) } diff --git a/components/script/dom/textdecoder.rs b/components/script/dom/textdecoder.rs index c0dcd485c9f..30ecb84f2af 100644 --- a/components/script/dom/textdecoder.rs +++ b/components/script/dom/textdecoder.rs @@ -4,6 +4,7 @@ use dom::bindings::codegen::Bindings::TextDecoderBinding; use dom::bindings::codegen::Bindings::TextDecoderBinding::TextDecoderMethods; +use dom::bindings::conversions::array_buffer_view_data; use dom::bindings::error::{Error, Fallible}; use dom::bindings::global::GlobalRef; use dom::bindings::js::Root; @@ -13,10 +14,8 @@ use dom::bindings::trace::JSTraceable; use encoding::Encoding; use encoding::label::encoding_from_whatwg_label; use encoding::types::{DecoderTrap, EncodingRef}; -use js::jsapi::JS_GetObjectAsArrayBufferView; use js::jsapi::{JSContext, JSObject}; use std::borrow::ToOwned; -use std::{ptr, slice}; use util::str::DOMString; #[dom_struct] @@ -89,21 +88,20 @@ impl TextDecoderMethods for TextDecoder { None => return Ok(USVString("".to_owned())), }; - let mut length = 0; - let mut data = ptr::null_mut(); - if unsafe { JS_GetObjectAsArrayBufferView(input, &mut length, &mut data).is_null() } { - return Err(Error::Type("Argument to TextDecoder.decode is not an ArrayBufferView".to_owned())); - } - - let buffer = unsafe { - slice::from_raw_parts(data as *const _, length as usize) + let data = match unsafe { array_buffer_view_data::(input) } { + Some(data) => data, + None => { + return Err(Error::Type("Argument to TextDecoder.decode is not an ArrayBufferView".to_owned())); + } }; + let trap = if self.fatal { DecoderTrap::Strict } else { DecoderTrap::Replace }; - match self.encoding.decode(buffer, trap) { + + match self.encoding.decode(data, trap) { Ok(s) => Ok(USVString(s)), Err(_) => Err(Error::Type("Decoding failed".to_owned())), } diff --git a/components/script/dom/webglrenderingcontext.rs b/components/script/dom/webglrenderingcontext.rs index 3938f97eb6b..7fb094e62c2 100644 --- a/components/script/dom/webglrenderingcontext.rs +++ b/components/script/dom/webglrenderingcontext.rs @@ -9,7 +9,7 @@ use dom::bindings::codegen::Bindings::WebGLRenderingContextBinding::WebGLRenderi use dom::bindings::codegen::Bindings::WebGLRenderingContextBinding::{WebGLRenderingContextMethods}; use dom::bindings::codegen::Bindings::WebGLRenderingContextBinding::{self, WebGLContextAttributes}; use dom::bindings::codegen::UnionTypes::ImageDataOrHTMLImageElementOrHTMLCanvasElementOrHTMLVideoElement; -use dom::bindings::conversions::ToJSValConvertible; +use dom::bindings::conversions::{ToJSValConvertible, array_buffer_view_to_vec}; use dom::bindings::global::{GlobalField, GlobalRef}; use dom::bindings::inheritance::Castable; use dom::bindings::js::{JS, LayoutJS, MutNullableHeap, Root}; @@ -29,7 +29,6 @@ use dom::webgluniformlocation::WebGLUniformLocation; use euclid::size::Size2D; use ipc_channel::ipc::{self, IpcSender}; use js::jsapi::{JSContext, JSObject, RootedValue}; -use js::jsapi::{JS_GetObjectAsArrayBufferView}; use js::jsval::{BooleanValue, DoubleValue, Int32Value, JSVal, NullValue, UndefinedValue}; use net_traits::image::base::PixelFormat; use net_traits::image_cache_task::ImageResponse; @@ -37,7 +36,6 @@ use offscreen_gl_context::GLContextAttributes; use script_traits::ScriptMsg as ConstellationMsg; use std::cell::Cell; use std::sync::mpsc::channel; -use std::{ptr, slice, mem}; use util::str::DOMString; use util::vec::byte_swap; @@ -166,20 +164,6 @@ impl WebGLRenderingContext { self.canvas.upcast::().dirty(NodeDamage::OtherNodeDamage); } - #[allow(unsafe_code)] - fn array_buffer_view_to_vec(&self, data: *mut JSObject) -> Option> { - unsafe { - let mut byte_length = 0; - let mut ptr = ptr::null_mut(); - let buffer_data = JS_GetObjectAsArrayBufferView(data, &mut byte_length, &mut ptr); - if buffer_data.is_null() { - self.webgl_error(InvalidValue); // https://github.com/servo/servo/issues/5014 - return None; - } - Some(slice::from_raw_parts(ptr as *mut T, byte_length as usize / mem::size_of::()).to_vec()) - } - } - fn vertex_attrib(&self, indx: u32, x: f32, y: f32, z: f32, w: f32) { self.ipc_renderer .send(CanvasMsg::WebGL(CanvasWebGLMsg::VertexAttrib(indx, x, y, z, w))) @@ -456,8 +440,12 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { Some(data) => data, None => return self.webgl_error(InvalidValue), }; - if let Some(data_vec) = self.array_buffer_view_to_vec::(data) { + if let Some(data_vec) = array_buffer_view_to_vec::(data) { handle_potential_webgl_error!(self, bound_buffer.buffer_data(target, &data_vec, usage)); + } else { + // NB: array_buffer_view_to_vec should never fail when + // we have WebIDL support for Float32Array etc. + self.webgl_error(InvalidValue); } } @@ -480,13 +468,15 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { if offset < 0 { return self.webgl_error(InvalidValue); } - if let Some(data_vec) = self.array_buffer_view_to_vec::(data) { + if let Some(data_vec) = array_buffer_view_to_vec::(data) { if (offset as usize) + data_vec.len() > bound_buffer.capacity() { return self.webgl_error(InvalidValue); } self.ipc_renderer .send(CanvasMsg::WebGL(CanvasWebGLMsg::BufferSubData(target, offset as isize, data_vec))) .unwrap() + } else { + self.webgl_error(InvalidValue); } } @@ -958,7 +948,7 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { None => return, }; - if let Some(data_vec) = self.array_buffer_view_to_vec::(data) { + if let Some(data_vec) = array_buffer_view_to_vec::(data) { if data_vec.len() < 4 { return self.webgl_error(InvalidOperation); } @@ -966,6 +956,8 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { self.ipc_renderer .send(CanvasMsg::WebGL(CanvasWebGLMsg::Uniform4fv(uniform_id, data_vec))) .unwrap() + } else { + self.webgl_error(InvalidValue); } } @@ -984,11 +976,13 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { #[allow(unsafe_code)] // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.10 fn VertexAttrib1fv(&self, _cx: *mut JSContext, indx: u32, data: *mut JSObject) { - if let Some(data_vec) = self.array_buffer_view_to_vec::(data) { + if let Some(data_vec) = array_buffer_view_to_vec::(data) { if data_vec.len() < 4 { return self.webgl_error(InvalidOperation); } self.vertex_attrib(indx, data_vec[0], 0f32, 0f32, 1f32) + } else { + self.webgl_error(InvalidValue); } } @@ -1000,11 +994,13 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { #[allow(unsafe_code)] // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.10 fn VertexAttrib2fv(&self, _cx: *mut JSContext, indx: u32, data: *mut JSObject) { - if let Some(data_vec) = self.array_buffer_view_to_vec::(data) { + if let Some(data_vec) = array_buffer_view_to_vec::(data) { if data_vec.len() < 2 { return self.webgl_error(InvalidOperation); } self.vertex_attrib(indx, data_vec[0], data_vec[1], 0f32, 1f32) + } else { + self.webgl_error(InvalidValue); } } @@ -1016,11 +1012,13 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { #[allow(unsafe_code)] // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.10 fn VertexAttrib3fv(&self, _cx: *mut JSContext, indx: u32, data: *mut JSObject) { - if let Some(data_vec) = self.array_buffer_view_to_vec::(data) { + if let Some(data_vec) = array_buffer_view_to_vec::(data) { if data_vec.len() < 3 { return self.webgl_error(InvalidOperation); } self.vertex_attrib(indx, data_vec[0], data_vec[1], data_vec[2], 1f32) + } else { + self.webgl_error(InvalidValue); } } @@ -1032,11 +1030,13 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { #[allow(unsafe_code)] // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.10 fn VertexAttrib4fv(&self, _cx: *mut JSContext, indx: u32, data: *mut JSObject) { - if let Some(data_vec) = self.array_buffer_view_to_vec::(data) { + if let Some(data_vec) = array_buffer_view_to_vec::(data) { if data_vec.len() < 4 { return self.webgl_error(InvalidOperation); } self.vertex_attrib(indx, data_vec[0], data_vec[1], data_vec[2], data_vec[3]) + } else { + self.webgl_error(InvalidValue); } } From 43d395a682c1b03497e4869744b4b25e92906cc6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 4 Jan 2016 15:18:40 +0100 Subject: [PATCH 3/4] conversions: Add a checked version to array_buffer_view_data --- components/script/dom/bindings/conversions.rs | 30 +++++++++++++++++-- components/script/dom/crypto.rs | 2 +- .../script/dom/webglrenderingcontext.rs | 15 +++++----- 3 files changed, 35 insertions(+), 12 deletions(-) diff --git a/components/script/dom/bindings/conversions.rs b/components/script/dom/bindings/conversions.rs index 63af3f13f93..489e1788f11 100644 --- a/components/script/dom/bindings/conversions.rs +++ b/components/script/dom/bindings/conversions.rs @@ -46,9 +46,10 @@ use js::glue::{RUST_JSID_IS_STRING, RUST_JSID_TO_STRING, UnwrapObject}; use js::jsapi::{HandleId, HandleObject, HandleValue, JS_GetClass}; use js::jsapi::{JSClass, JSContext, JSObject, MutableHandleValue}; use js::jsapi::{JS_GetLatin1StringCharsAndLength, JS_GetReservedSlot}; +use js::jsapi::{JS_GetObjectAsArrayBufferView, JS_GetArrayBufferViewType}; use js::jsapi::{JS_GetTwoByteStringCharsAndLength, JS_NewStringCopyN}; use js::jsapi::{JS_StringHasLatin1Chars, JS_WrapValue}; -use js::jsapi::{JS_GetObjectAsArrayBufferView}; +use js::jsapi::{Type}; use js::jsval::{ObjectValue, StringValue}; use js::rust::ToString; use libc; @@ -348,7 +349,8 @@ unsafe impl ArrayBufferViewContents for i32 {} unsafe impl ArrayBufferViewContents for f32 {} unsafe impl ArrayBufferViewContents for f64 {} -/// Returns a mutable slice of the Array Buffer View data, viewed as T +/// Returns a mutable slice of the Array Buffer View data, viewed as T, without checking the real +/// type of it. pub unsafe fn array_buffer_view_data<'a, T: ArrayBufferViewContents>(abv: *mut JSObject) -> Option<&'a mut [T]> { let mut byte_length = 0; let mut ptr = ptr::null_mut(); @@ -359,9 +361,31 @@ pub unsafe fn array_buffer_view_data<'a, T: ArrayBufferViewContents>(abv: *mut J Some(slice::from_raw_parts_mut(ptr as *mut T, byte_length as usize / mem::size_of::())) } -/// Returns a copy of the ArrayBufferView data +/// Returns a copy of the ArrayBufferView data, viewed as T, without checking the real type of it. pub fn array_buffer_view_to_vec(abv: *mut JSObject) -> Option> { unsafe { array_buffer_view_data(abv).map(|data| data.to_vec()) } } + +/// Returns a mutable slice of the Array Buffer View data, viewed as T, checking that the real type +/// of it is ty. +pub unsafe fn array_buffer_view_data_checked<'a, T: ArrayBufferViewContents>(abv: *mut JSObject, + ty: Type) -> Option<&'a mut [T]> { + array_buffer_view_data::(abv).and_then(|data| { + let real_ty = JS_GetArrayBufferViewType(abv); + if real_ty as i32 == ty as i32 { + Some(data) + } else { + None + } + }) +} + +/// Returns a copy of the ArrayBufferView data, viewed as T, checking that the real type +/// of it is ty. +pub fn array_buffer_view_to_vec_checked(abv: *mut JSObject, ty: Type) -> Option> { + unsafe { + array_buffer_view_data_checked(abv, ty).map(|data| data.to_vec()) + } +} diff --git a/components/script/dom/crypto.rs b/components/script/dom/crypto.rs index bd75b58db1b..b8c595f3aa4 100644 --- a/components/script/dom/crypto.rs +++ b/components/script/dom/crypto.rs @@ -3,9 +3,9 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ use dom::bindings::cell::DOMRefCell; -use dom::bindings::conversions::array_buffer_view_data; use dom::bindings::codegen::Bindings::CryptoBinding; use dom::bindings::codegen::Bindings::CryptoBinding::CryptoMethods; +use dom::bindings::conversions::array_buffer_view_data; use dom::bindings::error::{Error, Fallible}; use dom::bindings::global::GlobalRef; use dom::bindings::js::Root; diff --git a/components/script/dom/webglrenderingcontext.rs b/components/script/dom/webglrenderingcontext.rs index 7fb094e62c2..43fb4d1d325 100644 --- a/components/script/dom/webglrenderingcontext.rs +++ b/components/script/dom/webglrenderingcontext.rs @@ -9,7 +9,7 @@ use dom::bindings::codegen::Bindings::WebGLRenderingContextBinding::WebGLRenderi use dom::bindings::codegen::Bindings::WebGLRenderingContextBinding::{WebGLRenderingContextMethods}; use dom::bindings::codegen::Bindings::WebGLRenderingContextBinding::{self, WebGLContextAttributes}; use dom::bindings::codegen::UnionTypes::ImageDataOrHTMLImageElementOrHTMLCanvasElementOrHTMLVideoElement; -use dom::bindings::conversions::{ToJSValConvertible, array_buffer_view_to_vec}; +use dom::bindings::conversions::{ToJSValConvertible, array_buffer_view_to_vec_checked, array_buffer_view_to_vec}; use dom::bindings::global::{GlobalField, GlobalRef}; use dom::bindings::inheritance::Castable; use dom::bindings::js::{JS, LayoutJS, MutNullableHeap, Root}; @@ -28,7 +28,7 @@ use dom::webgltexture::{TexParameterValue, WebGLTexture}; use dom::webgluniformlocation::WebGLUniformLocation; use euclid::size::Size2D; use ipc_channel::ipc::{self, IpcSender}; -use js::jsapi::{JSContext, JSObject, RootedValue}; +use js::jsapi::{JSContext, JSObject, RootedValue, Type}; use js::jsval::{BooleanValue, DoubleValue, Int32Value, JSVal, NullValue, UndefinedValue}; use net_traits::image::base::PixelFormat; use net_traits::image_cache_task::ImageResponse; @@ -948,7 +948,7 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { None => return, }; - if let Some(data_vec) = array_buffer_view_to_vec::(data) { + if let Some(data_vec) = array_buffer_view_to_vec_checked::(data, Type::Float32) { if data_vec.len() < 4 { return self.webgl_error(InvalidOperation); } @@ -976,7 +976,7 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { #[allow(unsafe_code)] // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.10 fn VertexAttrib1fv(&self, _cx: *mut JSContext, indx: u32, data: *mut JSObject) { - if let Some(data_vec) = array_buffer_view_to_vec::(data) { + if let Some(data_vec) = array_buffer_view_to_vec_checked::(data, Type::Float32) { if data_vec.len() < 4 { return self.webgl_error(InvalidOperation); } @@ -994,7 +994,7 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { #[allow(unsafe_code)] // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.10 fn VertexAttrib2fv(&self, _cx: *mut JSContext, indx: u32, data: *mut JSObject) { - if let Some(data_vec) = array_buffer_view_to_vec::(data) { + if let Some(data_vec) = array_buffer_view_to_vec_checked::(data, Type::Float32) { if data_vec.len() < 2 { return self.webgl_error(InvalidOperation); } @@ -1012,7 +1012,7 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { #[allow(unsafe_code)] // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.10 fn VertexAttrib3fv(&self, _cx: *mut JSContext, indx: u32, data: *mut JSObject) { - if let Some(data_vec) = array_buffer_view_to_vec::(data) { + if let Some(data_vec) = array_buffer_view_to_vec_checked::(data, Type::Float32) { if data_vec.len() < 3 { return self.webgl_error(InvalidOperation); } @@ -1027,10 +1027,9 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { self.vertex_attrib(indx, x, y, z, w) } - #[allow(unsafe_code)] // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.10 fn VertexAttrib4fv(&self, _cx: *mut JSContext, indx: u32, data: *mut JSObject) { - if let Some(data_vec) = array_buffer_view_to_vec::(data) { + if let Some(data_vec) = array_buffer_view_to_vec_checked::(data, Type::Float32) { if data_vec.len() < 4 { return self.webgl_error(InvalidOperation); } From d30f05554b981ae564293234c55ad43bc2df8134 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 4 Jan 2016 15:51:01 +0100 Subject: [PATCH 4/4] conversions: Make a `is_type_compatible` method for ArrayBufferViews. --- components/script/dom/bindings/conversions.rs | 76 +++++++++++++++---- .../script/dom/webglrenderingcontext.rs | 12 +-- 2 files changed, 67 insertions(+), 21 deletions(-) diff --git a/components/script/dom/bindings/conversions.rs b/components/script/dom/bindings/conversions.rs index 489e1788f11..f9031d3369d 100644 --- a/components/script/dom/bindings/conversions.rs +++ b/components/script/dom/bindings/conversions.rs @@ -339,15 +339,62 @@ impl ToJSValConvertible for Root { } /// A JS ArrayBufferView contents can only be viewed as the types marked with this trait -pub unsafe trait ArrayBufferViewContents: Clone {} -unsafe impl ArrayBufferViewContents for u8 {} -unsafe impl ArrayBufferViewContents for i8 {} -unsafe impl ArrayBufferViewContents for u16 {} -unsafe impl ArrayBufferViewContents for i16 {} -unsafe impl ArrayBufferViewContents for u32 {} -unsafe impl ArrayBufferViewContents for i32 {} -unsafe impl ArrayBufferViewContents for f32 {} -unsafe impl ArrayBufferViewContents for f64 {} +pub unsafe trait ArrayBufferViewContents: Clone { + /// Check if the JS ArrayBufferView type is compatible with the implementor of the + /// trait + fn is_type_compatible(ty: Type) -> bool; +} + +unsafe impl ArrayBufferViewContents for u8 { + fn is_type_compatible(ty: Type) -> bool { + match ty { + Type::Uint8 | + Type::Uint8Clamped => true, + _ => false, + } + } +} + +unsafe impl ArrayBufferViewContents for i8 { + fn is_type_compatible(ty: Type) -> bool { + ty as i32 == Type::Int8 as i32 + } +} + +unsafe impl ArrayBufferViewContents for u16 { + fn is_type_compatible(ty: Type) -> bool { + ty as i32 == Type::Uint16 as i32 + } +} + +unsafe impl ArrayBufferViewContents for i16 { + fn is_type_compatible(ty: Type) -> bool { + ty as i32 == Type::Int16 as i32 + } +} + +unsafe impl ArrayBufferViewContents for u32 { + fn is_type_compatible(ty: Type) -> bool { + ty as i32 == Type::Uint32 as i32 + } +} + +unsafe impl ArrayBufferViewContents for i32 { + fn is_type_compatible(ty: Type) -> bool { + ty as i32 == Type::Int32 as i32 + } +} + +unsafe impl ArrayBufferViewContents for f32 { + fn is_type_compatible(ty: Type) -> bool { + ty as i32 == Type::Float32 as i32 + } +} +unsafe impl ArrayBufferViewContents for f64 { + fn is_type_compatible(ty: Type) -> bool { + ty as i32 == Type::Float64 as i32 + } +} /// Returns a mutable slice of the Array Buffer View data, viewed as T, without checking the real /// type of it. @@ -370,11 +417,10 @@ pub fn array_buffer_view_to_vec(abv: *mut JSObject) /// Returns a mutable slice of the Array Buffer View data, viewed as T, checking that the real type /// of it is ty. -pub unsafe fn array_buffer_view_data_checked<'a, T: ArrayBufferViewContents>(abv: *mut JSObject, - ty: Type) -> Option<&'a mut [T]> { +pub unsafe fn array_buffer_view_data_checked<'a, T: ArrayBufferViewContents>(abv: *mut JSObject) + -> Option<&'a mut [T]> { array_buffer_view_data::(abv).and_then(|data| { - let real_ty = JS_GetArrayBufferViewType(abv); - if real_ty as i32 == ty as i32 { + if T::is_type_compatible(JS_GetArrayBufferViewType(abv)) { Some(data) } else { None @@ -384,8 +430,8 @@ pub unsafe fn array_buffer_view_data_checked<'a, T: ArrayBufferViewContents>(abv /// Returns a copy of the ArrayBufferView data, viewed as T, checking that the real type /// of it is ty. -pub fn array_buffer_view_to_vec_checked(abv: *mut JSObject, ty: Type) -> Option> { +pub fn array_buffer_view_to_vec_checked(abv: *mut JSObject) -> Option> { unsafe { - array_buffer_view_data_checked(abv, ty).map(|data| data.to_vec()) + array_buffer_view_data_checked(abv).map(|data| data.to_vec()) } } diff --git a/components/script/dom/webglrenderingcontext.rs b/components/script/dom/webglrenderingcontext.rs index 43fb4d1d325..6b6c310bcf0 100644 --- a/components/script/dom/webglrenderingcontext.rs +++ b/components/script/dom/webglrenderingcontext.rs @@ -28,7 +28,7 @@ use dom::webgltexture::{TexParameterValue, WebGLTexture}; use dom::webgluniformlocation::WebGLUniformLocation; use euclid::size::Size2D; use ipc_channel::ipc::{self, IpcSender}; -use js::jsapi::{JSContext, JSObject, RootedValue, Type}; +use js::jsapi::{JSContext, JSObject, RootedValue}; use js::jsval::{BooleanValue, DoubleValue, Int32Value, JSVal, NullValue, UndefinedValue}; use net_traits::image::base::PixelFormat; use net_traits::image_cache_task::ImageResponse; @@ -948,7 +948,7 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { None => return, }; - if let Some(data_vec) = array_buffer_view_to_vec_checked::(data, Type::Float32) { + if let Some(data_vec) = array_buffer_view_to_vec_checked::(data) { if data_vec.len() < 4 { return self.webgl_error(InvalidOperation); } @@ -976,7 +976,7 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { #[allow(unsafe_code)] // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.10 fn VertexAttrib1fv(&self, _cx: *mut JSContext, indx: u32, data: *mut JSObject) { - if let Some(data_vec) = array_buffer_view_to_vec_checked::(data, Type::Float32) { + if let Some(data_vec) = array_buffer_view_to_vec_checked::(data) { if data_vec.len() < 4 { return self.webgl_error(InvalidOperation); } @@ -994,7 +994,7 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { #[allow(unsafe_code)] // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.10 fn VertexAttrib2fv(&self, _cx: *mut JSContext, indx: u32, data: *mut JSObject) { - if let Some(data_vec) = array_buffer_view_to_vec_checked::(data, Type::Float32) { + if let Some(data_vec) = array_buffer_view_to_vec_checked::(data) { if data_vec.len() < 2 { return self.webgl_error(InvalidOperation); } @@ -1012,7 +1012,7 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { #[allow(unsafe_code)] // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.10 fn VertexAttrib3fv(&self, _cx: *mut JSContext, indx: u32, data: *mut JSObject) { - if let Some(data_vec) = array_buffer_view_to_vec_checked::(data, Type::Float32) { + if let Some(data_vec) = array_buffer_view_to_vec_checked::(data) { if data_vec.len() < 3 { return self.webgl_error(InvalidOperation); } @@ -1029,7 +1029,7 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.10 fn VertexAttrib4fv(&self, _cx: *mut JSContext, indx: u32, data: *mut JSObject) { - if let Some(data_vec) = array_buffer_view_to_vec_checked::(data, Type::Float32) { + if let Some(data_vec) = array_buffer_view_to_vec_checked::(data) { if data_vec.len() < 4 { return self.webgl_error(InvalidOperation); }