googlearchive / caja

Caja is a tool for safely embedding third party HTML, CSS and JavaScript in your website.
Apache License 2.0
1.13k stars 127 forks source link

Code Review: *jasvir/jsfunfuzz-original@1500 #317

Closed kpreid closed 9 years ago

kpreid commented 9 years ago

Original issue 317 created by jasvir on 2008-05-27T02:05:55.000Z:

Description:

Added jsfunfuzz and hooks to call the parser using it to flush out errors parsing.

Affected Paths: A //trunk/src/javatests/com/google/caja/parser/js/JsFunFuzz.java A //trunk/src/third_party/js/jsfunfuzz A //trunk/src/third_party/js/jsfunfuzz/about.txt A //trunk/src/third_party/js/jsfunfuzz/jsfunfuzz.html A //trunk/src/third_party/js/jsfunfuzz/jsfunfuzz.js A //trunk/src/third_party/js/jsfunfuzz/multi_timed_run.py A //trunk/src/third_party/js/jsfunfuzz/using.txt

kpreid commented 9 years ago

Comment #1 originally posted by jasvir on 2008-05-27T02:07:57.000Z:

Note I had to make some changes to jsfunfuzz.js itself to make it work with Rhino. Since this is third_party code, this is not great but I am not sure how else to do this.

kpreid commented 9 years ago

Comment #2 originally posted by mikesamuel on 2008-05-27T03:06:29.000Z:

<empty>

kpreid commented 9 years ago

Comment #3 originally posted by mikesamuel on 2008-05-27T03:16:51.000Z:

changes/jasvir/jsfunfuzz/trunk/src/third_party/js/jsfunfuzz/about.txt@1465 Seems to be missing linebreaks

changes/jasvir/jsfunfuzz/trunk/src/third_party/js/jsfunfuzz/jsfunfuzz.js@1465 You mentioned changes. If this has changes, can we check in the original first, and then put the changes in as a separate CL?

changes/jasvir/jsfunfuzz/trunk/src/third_party/js/jsfunfuzz/multi_timed_run.py@1465 From the python style guide: Although you may see this in a lot of current code, do not use #!/usr/bin/env python2.4. Due to a complex interaction between the kernel and killall, scripts using /usr/bin/env cannot be reliably killed via killall.

kpreid commented 9 years ago

Comment #4 originally posted by mikesamuel on 2008-05-27T04:27:13.000Z:

<empty>

kpreid commented 9 years ago

Comment #5 originally posted by jasvir on 2008-05-30T01:18:41.000Z:

kpreid commented 9 years ago

Comment #6 originally posted by mikesamuel on 2008-05-30T03:30:43.000Z:

Please remove the svn:executable property from all these files. svn propdel svn:executable should do it.

Then the original stuff LGTM. I'll review the new stuff after you've merged with the original code.

kpreid commented 9 years ago

Comment #7 originally posted by jasvir on 2008-05-30T17:35:15.000Z:

<empty>