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

Replace timespan with Moment and remove security warnings #22

Merged

Conversation

@Shwetajain148
Copy link

@Shwetajain148 Shwetajain148 commented Oct 10, 2017

@mchaudhary @mostlyjason, In this pull request, I have removed the timespan dependency which was causing the security vulnerability issues in node applications. The Node Security Platform shows these warnings in a node project.

This PR will fix the issue #20. Please review.

@@ -224,7 +224,7 @@ common.loggly = function () {
//
// retries to send buffered logs to loggly in every 30 seconds
//
if (timerFunctionForBufferedLogs === null) {
if (timerFunctionForBufferedLogs === null && bufferOptions) {

This comment has been minimized.

@Shwetajain148

Shwetajain148 Oct 10, 2017
Author

@mchaudhary @mostlyjason, At this point, if we are using the search functionality of our library like- (In our app.js file)

client.search('foo', function (err, results) {
  console.dir(results.events);
});

Then the library does not set the bufferOptions object and when the library tries to access the bufferOptions.retriesInMilliSeconds property, an error is thrown so I put this check to enter into this block only when the bufferOptions object is set and has some value.

@@ -258,7 +258,7 @@ common.loggly = function () {
// This function will store logs into buffer
//
function storeLogs(logs) {
if (!logs.length) return;
if (!logs.length || !bufferOptions) return;

This comment has been minimized.

@Shwetajain148

Shwetajain148 Oct 10, 2017
Author

@mchaudhary @mostlyjason The same condition lies here too. I put this check to store the logs only when the bufferOptions object is set and containing the value for retriesInMilliSeconds and size otherwise this code line will throw an error.

@Shwetajain148 Shwetajain148 changed the title Replace timespan with Moment and and remove security warnings Replace timespan with Moment and remove security warnings Oct 10, 2017
@mchaudhary mchaudhary merged commit 70698ac into loggly:master Oct 10, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.