Auto merge of #21755 - servo:webgl, r=jdm

Validate GLSL names (fixes #21287)

<!-- 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/21755)
<!-- Reviewable:end -->

3 files changed, 68 insertions, 79 deletions

diff --git a/components/script/dom/webglprogram.rs b/components/script/dom/webglprogram.rs
index 3c2a17ca4fb..5f857188481 100644
--- a/components/script/dom/webglprogram.rs
+++ b/components/script/dom/webglprogram.rs
@@ -14,7 +14,7 @@ use dom::bindings::root::{DomRoot, MutNullableDom};
 use dom::bindings::str::DOMString;
 use dom::webglactiveinfo::WebGLActiveInfo;
 use dom::webglobject::WebGLObject;
-use dom::webglrenderingcontext::{MAX_UNIFORM_AND_ATTRIBUTE_LEN, WebGLRenderingContext};
+use dom::webglrenderingcontext::WebGLRenderingContext;
 use dom::webglshader::WebGLShader;
 use dom::webgluniformlocation::WebGLUniformLocation;
 use dom_struct::dom_struct;
@@ -268,12 +268,11 @@ impl WebGLProgram {
         if self.is_deleted() {
             return Err(WebGLError::InvalidOperation);
         }
-        if name.len() > MAX_UNIFORM_AND_ATTRIBUTE_LEN {
-            return Err(WebGLError::InvalidValue);
-        }
 
-        // Check if the name is reserved
-        if name.starts_with("gl_") || name.starts_with("webgl") || name.starts_with("_webgl_") {
+        if !validate_glsl_name(&name)? {
+            return Ok(());
+        }
+        if name.starts_with("gl_") {
             return Err(WebGLError::InvalidOperation);
         }
 
@@ -325,17 +324,11 @@ impl WebGLProgram {
         if !self.is_linked() || self.is_deleted() {
             return Err(WebGLError::InvalidOperation);
         }
-        if name.len() > MAX_UNIFORM_AND_ATTRIBUTE_LEN {
-            return Err(WebGLError::InvalidValue);
-        }
 
-        // Check if the name is reserved
-        if name.starts_with("gl_") {
+        if !validate_glsl_name(&name)? {
             return Ok(-1);
         }
-
-        // https://www.khronos.org/registry/webgl/specs/latest/1.0/#GLSL_CONSTRUCTS
-        if name.starts_with("webgl_") || name.starts_with("_webgl_") {
+        if name.starts_with("gl_") {
             return Ok(-1);
         }
 
@@ -356,17 +349,11 @@ impl WebGLProgram {
         if !self.is_linked() || self.is_deleted() {
             return Err(WebGLError::InvalidOperation);
         }
-        if name.len() > MAX_UNIFORM_AND_ATTRIBUTE_LEN {
-            return Err(WebGLError::InvalidValue);
-        }
 
-        // Check if the name is reserved
-        if name.starts_with("gl_") {
+        if !validate_glsl_name(&name)? {
             return Ok(None);
         }
-
-        // https://www.khronos.org/registry/webgl/specs/latest/1.0/#GLSL_CONSTRUCTS
-        if name.starts_with("webgl_") || name.starts_with("_webgl_") {
+        if name.starts_with("gl_") {
             return Ok(None);
         }
 
@@ -459,6 +446,62 @@ impl Drop for WebGLProgram {
     }
 }
 
+fn validate_glsl_name(name: &str) -> WebGLResult<bool> {
+    if name.is_empty() {
+        return Ok(false);
+    }
+    if name.len() > MAX_UNIFORM_AND_ATTRIBUTE_LEN {
+        return Err(WebGLError::InvalidValue);
+    }
+    for c in name.chars() {
+        validate_glsl_char(c)?;
+    }
+    if name.starts_with("webgl_") || name.starts_with("_webgl_") {
+        return Err(WebGLError::InvalidOperation);
+    }
+    Ok(true)
+}
+
+fn validate_glsl_char(c: char) -> WebGLResult<()> {
+    match c {
+        'a'..='z' |
+        'A'..='Z' |
+        '0'..='9' |
+        ' ' |
+        '\t' |
+        '\u{11}' |
+        '\u{12}' |
+        '\r' |
+        '\n' |
+        '_' |
+        '.' |
+        '+' |
+        '-' |
+        '/' |
+        '*' |
+        '%' |
+        '<' |
+        '>' |
+        '[' |
+        ']' |
+        '(' |
+        ')' |
+        '{' |
+        '}' |
+        '^' |
+        '|' |
+        '&' |
+        '~' |
+        '=' |
+        '!' |
+        ':' |
+        ';' |
+        ',' |
+        '?' => Ok(()),
+        _ => Err(WebGLError::InvalidValue),
+    }
+}
+
 fn parse_uniform_name(name: &str) -> Option<(&str, Option<i32>)> {
     if !name.ends_with(']') {
         return Some((name, None));
@@ -469,3 +512,5 @@ fn parse_uniform_name(name: &str) -> Option<(&str, Option<i32>)> {
         .ok()?;
     Some((&name[..bracket_pos], Some(index)))
 }
+
+pub const MAX_UNIFORM_AND_ATTRIBUTE_LEN: usize = 256;
diff --git a/components/script/dom/webglrenderingcontext.rs b/components/script/dom/webglrenderingcontext.rs
index f299b3ff2e4..2ee227427e3 100644
--- a/components/script/dom/webglrenderingcontext.rs
+++ b/components/script/dom/webglrenderingcontext.rs
@@ -77,8 +77,6 @@ pub fn is_gles() -> bool {
     cfg!(any(target_os = "android", target_os = "ios"))
 }
 
-pub const MAX_UNIFORM_AND_ATTRIBUTE_LEN: usize = 256;
-
 // From the GLES 2.0.25 spec, page 85:
 //
 //     "If a texture that is currently bound to one of the targets
diff --git a/tests/wpt/webgl/meta/conformance/misc/invalid-passed-params.html.ini b/tests/wpt/webgl/meta/conformance/misc/invalid-passed-params.html.ini
index 6f42f87c602..130b507cef4 100644
--- a/tests/wpt/webgl/meta/conformance/misc/invalid-passed-params.html.ini
+++ b/tests/wpt/webgl/meta/conformance/misc/invalid-passed-params.html.ini
@@ -1,71 +1,17 @@
 [invalid-passed-params.html]
-  bug: https://github.com/servo/servo/issues/21287
+  bug: https://github.com/servo/servo/issues/21754
   [WebGL test #44: context.getError() should be 1281. Was 0.]
     expected: FAIL
 
-  [WebGL test #55: context.getError() should be 1281. Was 0.]
-    expected: FAIL
-
-  [WebGL test #58: context.getError() should be 1281. Was 0.]
-    expected: FAIL
-
-  [WebGL test #53: context.getError() should be 1281. Was 0.]
-    expected: FAIL
-
-  [WebGL test #46: context.getError() should be 1281. Was 0.]
-    expected: FAIL
-
-  [WebGL test #62: context.getError() should be 1281. Was 0.]
-    expected: FAIL
-
-  [WebGL test #48: context.getError() should be 1281. Was 0.]
-    expected: FAIL
-
-  [WebGL test #50: context.getError() should be 1281. Was 0.]
-    expected: FAIL
-
   [WebGL test #34: context.getError() should be 1281. Was 0.]
     expected: FAIL
 
-  [WebGL test #59: context.getError() should be 1281. Was 0.]
-    expected: FAIL
-
-  [WebGL test #51: context.getError() should be 1281. Was 0.]
-    expected: FAIL
-
-  [WebGL test #47: context.getError() should be 1281. Was 0.]
-    expected: FAIL
-
-  [WebGL test #57: context.getError() should be 1281. Was 0.]
-    expected: FAIL
-
-  [WebGL test #49: context.getError() should be 1281. Was 0.]
-    expected: FAIL
-
   [WebGL test #42: context.getError() should be 1281. Was 0.]
     expected: FAIL
 
   [WebGL test #36: context.getError() should be 1281. Was 0.]
     expected: FAIL
 
-  [WebGL test #60: context.getError() should be 1281. Was 0.]
-    expected: FAIL
-
-  [WebGL test #56: context.getError() should be 1281. Was 0.]
-    expected: FAIL
-
-  [WebGL test #54: context.getError() should be 1281. Was 0.]
-    expected: FAIL
-
-  [WebGL test #61: context.getError() should be 1281. Was 0.]
-    expected: FAIL
-
-  [WebGL test #45: context.getError() should be 1281. Was 0.]
-    expected: FAIL
-
-  [WebGL test #52: context.getError() should be 1281. Was 0.]
-    expected: FAIL
-
   [WebGL test #40: context.getError() should be 1281. Was 0.]
     expected: FAIL