[SCM] WebKit Debian packaging branch, webkit-1.3, updated. upstream/1.3.7-4207-g178b198
kbr at google.com
kbr at google.com
Sun Feb 20 23:11:57 UTC 2011
The following commit has been merged in the webkit-1.3 branch:
commit 592c42697d4ffa12bc130c62c8b321d5c685d5ec
Author: kbr at google.com <kbr at google.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date: Tue Jan 18 22:11:48 2011 +0000
2011-01-18 Kenneth Russell <kbr at google.com>
Reviewed by David Levin.
Must strip comments from WebGL shaders before enforcing character set
https://bugs.webkit.org/show_bug.cgi?id=52390
Strip comments from incoming shaders, preserving line numbers,
before validating that they conform to the ESSL character set.
Revert changes from http://trac.webkit.org/changeset/75735 which
allowed invalid characters to be passed to certain APIs.
Tested with WebGL layout tests, conformance test suite and several
WebGL demos in both Safari and Chromium.
* html/canvas/WebGLRenderingContext.cpp:
(WebCore::StripComments::StripComments::process):
(WebCore::WebGLRenderingContext::shaderSource):
2011-01-18 Kenneth Russell <kbr at google.com>
Reviewed by David Levin.
Must strip comments from WebGL shaders before enforcing character set
https://bugs.webkit.org/show_bug.cgi?id=52390
Incorporated non-ASCII GLSL conformance tests from Khronos
repository. Updated and synchronized invalid-passed-params.html
with Khronos repository, undoing changes from
http://trac.webkit.org/changeset/75735 .
* fast/canvas/webgl/glsl-conformance-expected.txt:
* fast/canvas/webgl/invalid-passed-params-expected.txt:
* fast/canvas/webgl/invalid-passed-params.html:
* fast/canvas/webgl/shaders/00_shaders.txt:
* fast/canvas/webgl/shaders/misc: Added.
* fast/canvas/webgl/shaders/misc/00_shaders.txt: Added.
* fast/canvas/webgl/shaders/misc/non-ascii-comments.vert: Added.
* fast/canvas/webgl/shaders/misc/non-ascii.vert: Added.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@76063 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 1da16d6..5fcdaee 100644
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,24 @@
+2011-01-18 Kenneth Russell <kbr at google.com>
+
+ Reviewed by David Levin.
+
+ Must strip comments from WebGL shaders before enforcing character set
+ https://bugs.webkit.org/show_bug.cgi?id=52390
+
+ Incorporated non-ASCII GLSL conformance tests from Khronos
+ repository. Updated and synchronized invalid-passed-params.html
+ with Khronos repository, undoing changes from
+ http://trac.webkit.org/changeset/75735 .
+
+ * fast/canvas/webgl/glsl-conformance-expected.txt:
+ * fast/canvas/webgl/invalid-passed-params-expected.txt:
+ * fast/canvas/webgl/invalid-passed-params.html:
+ * fast/canvas/webgl/shaders/00_shaders.txt:
+ * fast/canvas/webgl/shaders/misc: Added.
+ * fast/canvas/webgl/shaders/misc/00_shaders.txt: Added.
+ * fast/canvas/webgl/shaders/misc/non-ascii-comments.vert: Added.
+ * fast/canvas/webgl/shaders/misc/non-ascii.vert: Added.
+
2011-01-18 Chris Marrin <cmarrin at apple.com>
Reviewed by Simon Fraser.
diff --git a/LayoutTests/fast/canvas/webgl/glsl-conformance-expected.txt b/LayoutTests/fast/canvas/webgl/glsl-conformance-expected.txt
index 9320ca9..125240f 100644
--- a/LayoutTests/fast/canvas/webgl/glsl-conformance-expected.txt
+++ b/LayoutTests/fast/canvas/webgl/glsl-conformance-expected.txt
@@ -94,6 +94,8 @@ PASS [shaders/implicit/ternary_int_float.vert/fshader]: implicit cast of int to
PASS [shaders/implicit/ternary_ivec2_vec2.vert/fshader]: implicit cast of ivec2 to vec2 in ternary expression should fail
PASS [shaders/implicit/ternary_ivec3_vec3.vert/fshader]: implicit cast of ivec3 to vec3 in ternary expression should fail
PASS [shaders/implicit/ternary_ivec4_vec4.vert/fshader]: implicit cast of ivec4 to vec4 in ternary expression should fail
+PASS [shaders/misc/non-ascii.vert/fshader]: Non ascii data in source should fail
+PASS [shaders/misc/non-ascii-comments.vert/fshader]: Non ascii comments in source should succeed
PASS [shaders/reserved/_webgl_field.vert/fshader]: use of reserved _webgl prefix as structure field should fail
PASS [shaders/reserved/_webgl_function.vert/fshader]: use of reserved _webgl prefix as function name should fail
PASS [shaders/reserved/_webgl_struct.vert/fshader]: use of reserved _webgl prefix as structure name should fail
diff --git a/LayoutTests/fast/canvas/webgl/invalid-passed-params-expected.txt b/LayoutTests/fast/canvas/webgl/invalid-passed-params-expected.txt
index 31d01c0..3bdc9c5 100644
--- a/LayoutTests/fast/canvas/webgl/invalid-passed-params-expected.txt
+++ b/LayoutTests/fast/canvas/webgl/invalid-passed-params-expected.txt
@@ -63,24 +63,42 @@ PASS context.getError() is context.NO_ERROR
PASS context.getError() is context.NO_ERROR
Test shaderSource() with invalid characters
+PASS context.getError() is context.NO_ERROR
PASS context.getError() is context.INVALID_VALUE
+PASS context.getError() is context.NO_ERROR
PASS context.getError() is context.INVALID_VALUE
+PASS context.getError() is context.NO_ERROR
+PASS context.getError() is context.INVALID_VALUE
+PASS context.getError() is context.NO_ERROR
+PASS context.getError() is context.INVALID_VALUE
+PASS context.getError() is context.NO_ERROR
+PASS context.getError() is context.INVALID_VALUE
+PASS context.getError() is context.NO_ERROR
PASS context.getError() is context.INVALID_VALUE
Test bindAttribLocation() with invalid characters
PASS context.getError() is context.INVALID_VALUE
PASS context.getError() is context.INVALID_VALUE
PASS context.getError() is context.INVALID_VALUE
+PASS context.getError() is context.INVALID_VALUE
+PASS context.getError() is context.INVALID_VALUE
+PASS context.getError() is context.INVALID_VALUE
Test getAttribLocation() with invalid characters
PASS context.getError() is context.INVALID_VALUE
PASS context.getError() is context.INVALID_VALUE
PASS context.getError() is context.INVALID_VALUE
+PASS context.getError() is context.INVALID_VALUE
+PASS context.getError() is context.INVALID_VALUE
+PASS context.getError() is context.INVALID_VALUE
Test getUniformLocation() with invalid characters
PASS context.getError() is context.INVALID_VALUE
PASS context.getError() is context.INVALID_VALUE
PASS context.getError() is context.INVALID_VALUE
+PASS context.getError() is context.INVALID_VALUE
+PASS context.getError() is context.INVALID_VALUE
+PASS context.getError() is context.INVALID_VALUE
PASS successfullyParsed is true
diff --git a/LayoutTests/fast/canvas/webgl/invalid-passed-params.html b/LayoutTests/fast/canvas/webgl/invalid-passed-params.html
index 59f6cd4..0bac52b 100644
--- a/LayoutTests/fast/canvas/webgl/invalid-passed-params.html
+++ b/LayoutTests/fast/canvas/webgl/invalid-passed-params.html
@@ -77,17 +77,27 @@ shouldGenerateGLError(context, context.NO_ERROR, "context.viewport(0, 0, 16, 16)
debug("");
debug("Set up a program to test invalid characters");
-var invalidSet = ['$', '@', '\\'];
+var invalidSet = ['"', '$', '`', '@', '\\', "'"];
var validUniformName = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ_1234567890";
var validAttribName = "abcdefghijklmnopqrstuvwxyz_ABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890";
-var validShaderSource = "uniform float " + validUniformName + ";\n"
- + "varying float " + validAttribName + ";\n"
- + "void main() {\n"
- + validAttribName + " = " + validUniformName + ";\n"
- + "gl_Position = vec4(0.0, 0.0, 0.0, 1.0); }\n";
- + "//.+-/*%<>[](){}^|&~=!:;,?# ";
+function generateShaderSource(opt_invalidIdentifierChar, opt_invalidCommentChar) {
+ var invalidIdentifierString = "";
+ var invalidCommentString = "";
+ if (opt_invalidIdentifierChar != undefined) {
+ invalidIdentifierString += opt_invalidIdentifierChar;
+ }
+ if (opt_invalidCommentChar != undefined) {
+ invalidCommentString += opt_invalidCommentChar;
+ }
+ return "uniform float " + validUniformName + invalidIdentifierString + ";\n"
+ + "varying float " + validAttribName + ";\n"
+ + "void main() {\n"
+ + validAttribName + " = " + validUniformName + ";\n"
+ + "gl_Position = vec4(0.0, 0.0, 0.0, 1.0); }\n";
+ + "//.+-/*%<>[](){}^|&~=!:;,?# " + invalidCommentString;
+}
var vShader = context.createShader(context.VERTEX_SHADER);
-context.shaderSource(vShader, validShaderSource);
+context.shaderSource(vShader, generateShaderSource());
context.compileShader(vShader);
shouldBe("context.getError()", "context.NO_ERROR");
var fShader = context.createShader(context.FRAGMENT_SHADER);
@@ -113,7 +123,10 @@ shouldBe("context.getError()", "context.NO_ERROR");
debug("");
debug("Test shaderSource() with invalid characters");
for (var i = 0; i < invalidSet.length; ++i) {
- var invalidShaderSource = validShaderSource + "\n//" + invalidSet[i];
+ var validShaderSource = generateShaderSource(undefined, invalidSet[i]);
+ context.shaderSource(vShader, validShaderSource);
+ shouldBe("context.getError()", "context.NO_ERROR");
+ var invalidShaderSource = generateShaderSource(invalidSet[i], undefined);
context.shaderSource(vShader, invalidShaderSource);
shouldBe("context.getError()", "context.INVALID_VALUE");
}
diff --git a/LayoutTests/fast/canvas/webgl/shaders/00_shaders.txt b/LayoutTests/fast/canvas/webgl/shaders/00_shaders.txt
index 31dcaea..32dfe2d 100644
--- a/LayoutTests/fast/canvas/webgl/shaders/00_shaders.txt
+++ b/LayoutTests/fast/canvas/webgl/shaders/00_shaders.txt
@@ -1,3 +1,4 @@
implicit/00_shaders.txt
+misc/00_shaders.txt
reserved/00_shaders.txt
diff --git a/LayoutTests/fast/canvas/webgl/shaders/misc/00_shaders.txt b/LayoutTests/fast/canvas/webgl/shaders/misc/00_shaders.txt
new file mode 100644
index 0000000..3bcf92e
--- /dev/null
+++ b/LayoutTests/fast/canvas/webgl/shaders/misc/00_shaders.txt
@@ -0,0 +1,3 @@
+non-ascii.vert
+non-ascii-comments.vert
+
diff --git a/LayoutTests/fast/canvas/webgl/shaders/misc/non-ascii-comments.vert b/LayoutTests/fast/canvas/webgl/shaders/misc/non-ascii-comments.vert
new file mode 100644
index 0000000..2f906a8
--- /dev/null
+++ b/LayoutTests/fast/canvas/webgl/shaders/misc/non-ascii-comments.vert
@@ -0,0 +1,9 @@
+// Non ascii comments in source should succeed
+// ‚±‚ê‚Í‚`‚r‚b‚h‚h‚Å‚Í‚È‚¢‚Å‚·B
+// ‚s‚ˆ‚‰‚“@‚h‚“@‚m‚‚”@‚`‚r‚b‚h‚h
+/*
+ * ‚s‚ˆ‚‰‚“@‚h‚“@‚m‚‚”@‚`‚r‚b‚h‚h
+ */
+void main() {
+ gl_Position = vec4(1,1,1,1);
+}
diff --git a/LayoutTests/fast/canvas/webgl/shaders/misc/non-ascii.vert b/LayoutTests/fast/canvas/webgl/shaders/misc/non-ascii.vert
new file mode 100644
index 0000000..cc80bc4
--- /dev/null
+++ b/LayoutTests/fast/canvas/webgl/shaders/misc/non-ascii.vert
@@ -0,0 +1,8 @@
+// Non ascii data in source should fail
+// See GLSL ES Spec 1.0.17 section 3.1 and 3.2
+// ‚±‚ê‚Í‚`‚r‚b‚h‚h‚Å‚Í‚È‚¢‚Å‚·B
+// ‚s‚ˆ‚‰‚“@‚h‚“@‚m‚‚”@‚`‚r‚b‚h‚h
+uniform mat4 ‚m‚‚”‚`‚r‚b‚h‚h;
+void main() {
+ gl_Position = vec4(1,1,1,1);
+}
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index ab64377..6efc1e9 100644
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,22 @@
+2011-01-18 Kenneth Russell <kbr at google.com>
+
+ Reviewed by David Levin.
+
+ Must strip comments from WebGL shaders before enforcing character set
+ https://bugs.webkit.org/show_bug.cgi?id=52390
+
+ Strip comments from incoming shaders, preserving line numbers,
+ before validating that they conform to the ESSL character set.
+ Revert changes from http://trac.webkit.org/changeset/75735 which
+ allowed invalid characters to be passed to certain APIs.
+
+ Tested with WebGL layout tests, conformance test suite and several
+ WebGL demos in both Safari and Chromium.
+
+ * html/canvas/WebGLRenderingContext.cpp:
+ (WebCore::StripComments::StripComments::process):
+ (WebCore::WebGLRenderingContext::shaderSource):
+
2011-01-18 Ryosuke Niwa <rniwa at webkit.org>
Reviewed by Eric Seidel.
diff --git a/Source/WebCore/html/canvas/WebGLRenderingContext.cpp b/Source/WebCore/html/canvas/WebGLRenderingContext.cpp
index c445e2b..8711910 100644
--- a/Source/WebCore/html/canvas/WebGLRenderingContext.cpp
+++ b/Source/WebCore/html/canvas/WebGLRenderingContext.cpp
@@ -62,6 +62,7 @@
#include <wtf/ByteArray.h>
#include <wtf/OwnArrayPtr.h>
#include <wtf/PassOwnArrayPtr.h>
+#include <wtf/text/StringBuilder.h>
namespace WebCore {
@@ -101,12 +102,11 @@ namespace {
// Return true if a character belongs to the ASCII subset as defined in
// GLSL ES 1.0 spec section 3.1.
- // We make exceptions for " ' `.
bool validateCharacter(unsigned char c)
{
// Printing characters are valid except " $ ` @ \ ' DEL.
if (c >= 32 && c <= 126
- && c != '$' && c != '@' && c != '\\')
+ && c != '"' && c != '$' && c != '`' && c != '@' && c != '\\' && c != '\'')
return true;
// Horizontal tab, line feed, vertical tab, form feed, carriage return
// are also valid.
@@ -115,6 +115,189 @@ namespace {
return false;
}
+ // Strips comments from shader text. This allows non-ASCII characters
+ // to be used in comments without potentially breaking OpenGL
+ // implementations not expecting characters outside the GLSL ES set.
+ class StripComments {
+ public:
+ StripComments(const String& str)
+ : m_parseState(BeginningOfLine)
+ , m_sourceString(str)
+ , m_length(str.length())
+ , m_position(0)
+ {
+ parse();
+ }
+
+ String result()
+ {
+ return m_builder.toString();
+ }
+
+ private:
+ bool hasMoreCharacters()
+ {
+ return (m_position < m_length);
+ }
+
+ void parse()
+ {
+ while (hasMoreCharacters()) {
+ process(current());
+ // process() might advance the position.
+ if (hasMoreCharacters())
+ advance();
+ }
+ }
+
+ void process(UChar);
+
+ bool peek(UChar& character)
+ {
+ if (m_position + 1 >= m_length)
+ return false;
+ character = m_sourceString[m_position + 1];
+ return true;
+ }
+
+ UChar current()
+ {
+ ASSERT(m_position < m_length);
+ return m_sourceString[m_position];
+ }
+
+ void advance()
+ {
+ ++m_position;
+ }
+
+ bool isNewline(UChar character)
+ {
+ // Don't attempt to canonicalize newline related characters.
+ return (character == '\n' || character == '\r');
+ }
+
+ void emit(UChar character)
+ {
+ m_builder.append(character);
+ }
+
+ enum ParseState {
+ // Have not seen an ASCII non-whitespace character yet on
+ // this line. Possible that we might see a preprocessor
+ // directive.
+ BeginningOfLine,
+
+ // Have seen at least one ASCII non-whitespace character
+ // on this line.
+ MiddleOfLine,
+
+ // Handling a preprocessor directive. Passes through all
+ // characters up to the end of the line. Disables comment
+ // processing.
+ InPreprocessorDirective,
+
+ // Handling a single-line comment. The comment text is
+ // replaced with a single space.
+ InSingleLineComment,
+
+ // Handling a multi-line comment. Newlines are passed
+ // through to preserve line numbers.
+ InMultiLineComment
+ };
+
+ ParseState m_parseState;
+ String m_sourceString;
+ unsigned m_length;
+ unsigned m_position;
+ StringBuilder m_builder;
+ };
+
+ void StripComments::process(UChar c)
+ {
+ if (isNewline(c)) {
+ // No matter what state we are in, pass through newlines
+ // so we preserve line numbers.
+ emit(c);
+
+ if (m_parseState != InMultiLineComment)
+ m_parseState = BeginningOfLine;
+
+ return;
+ }
+
+ UChar temp = 0;
+ switch (m_parseState) {
+ case BeginningOfLine:
+ if (WTF::isASCIISpace(c)) {
+ emit(c);
+ break;
+ }
+
+ if (c == '#') {
+ m_parseState = InPreprocessorDirective;
+ emit(c);
+ break;
+ }
+
+ // Transition to normal state and re-handle character.
+ m_parseState = MiddleOfLine;
+ process(c);
+ break;
+
+ case MiddleOfLine:
+ if (c == '/' && peek(temp)) {
+ if (temp == '/') {
+ m_parseState = InSingleLineComment;
+ emit(' ');
+ advance();
+ break;
+ }
+
+ if (temp == '*') {
+ m_parseState = InMultiLineComment;
+ // Emit the comment start in case the user has
+ // an unclosed comment and we want to later
+ // signal an error.
+ emit('/');
+ emit('*');
+ advance();
+ break;
+ }
+ }
+
+ emit(c);
+ break;
+
+ case InPreprocessorDirective:
+ // No matter what the character is, just pass it
+ // through. Do not parse comments in this state. This
+ // might not be the right thing to do long term, but it
+ // should handle the #error preprocessor directive.
+ emit(c);
+ break;
+
+ case InSingleLineComment:
+ // The newline code at the top of this function takes care
+ // of resetting our state when we get out of the
+ // single-line comment. Swallow all other characters.
+ break;
+
+ case InMultiLineComment:
+ if (c == '*' && peek(temp) && temp == '/') {
+ emit('*');
+ emit('/');
+ m_parseState = MiddleOfLine;
+ advance();
+ break;
+ }
+
+ // Swallow all other characters. Unclear whether we may
+ // want or need to just emit a space per character to try
+ // to preserve column numbers for debugging purposes.
+ break;
+ }
+ }
} // namespace anonymous
class WebGLStateRestorer {
@@ -2547,9 +2730,10 @@ void WebGLRenderingContext::shaderSource(WebGLShader* shader, const String& stri
UNUSED_PARAM(ec);
if (isContextLost() || !validateWebGLObject(shader))
return;
- if (!validateString(string))
+ String stringWithoutComments = StripComments(string).result();
+ if (!validateString(stringWithoutComments))
return;
- m_context->shaderSource(objectOrZero(shader), string);
+ m_context->shaderSource(objectOrZero(shader), stringWithoutComments);
cleanupAfterGraphicsCall(false);
}
--
WebKit Debian packaging
More information about the Pkg-webkit-commits
mailing list