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

Setup script : infinite loop #1049

Closed
adamotte opened this issue Feb 17, 2014 · 18 comments
Closed

Setup script : infinite loop #1049

adamotte opened this issue Feb 17, 2014 · 18 comments
Assignees
Labels
Milestone

Comments

@adamotte
Copy link

Environment: Raspberry pi type B with
node: 0.10.25
npm: 1.3.24
redis-server: 2.8.6
nodebb: 0.3.2

When i run the following command:

./nodebb setup

I obtain the error:

(node) warning: Recursive process.nextTick detected. This will break in the next version of node. Please use setImmediate for recursive deferral.
RangeError: Maximum call stack size exceeded

Warning display 464 times.

Setup trace:

**info:** NodeBB v0.3.2 Copyright (C) 2013 DesignCreatePlay Inc.
**info:** This program comes with ABSOLUTELY NO WARRANTY.
**info:** This is free software, and you are welcome to redistribute it under certain conditions.
**info:**
**info:** NodeBB Setup Triggered via Command Line
**info:** Welcome to NodeBB!
**info:** This looks like a new installation, so you'll have to answer a few questions about your environment before we can proceed.
**info:** Press enter to accept the default setting (shown in brackets).
URL of this installation (http://localhost)
Port number of your NodeBB (4567)
Use a port number to access NodeBB? (y)
Please enter a NodeBB secret (password)
IP or Hostname to bind to (0.0.0.0)
Which database to use (redis)
Host IP or address of your Redis instance (127.0.0.1)
Host port of your Redis instance (6379)
Password of your Redis database password
Which database to use (0..n) (0)
**info:** Configuration Saved OK
**info:** Populating database with default configs, if not already set...
warn: No administrators have been detected, running initial user setup
Administrator username admin
Administrator email address admin@mail.com
Password
Confirm Password
(node) warning: Recursive process.nextTick detected. This will break in the next version of node. Please use setImmediate for recursive deferral.
...
RangeError: Maximum call stack size exceeded
@julianlam
Copy link
Member

Time to dust off my RPi. What OS are you using? I'll download Raspbian and give it a whirl.

@julianlam julianlam added this to the 0.4.0 milestone Feb 17, 2014
@julianlam julianlam self-assigned this Feb 17, 2014
@julianlam julianlam added the bug label Feb 17, 2014
@adamotte
Copy link
Author

I'm on raspbian. Up to date
Excellent work and project by the way ;)

@julianlam
Copy link
Member

Thanks @adamotte! Still compiling Node.js... ⌚

Edit: Done now, only took two hours and 20 minutes

Edit 2: Reproduced on RPi.

@julianlam
Copy link
Member

I'm thinking it's related to bcrypt... the error comes up when attempting to hash a password prior to storage.

@adamotte
Copy link
Author

Ok @julianlam do you have an alternative?

@julianlam
Copy link
Member

At present, unsure whether the problem lies upstream or whether it's
NodeBB. While I have no doubt it'll run on an RPi, are you sure you want
to? It's a tad slow 😄
On Feb 19, 2014 3:26 PM, "Anthony Damotte" notifications@github.com wrote:

Ok @julianlam https://github.com/julianlam do you have an alternative?


Reply to this email directly or view it on GitHubhttps://github.com//issues/1049#issuecomment-35544304
.

@adamotte
Copy link
Author

With your trick, i found this post bcript fix for ghost on RPi, adapt to nodebb and that's work !

Do you want a pull-request?

@akhoury
Copy link
Member

akhoury commented Feb 19, 2014

@adamotte ahh .. NodeBB just migrated from bcrypt to use bcryptjs

@julianlam config? just like redis vs mongo? lol

EDIT: RPi really?

@adamotte
Copy link
Author

I didn't notice that nodebb just migrated. Maybe you could just write it on a wiki for RPi user.

@julianlam
Copy link
Member

This still happens on latest code with bcrypt-nodejs, afaict
On Feb 19, 2014 4:21 PM, "Anthony Damotte" notifications@github.com wrote:

I didn't notice that nodebb just migrated. Maybe you could just write it
on a wiki for RPi user.


Reply to this email directly or view it on GitHubhttps://github.com//issues/1049#issuecomment-35550369
.

@akhoury
Copy link
Member

akhoury commented Feb 19, 2014

@adamotte I have a theory, but I can't test it, I don't have a RPi - I had this issue with the import plugin, where async call were going going sync, even with process.nextTick so I had to use setTimeout on them, which, did it for me

can you try the following:

# find this file and edit it
vim NodeBB/node_modules/bcryptjs/bcrypt.js

find this function _nextTick definition, probably line 561'ish, and change it

FROM

function _nextTick(callback) {
        if (typeof process !== 'undefined' && typeof process.nextTick === 'function') {
            process.nextTick(callback);
        } else {
            setTimeout(callback, 0);
        }
    }

TO

function _nextTick(callback) {
            setTimeout(callback, 0);
  }

unrelated note

@Julian,looking at the bcryptjs hash function

bcrypt.hash = function(s, salt, callback) {
        if (typeof callback != 'function') {
            throw(new Error("Illegal 'callback': "+callback));
        }
        if (typeof salt == 'number') {
            bcrypt.genSalt(salt, function(err, salt) {
                _hash(s, salt, callback);
            });
        } else {
            _hash(s, salt, callback);
        }
    };

calling hash() with a number, will call genSalt(), so you do not need to call genSalt() then hash(), ,

    bcrypt.hash(password, nconf.get('bcrypt_rounds'), callback);

@barisusakli
Copy link
Member

nconf.get('bcrpyt_rounds') is a string

typeof '12' == 'number'
false

javascript is scary. Imagine we put that in and then 12 is used as the salt for everything 😀

bcrypt.hash(password, parseInt(nconf.get('bcrypt_rounds'), 10), callback);

FTW.

@akhoury
Copy link
Member

akhoury commented Feb 19, 2014

ugh, yea, I keep forgetting that nconf, but to be fair, it shouldnt be, the nconf config you be normalized, so the meta.config values, no more '1' vs 1 or '0' vs 0, there is places in the code where i've seen if (meta.config.areYouAWizard === '0') or if (meta.config.iCanHazTehCode === '1') that's not cool, these should falsy and truthy, '0' is truthy

@barisusakli
Copy link
Member

Haha well after I typed that i went and checked turns out it is a number after all but doesn't hurt to be careful. After working with redis so long I tend to be extra cautious.

@julianlam
Copy link
Member

If you'd like to try the fix outlined in the Ghost blog (the technologies Ghost and NodeBB use are very similar, after all...), you can give it a try and let us know how it goes!

  1. npm install bcrypt
  2. Replace bcryptjs with bcrypt in line 1 of /src/user.js
  3. Replace bcryptjs with bcrypt in line 5 of /src/routes/authentication.js

Edit: Confirmed that the above steps work.

@julianlam
Copy link
Member

I've opened an issue upstream. In the meantime, the workaround will have to suffice.

@julianlam
Copy link
Member

@adamotte -- can you give v0.7.11 of bcryptjs a whirl? Let me know if this is still an issue.

@adamotte
Copy link
Author

I confirmed that your steps work like a charm.

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

No branches or pull requests

4 participants