Sunday, March 29, 2009

Fixing Spidermonkey's readline()

I had occasion to try out the Spidermonkey Javascript engine today and found an annoying misfeature for which I was able to generate a fix; so I thought I'd put it out here in case other people are as annoyed by this as I was.

Because Spidermonkey is designed to run as an interactive command-line shell and not in a web browser, it includes a readline() method for getting input from the user. It's a very handy feature, but it has a bug: there's no way to tell when you've reached the end of the input stream. I'm apparently not the first person to be annoyed by this. Awesome Andy tried to fix it with a horrible hack which just made me cringe and think that there has to be a better way. There is. That's the wonderful thing about open source. If you don't like the way something works you can change it.

The Right Way to fix Spidermonkey's readline is to simply have it retain the linefeed at the end of lines. That way, empty lines have length 1. When you get to the end of file (and only then) you get a result of length zero. The fix is absolutely trivial. It's a one-line change to the file js.c:



*** js.c.~3.93.2.15.~ 2007-04-20 11:45:18.000000000 -0700
--- js.c 2009-03-29 12:46:36.000000000 -0700
***************
*** 630,635 ****
--- 630,636 ----

/* Are we done? */
if (buf[buflength - 1] == '\n') {
+ buflength++;
buf[buflength - 1] = '\0';
break;
}


That's it!

2 comments:

Mitch074 said...

Wouldn't that actually cause the same problem that made the Spidermonkey crew and distribution builders disable the file object by default?

In fact, since we need to recompile Spidermonkey for your patch, why not simply enable the file? Because, considering how ECMAscript interprets carriage returns in some cases, this may cause readLine() to be non-standard...

Ron said...

> Wouldn't that actually cause the same problem that made the Spidermonkey crew and distribution builders disable the file object by default?

What problem is that? The only problem with the file object that I'm aware of is that it's a security hole.

> In fact, since we need to recompile Spidermonkey for your patch, why not simply enable the file?

You could, but then you still couldn't read to EOF from stdin. And you'd have the aforementioned security hole.

> Because, considering how ECMAscript interprets carriage returns in some cases, this may cause readLine() to be non-standard...

Spidermonkey doesn't use ReadLine internally; it's purely a user function, and it's also an extension to the standard. So I don't see how this could be an issue.