WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
49576
jsc does not ignore shebang
https://bugs.webkit.org/show_bug.cgi?id=49576
Summary
jsc does not ignore shebang
John Tantalo
Reported
2010-11-15 20:42:12 PST
Created
attachment 73956
[details]
JavaScript source with shebang. Various other JavaScript engines [1] correctly ignore a shebang [2] as the first line of a file. Invoking jsc with a source file that contains a shebang generates a lex error. It should instead ignore the shebang. [1] nodejs, narwhal, ringo [2]
http://en.wikipedia.org/wiki/Shebang_(Unix
)
Attachments
JavaScript source with shebang.
(32 bytes, text/javascript)
2010-11-15 20:42 PST
,
John Tantalo
no flags
Details
svn diff JavaScriptCore/parser/Lexer.cpp
(1.59 KB, patch)
2010-11-15 20:55 PST
,
John Tantalo
no flags
Details
Formatted Diff
Diff
proposed patch
(2.68 KB, patch)
2010-11-15 21:49 PST
,
John Tantalo
ggaren
: review-
Details
Formatted Diff
Diff
proposed patch
(1.00 KB, patch)
2010-11-16 11:18 PST
,
John Tantalo
no flags
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
John Tantalo
Comment 1
2010-11-15 20:55:41 PST
Created
attachment 73957
[details]
svn diff JavaScriptCore/parser/Lexer.cpp I added a new character type for a hash, i.e., #, ASCII 35. When this character was previously encountered, it would have immediately triggered a lexical error. My change is to first check if this character is at the start of the line. If not, it is an error. Next, I check if the next character is a bang, i.e,. !, ASCII 33. If it is, then the rest of the line is treated as a comment. If not, then it is an error. Since the hash character was previously illegal, the only potential danger of this patch is that source which previously failed to lex may now be executed This patch does not enforce that the shebang occurs on the first line of the source. Any occurrence of a hash other than the first character of the source should be illegal. I don't know how to do this, so I will submit my current patch as a viable solution to the bug.
John Tantalo
Comment 2
2010-11-15 21:49:31 PST
Created
attachment 73960
[details]
proposed patch Replaced diff with a real patch, with a test, changelog, and correct style.
Geoffrey Garen
Comment 3
2010-11-16 10:58:58 PST
Comment on
attachment 73960
[details]
proposed patch I think you've made this change at the wrong level of abstraction. Accounting for shebang in the lexer applies this Unix-only rule to all web pages and operating systems. I think a better fix would be for the jsc tool itself to strip out shebang before evaluating a source file.
John Tantalo
Comment 4
2010-11-16 11:18:27 PST
Created
attachment 74017
[details]
proposed patch This patch modifies jsc's file load function to detect a shebang in the first two characters of the file buffer. If it finds a shebang, it replaces it with "//", which causes the rest of the line to be interpreted as a JavaScript comment.
Geoffrey Garen
Comment 5
2010-11-29 11:40:03 PST
Comment on
attachment 74017
[details]
proposed patch r=me
WebKit Commit Bot
Comment 6
2010-12-06 10:47:50 PST
Comment on
attachment 74017
[details]
proposed patch Clearing flags on attachment: 74017 Committed
r73377
: <
http://trac.webkit.org/changeset/73377
>
WebKit Commit Bot
Comment 7
2010-12-06 10:47:56 PST
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug