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

DeprecationWarning: Buffer() is deprecated due to security and usability issues. #668

Open
msi-matthew opened this issue Jul 2, 2019 · 12 comments

Comments

@msi-matthew
Copy link

When trying to chain pool connections using IISNode, Express@4.16.2, and mssql@3.3.0 I get the following error

(node:11912) [DEP0005] DeprecationWarning: Buffer() is deprecated due to security and usability issues. Please use the Buffer.alloc(), Buffer.allocUnsafe(), or Buffer.from() methods instead.
(node:11912) UnhandledPromiseRejectionWarning: ReferenceError: response is not defined
    at sql.connect.then.pool (C:\inetpub\path\to\project\controllers\test.js:82:42)
    at process._tickCallback (internal/process/next_tick.js:68:7)
(node:11912) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 1)
(node:11912) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

This is happens when trying to use promises to chain requests together as follows:

exports.chain = (req, res) => {
  sql.connect(config)
    .then(pool => {
      return pool.request()
        .execute('chain')
      })
    .then(response => {
      return pool.request()
        .input('param', sql.NVarChar(300), response[0][0]['response'])
        .execute('chain2')
    })
    .then(result => { res.send(result) })
    .catch(err => { res.send(err) })
  
  sql.on('error', err => {
      // ... error handler
  })
}

I can't tell which package is throwing this error, although I think it's caused by IISNode, or the mssql package.

Does anyone have any guidance as to how to restructure these promises or which package needs to be modified to prevent this error?

I am using mssql@3.3.0 since version 4.x+ crashes using IISNode.

@nainkunal933
Copy link

@msi-matthew I think this issue is coming from your mssql package. IISNode usually does not throw errors like these. mssql@3.3.0 was released in 2016 and I would recommend updating the package. I have seen IISNode crashing on newer packages if the node.js version on the server is too old. Check your server's Node js version by using node -v. If it is old I would recommend updating the node js version along with the mssql version. This should solve this. Please let me know if it worked.

@nainkunal933
Copy link

@msi-matthew The issue can also be because of IISNode's internal code (Interceptor.js). Check this issue: Azure#60.

@msi-matthew
Copy link
Author

@nainkunal933 We're running node version 6.9.0.

Updating the interceptor.js file did eliminate the Buffer() errors in the logs, but running tests when upgrading to 5.1.0 and 6.0.0-alpha.9 all that is returned from the client is {} instead of a full response.

Also, I get App now running on localhost: undefined logged by IISNode during each call for these newer mssql packages...

@SuperstrongBE
Copy link

SuperstrongBE commented Nov 25, 2019

I've the same error.
Work fine with minimal express code like this

var express = require('express'); const fs = require('fs'); var app = express(); var http = require('https').createServer(app); app.get('/', function(req, res){ res.send('sigma.4x.ai'); }); http.listen(process.env.PORT, function(){ console.log('listening on *:80'); });

The error occur when switch from http to https

var http = require('https').createServer(app);

@savanipoojan78
Copy link

@nainkunal933 i also update the Interceptor file still i am geting error

@msi-matthew
Copy link
Author

msi-matthew commented Jan 6, 2020

@savanipoojan78 Which versions of Express and mssql are you using? It's been a long time since I've looked at this, so I can't remember all the details, but at some point Express changed its response object from response[0][0]['response'] to result.recordset. If you log result you will see the new form.

@savanipoojan78
Copy link

"dependencies": {
"asyncawait": "^3.0.0",
"express": "^4.17.1",
"mssql": "^6.0.1",
"socket.io": "^2.3.0"
}

@msi-matthew
Copy link
Author

@savanipoojan78 Please provide some code.

@fedorbirjukov
Copy link

This comment by ottomeister on this StackOverflow question should help:

iisnode itself is known to use new Buffer(), which will trigger this warning. See github.com/Azure/issues/60 and github.com/Azure/pull/62 for proposed fixes. Or see stackoverflow.com/a/53402712/1334967 for a way to silence the warning.

This issue can be closed. Issue #672 can be closed, too.

@DjovDev
Copy link

DjovDev commented May 28, 2020

Thanks for pointing that out.

Adding
<iisnode nodeProcessCommandLine="node.exe --no-deprecation --no-warnings"/>
to my web.config has successfully silenced the messagesq for me.

@ayyash
Copy link

ayyash commented Jun 25, 2020

@DjovDev yes but do you think silencing this issue is safe enough? any security side effects?

@DjovDev
Copy link

DjovDev commented Jun 25, 2020

@ayyash honestly I don't think using IISnode in general is secure, it seems to be a project that has been abandoned and deprecated in some manner. The last commit on this branch was 4 years ago. I am somewhat forced to use it as the company I work for only uses Windows based machines to host their applications on, and a few years ago it seemed like a good pick for our company.

I (and maybe you too) should probably look into using Docker as a service on windows machines as it is actively supported by a large company at the moment.

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

No branches or pull requests

7 participants