Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Infinite Loop on Express Test #107

Closed
johnnyman727 opened this issue Jun 7, 2014 · 11 comments
Closed

Infinite Loop on Express Test #107

johnnyman727 opened this issue Jun 7, 2014 · 11 comments

Comments

@johnnyman727
Copy link
Contributor

See my comment regarding the Express test in the colony test suite.

@kevinswiber
Copy link
Contributor

It may have to do with having both a split and replace with regular expressions on the same line.

See here: https://github.com/broofa/node-mime/blob/48cbcf979b4d1980098217e275f03da33a311c5e/mime.js#L59

var fields = line.replace(/\s*#.*|^\s*|\s*$/g, '').split(/\s+/);

@kevinswiber
Copy link
Contributor

Ah, scratch that. Looks like everything's like getting properly broken into an AST at some point.

There's something unfriendly about the expression itself, I think.

@kevinswiber
Copy link
Contributor

Currently Googling "JavaScript compatible regular expression C library" with a stern look on my face. :/

I'm hoping the issue is easier to solve than that.

@kevinswiber
Copy link
Contributor

Sorry for the tweet-style issue commenting. If it does come down to needing a regex engine that's JavaScript compatible... I just remembered a project called Duktape. They have a regex engine.

Site: http://duktape.org
Code: https://github.com/svaarala/duktape/tree/master/src

That may be an option.

@kevinswiber
Copy link
Contributor

Okay... confirmed this doesn't work in colony, though I didn't try using hsregex directly.

console.log('Hello World'.replace(/\s|l*/g, ''));

In colony, that's an infinite loop.

In Node, it's "HeoWord".

And for the record, it also appears to work properly in Duktape. Here's the gist I used to test it: https://gist.github.com/kevinswiber/d04bd75725713d42eec2

@kevinswiber
Copy link
Contributor

As you can read in the description of the issue linked below, I need an independent, JavaScript-compatible regular expression parser for my own needs, but if you were interested in using Duktape's implementation, as well, the author gives some advice here: svaarala/duktape#12 (comment)

Provided I get the time to attempt this, I can always circle back around to Tessel at a later date and see if it plugs in to the runtime nicely. Otherwise, there's likely a quicker fix for this specific issue.

tcr added a commit that referenced this issue Jun 9, 2014
tcr added a commit that referenced this issue Jun 9, 2014
Fixes zero-length regexp replace. See #107
@kevinswiber
Copy link
Contributor

I think #113 fixes this issue of the infinite loop. However, the express sample is still not running. More info being added to #110.

@kevinswiber
Copy link
Contributor

The mime module is trying to split its data file contents by line using this regular expression: [\r\n]+. The result of the split is wacky and breaks the module, thereby breaking the Content-Type header returned by Express.

More details here: #110 (comment)

@johnnyman727
Copy link
Contributor Author

Just as an update to anyone watching this issue, the Express test no longer goes into an infinite loop but it does not parse paths correctly.

@kevinswiber
Copy link
Contributor

This is related to #108. Trouble line: https://github.com/expressjs/parseurl/blob/master/index.js#L17

@johnnyman727
Copy link
Contributor Author

Express is hitting some other compat issues. Currently, upon require:

➜  sandbox  tessel run server.js
TESSEL! Connected to TM-00-04-f000da30-005d4344-60782586.
INFO Bundling directory /Users/Jon/Work/technical/sandbox (~710.25 KB)
INFO Deploying bundle (1.13 MB)...
INFO Running script...
[T]: src/colony/lua/colony-js.lua:906: bad argument #2 to 'f' (number expected, got nil)

I'm going to close this issue in favor of more specific Node/JS compat issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants