Commit 592c4269 authored by kbr@google.com's avatar kbr@google.com
Browse files

2011-01-18 Kenneth Russell <kbr@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@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
parent c5050644
2011-01-18 Kenneth Russell <kbr@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@apple.com>
 
Reviewed by Simon Fraser.
......
......@@ -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
......
......@@ -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
......
......@@ -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");
}
......
implicit/00_shaders.txt
misc/00_shaders.txt
reserved/00_shaders.txt
// Non ascii comments in source should succeed
// これはASCIIではないです。
// This Is Not ASCII
/*
* This Is Not ASCII
*/
void main() {
gl_Position = vec4(1,1,1,1);
}
// Non ascii data in source should fail
// See GLSL ES Spec 1.0.17 section 3.1 and 3.2
// これはASCIIではないです。
// This Is Not ASCII
uniform mat4 NotASCII;
void main() {
gl_Position = vec4(1,1,1,1);
}
2011-01-18 Kenneth Russell <kbr@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@webkit.org>
 
Reviewed by Eric Seidel.
......@@ -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);
}
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment