Closed Bug 76683 Opened 25 years ago Closed 24 years ago

RegExp regression (NullPointerException)

Categories

(Rhino Graveyard :: Core, defect)

Product:
Rhino Graveyard
Component:
Core
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
VERIFIED FIXED

People

(Reporter: beard, Assigned: rogerl)

Details

Attachments

(1 file)

Proposed fix - don't assume parens[i] exists, just blow it off.
766 bytes, patch
Details | Diff | Splinter Review
Bjorn showed me a problem where the regular expression code is generating a NullPointerException, when running in their web server product. They have been using an older version of Rhino that doesn't have this problem. Bjorn, please include the date of the older Rhino you were using, so we can narrow our search for the cause of the regression. Please also include the stack crawls we generated yesterday.
If there is a RegExp testcase I could addd to the testsuite, let me know. I just ran the full testsuite under Rhino and got 0 errors -
To amplify on that: the testsuite already contains many RegExp tests, but is there a new one I could add that shows this NullPointerException?
Patrick: Good news, I found the culprit regular expression throwing the NullPointerException: /(<!--([^-]|-[^-]|--[^>])*-->)|(<(tagPattern)((([ ][^\/>]*)?\/>)|(([ ][^>]*)?>)))|(<\/tagPattern[^>]*>)/ Here is the block of code that has the expression: regexp_cache: { // used to search for any start-end tag: anySE: /(<!--([^-]|-[^-]|--[^>])*-->)|(<([\$\w:\.\-]+)((([ ][^\/>]*)?\/>)|(([ ][^>]*)?>)))/, // used to search for a generic start-end tag: genSE: /(<!--([^-]|-[^-]|--[^>])*-->)|(<(tagPattern)((([ ][^\/>]*)?\/>)|(([ ][^>]*)?>)))/, // used to search for a generic end tag: genE : /(<!--([^-]|-[^-]|--[^>])*-->)|(<(tagPattern)((([ ][^\/>]*)?\/>)|(([ ][^>]*)?>)))|(<\/tagPattern[^>]*>)/, // tag start-end regexps: tagSE: {}, // tag end regexps: tagE : {}, // attribute regexps: attr : {} }, It's in a file called parseXML.js that is a part of fast_track. Let me know if you need more information.
Here's a stack trace for matching an empty string against the regexp: js> var re = /(<!--([^-]|-[^-]|--[^>])*-->)|(<(tagPattern)((([ ][^\/>]*)?\/>)|(([ ][^>]*)?>)))|(<\/tagPattern[^>]*>)/; js> try {"".match(re);} catch(e) {e.printStackTrace();} java.lang.NullPointerException at org.mozilla.javascript.regexp.NativeRegExp.matchRENodes(NativeRegExp.java:1505) at org.mozilla.javascript.regexp.NativeRegExp.matchRegExp(NativeRegExp.java:1866) at org.mozilla.javascript.regexp.NativeRegExp.executeRegExp(NativeRegExp.java:1912) at org.mozilla.javascript.regexp.RegExpImpl.matchOrReplace(RegExpImpl.java:201) at org.mozilla.javascript.regexp.RegExpImpl.match(RegExpImpl.java:77) at org.mozilla.javascript.NativeString.jsFunction_match(NativeString.java:710) at inv2.invoke() at org.mozilla.javascript.FunctionObject.doInvoke(FunctionObject.java:579) at org.mozilla.javascript.FunctionObject.callVarargs(FunctionObject.java:594) at org.mozilla.javascript.FunctionObject.call(FunctionObject.java:448) at org.mozilla.javascript.ScriptRuntime.call(ScriptRuntime.java:1218) at org.mozilla.javascript.Interpreter.interpret(Interpreter.java:1722) at org.mozilla.javascript.InterpretedScript.call(InterpretedScript.java:67) at org.mozilla.javascript.InterpretedScript.exec(InterpretedScript.java:58) at org.mozilla.javascript.Context.evaluateReader(Context.java:778) at org.mozilla.javascript.tools.shell.Main.evaluateReader(Main.java:286) at org.mozilla.javascript.tools.shell.Main.processSource(Main.java:210) at org.mozilla.javascript.tools.shell.Main.exec(Main.java:96) at org.mozilla.javascript.tools.shell.Main.main(Main.java:66) js> The problem is in line: state.parens[i].length = 0; We know that "state" isn't null as it gets used a few lines earlier. So it's got to be parens[i] that is causing the problem. The bug could have been introduced by me in revision 1.32 on 2001/03/06 when I replicated the Spidermonkey fix for bug 67773. The conversion wasn't straightforward, so I may have broken things. Could someone please run the above through Spidermonkey to see whether that breaks too ?
Here's a transcript from Spidermonkey: js> var re = /(<!--([^-]|-[^-]|--[^>])*-->)|(<(tagPattern)((([][^\/>]*)?\/>)|(([ ][^>]*)?>)))|(<\/tagPattern[^>]*>)/; js> "".match(re) null This is running on the Mac. So, any internal bad pointerness won't be caught. I'll also try on Linux and on Mac OS X.
I get the same results running on Linux and Mac OS X. I suspect your changes in translating to Java are probably the culprit. I'll take a look at your diffs.
I seem to recall that I had to slightly alter the code when converting from C++ since Java was throwing an ArrayIndexOutOfBounds or NullPointerException somewhere (which, of course, wouldn't trouble C++ ;). Then again, this may just have been a dream.
I checked out several old trees, going back as far as revision 1.22 of NativeRegExp (25 Sep 2000). They *all* exhibit the same problem. Revision 1.29 is the first time I touched NativeRegExp, so the problem is not my fault after all. Unfortunately it's not that easy to go back further since build.xml was introduced around that time and I don't fancy building rhino by hand.
I can go back further, as I have CodeWarrior projects that go back much further. I'll have to continue to search for the regression. Bjorn, can you confirm what version of js.jar this DOESN'T happen with?
Unfortunately, I don't know how old the version of js.jar is that we were using before. But it's definitely older than Sep 25 2000. I'd say it's at least as old as Jun 2000. bjorn
I will send a build with this patch to Bjorn's team, so they can test it.
Does this mean that we should be able to have it this week? That would be good. Regarding the version of js.jar we are using as I see it some of the comments it's from Nov 17 2000.
Testcase added to JS/Rhino testsuite: js/tests/ecma_3/RegExp/regress-76683.js To run this directly from the Rhino shell, do the following: (The 'shell.js' files contain utility functions the testcase needs) [/d/JS_TRUNK/mozilla/js/rhino] java org.mozilla.javascript.tools.shell.Main js> load('../tests/ecma_3/shell.js') js> load('../tests/ecma_3/RegExp/shell.js') js> load('../tests/ecma_3/RegExp/regress-76683.js')
Without the patch to this bug applied, we see the NullPointerException in the output of the testcase: js: "D:\JS_trunk\mozilla\js\tests\ecma_3\RegExp\regress-76683.js", line 58: uncaught JavaScript exception: java.lang.NullPointerException The testcase contains the regular expressions "anySE", "genSE", "genE" above. The NullPointerException seems only to be caused by "genE" -
I committed Roger's patch: Checking in src/org/mozilla/javascript/regexp/NativeRegExp.java; /cvsroot/mozilla/js/rhino/src/org/mozilla/javascript/regexp/NativeRegExp.java,v <-- NativeRegExp.java new revision: 1.2; previous revision: 1.1 done I will post a .jar to the ftp site.
The Rhino tree is closed right now. Your checkin will get obliterated as soon as leaf copies over the revision history.
Patrick: I didn't see this but today after getting the js.jar from rhinoTip.zip I tested one of the pages that heavily uses Javascript. The whole page was messed up and fills that page with $1, $2... characters. If you like to see it and have access the internal machines please take a look at http://cleopatra.netscape.com/fast_track/ftstatus.psp. I couldn't notice it before but it happens with the version of js.jar you gave me too.
Please isolate and file a new bug. This bug will be marked fixed as soon as the Rhino tree opens again.
...which it did a while ago.
Status: NEW → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
Verified fixed on WinNT. Testcase passes in both the rhino, rhinoi shells.
Status: RESOLVED → VERIFIED
Targeting as resolved against 1.5R3
Target Milestone: --- → 1.5R3
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: