From b5fc6db75db61d9765da38e0f085dd737380f9f4 Mon Sep 17 00:00:00 2001 From: Sheogorath Date: Sat, 8 Jun 2019 20:51:24 +0200 Subject: [PATCH 01/16] Rework debug logging We have various places with overly simple if statements that could be handled by our logging library. Also a lot of those logs are not marked as debug logs but as info logs, which can cause confusion during debugging. This patch removed unneeded if clauses around debug logging statements, reworks debug log messages towards ECMA templates and add some new logging statements which might be helpful in order to debug things like image uploads. Signed-off-by: Sheogorath --- lib/history.js | 7 ++----- lib/models/revision.js | 5 ++--- lib/realtime.js | 30 ++++++++++++++---------------- lib/web/auth/ldap/index.js | 4 ++-- lib/web/auth/openid/index.js | 4 ++-- lib/web/auth/saml/index.js | 4 ++-- lib/web/auth/utils.js | 5 ++--- lib/web/imageRouter/azure.js | 8 ++++---- lib/web/imageRouter/filesystem.js | 10 +++++----- lib/web/imageRouter/imgur.js | 12 +++++------- lib/web/imageRouter/index.js | 7 ++++--- lib/web/imageRouter/lutim.js | 15 +++++++-------- lib/web/imageRouter/s3.js | 1 + lib/web/statusRouter.js | 4 +--- lib/workers/dmpWorker.js | 11 +++-------- 15 files changed, 56 insertions(+), 71 deletions(-) diff --git a/lib/history.js b/lib/history.js index 9c389bf..88a7ee0 100644 --- a/lib/history.js +++ b/lib/history.js @@ -4,7 +4,6 @@ var LZString = require('lz-string') // core -var config = require('./config') var logger = require('./logger') var response = require('./response') var models = require('./models') @@ -56,9 +55,7 @@ function getHistory (userid, callback) { } history = parseHistoryToObject(history) } - if (config.debug) { - logger.info('read history success: ' + user.id) - } + logger.debug(`read history success: ${user.id}`) return callback(null, history) }).catch(function (err) { logger.error('read history failed: ' + err) @@ -140,7 +137,7 @@ function historyPost (req, res) { var noteId = req.params.noteId if (!noteId) { if (typeof req.body['history'] === 'undefined') return response.errorBadRequest(res) - if (config.debug) { logger.info('SERVER received history from [' + req.user.id + ']: ' + req.body.history) } + logger.debug(`SERVER received history from [${req.user.id}]: ${req.body.history}`) try { var history = JSON.parse(req.body.history) } catch (err) { diff --git a/lib/models/revision.js b/lib/models/revision.js index 4ee080d..69a2bf9 100644 --- a/lib/models/revision.js +++ b/lib/models/revision.js @@ -8,7 +8,6 @@ var shortId = require('shortid') var path = require('path') // core -var config = require('../config') var logger = require('../logger') var dmpWorker = createDmpWorker() @@ -18,7 +17,7 @@ function createDmpWorker () { var worker = childProcess.fork(path.resolve(__dirname, '../workers/dmpWorker.js'), { stdio: 'ignore' }) - if (config.debug) logger.info('dmp worker process started') + logger.debug('dmp worker process started') worker.on('message', function (data) { if (!data || !data.msg || !data.cacheKey) { return logger.error('dmp worker error: not enough data on message') @@ -36,7 +35,7 @@ function createDmpWorker () { }) worker.on('close', function (code) { dmpWorker = null - if (config.debug) logger.info('dmp worker process exited with code ' + code) + logger.debug(`dmp worker process exited with code ${code}`) }) return worker } diff --git a/lib/realtime.js b/lib/realtime.js index d04ffdc..d85d728 100644 --- a/lib/realtime.js +++ b/lib/realtime.js @@ -49,7 +49,7 @@ function secure (socket, next) { if (handshakeData.sessionID && handshakeData.cookie[config.sessionName] && handshakeData.cookie[config.sessionName] !== handshakeData.sessionID) { - if (config.debug) { logger.info('AUTH success cookie: ' + handshakeData.sessionID) } + logger.debug(`AUTH success cookie: ${handshakeData.sessionID}`) return next() } else { next(new Error('AUTH failed: Cookie is invalid.')) @@ -82,7 +82,7 @@ setInterval(function () { async.each(Object.keys(notes), function (key, callback) { var note = notes[key] if (note.server.isDirty) { - if (config.debug) logger.info('updater found dirty note: ' + key) + logger.debug(`updater found dirty note: ${key}`) note.server.isDirty = false updateNote(note, function (err, _note) { // handle when note already been clean up @@ -182,7 +182,7 @@ setInterval(function () { var socket = realtime.io.sockets.connected[key] if ((!socket && users[key]) || (socket && (!socket.rooms || socket.rooms.length <= 0))) { - if (config.debug) { logger.info('cleaner found redundant user: ' + key) } + logger.debug(`cleaner found redundant user: ${key}`) if (!socket) { socket = { id: key @@ -429,11 +429,11 @@ function finishConnection (socket, noteId, socketId) { if (config.debug) { let noteId = socket.noteId - logger.info('SERVER connected a client to [' + noteId + ']:') - logger.info(JSON.stringify(user)) - // logger.info(notes); + logger.debug(`SERVER connected a client to [${noteId}]:`) + logger.debug(JSON.stringify(user)) + logger.debug(notes) getStatus(function (data) { - logger.info(JSON.stringify(data)) + logger.debug(JSON.stringify(data)) }) } } @@ -541,10 +541,8 @@ function disconnect (socket) { if (isDisconnectBusy) return isDisconnectBusy = true - if (config.debug) { - logger.info('SERVER disconnected a client') - logger.info(JSON.stringify(users[socket.id])) - } + logger.debug('SERVER disconnected a client') + logger.debug(JSON.stringify(users[socket.id])) if (users[socket.id]) { delete users[socket.id] @@ -574,9 +572,9 @@ function disconnect (socket) { delete note.server delete notes[noteId] if (config.debug) { - // logger.info(notes); + logger.debug(notes) getStatus(function (data) { - logger.info(JSON.stringify(data)) + logger.debug(JSON.stringify(data)) }) } }) @@ -595,9 +593,9 @@ function disconnect (socket) { if (disconnectSocketQueue.length > 0) { disconnect(disconnectSocketQueue[0]) } if (config.debug) { - // logger.info(notes); + logger.debug(notes) getStatus(function (data) { - logger.info(JSON.stringify(data)) + logger.debug(JSON.stringify(data)) }) } } @@ -774,7 +772,7 @@ function connection (socket) { var noteId = socket.noteId var user = users[socket.id] if (!noteId || !notes[noteId] || !user) return - if (config.debug) { logger.info('SERVER received [' + noteId + '] user status from [' + socket.id + ']: ' + JSON.stringify(data)) } + logger.debug(`SERVER received [${noteId}] user status from [${socket.id}]: ${JSON.stringify(data)}`) if (data) { user.idle = data.idle user.type = data.type diff --git a/lib/web/auth/ldap/index.js b/lib/web/auth/ldap/index.js index 77790db..9614366 100644 --- a/lib/web/auth/ldap/index.js +++ b/lib/web/auth/ldap/index.js @@ -66,11 +66,11 @@ passport.use(new LDAPStrategy({ } if (needSave) { user.save().then(function () { - if (config.debug) { logger.debug('user login: ' + user.id) } + logger.debug(`user login: ${user.id}`) return done(null, user) }) } else { - if (config.debug) { logger.debug('user login: ' + user.id) } + logger.debug(`user login: ${user.id}`) return done(null, user) } } diff --git a/lib/web/auth/openid/index.js b/lib/web/auth/openid/index.js index 8d271a7..b0a28be 100644 --- a/lib/web/auth/openid/index.js +++ b/lib/web/auth/openid/index.js @@ -33,11 +33,11 @@ passport.use(new OpenIDStrategy({ } if (needSave) { user.save().then(function () { - if (config.debug) { logger.info('user login: ' + user.id) } + logger.debug(`user login: ${user.id}`) return done(null, user) }) } else { - if (config.debug) { logger.info('user login: ' + user.id) } + logger.debug(`user login: ${user.id}`) return done(null, user) } } diff --git a/lib/web/auth/saml/index.js b/lib/web/auth/saml/index.js index 2289b01..40a6f8b 100644 --- a/lib/web/auth/saml/index.js +++ b/lib/web/auth/saml/index.js @@ -62,11 +62,11 @@ passport.use(new SamlStrategy({ } if (needSave) { user.save().then(function () { - if (config.debug) { logger.debug('user login: ' + user.id) } + logger.debug(`user login: ${user.id}`) return done(null, user) }) } else { - if (config.debug) { logger.debug('user login: ' + user.id) } + logger.debug(`user login: ${user.id}`) return done(null, user) } } diff --git a/lib/web/auth/utils.js b/lib/web/auth/utils.js index ff7a123..141a0d6 100644 --- a/lib/web/auth/utils.js +++ b/lib/web/auth/utils.js @@ -1,7 +1,6 @@ 'use strict' const models = require('../../models') -const config = require('../../config') const logger = require('../../logger') exports.setReturnToFromReferer = function setReturnToFromReferer (req) { @@ -38,11 +37,11 @@ exports.passportGeneralCallback = function callback (accessToken, refreshToken, } if (needSave) { user.save().then(function () { - if (config.debug) { logger.info('user login: ' + user.id) } + logger.debug(`user login: ${user.id}`) return done(null, user) }) } else { - if (config.debug) { logger.info('user login: ' + user.id) } + logger.debug(`user login: ${user.id}`) return done(null, user) } } diff --git a/lib/web/imageRouter/azure.js b/lib/web/imageRouter/azure.js index cc98e5f..22ee558 100644 --- a/lib/web/imageRouter/azure.js +++ b/lib/web/imageRouter/azure.js @@ -7,13 +7,13 @@ const logger = require('../../logger') const azure = require('azure-storage') exports.uploadImage = function (imagePath, callback) { - if (!imagePath || typeof imagePath !== 'string') { - callback(new Error('Image path is missing or wrong'), null) + if (!callback || typeof callback !== 'function') { + logger.error('Callback has to be a function') return } - if (!callback || typeof callback !== 'function') { - logger.error('Callback has to be a function') + if (!imagePath || typeof imagePath !== 'string') { + callback(new Error('Image path is missing or wrong'), null) return } diff --git a/lib/web/imageRouter/filesystem.js b/lib/web/imageRouter/filesystem.js index 7c876d6..3ba09e8 100644 --- a/lib/web/imageRouter/filesystem.js +++ b/lib/web/imageRouter/filesystem.js @@ -6,15 +6,15 @@ const config = require('../../config') const logger = require('../../logger') exports.uploadImage = function (imagePath, callback) { - if (!imagePath || typeof imagePath !== 'string') { - callback(new Error('Image path is missing or wrong'), null) - return - } - if (!callback || typeof callback !== 'function') { logger.error('Callback has to be a function') return } + if (!imagePath || typeof imagePath !== 'string') { + callback(new Error('Image path is missing or wrong'), null) + return + } + callback(null, (new URL(path.basename(imagePath), config.serverURL + '/uploads/')).href) } diff --git a/lib/web/imageRouter/imgur.js b/lib/web/imageRouter/imgur.js index eee349f..dcb03a7 100644 --- a/lib/web/imageRouter/imgur.js +++ b/lib/web/imageRouter/imgur.js @@ -5,22 +5,20 @@ const logger = require('../../logger') const imgur = require('imgur') exports.uploadImage = function (imagePath, callback) { - if (!imagePath || typeof imagePath !== 'string') { - callback(new Error('Image path is missing or wrong'), null) + if (!callback || typeof callback !== 'function') { + logger.error('Callback has to be a function') return } - if (!callback || typeof callback !== 'function') { - logger.error('Callback has to be a function') + if (!imagePath || typeof imagePath !== 'string') { + callback(new Error('Image path is missing or wrong'), null) return } imgur.setClientId(config.imgur.clientID) imgur.uploadFile(imagePath) .then(function (json) { - if (config.debug) { - logger.info('SERVER uploadimage success: ' + JSON.stringify(json)) - } + logger.debug(`SERVER uploadimage success: ${JSON.stringify(json)}`) callback(null, json.data.link.replace(/^http:\/\//i, 'https://')) }).catch(function (err) { callback(new Error(err), null) diff --git a/lib/web/imageRouter/index.js b/lib/web/imageRouter/index.js index f3c2dec..0b59218 100644 --- a/lib/web/imageRouter/index.js +++ b/lib/web/imageRouter/index.js @@ -21,18 +21,19 @@ imageRouter.post('/uploadimage', function (req, res) { form.parse(req, function (err, fields, files) { if (err || !files.image || !files.image.path) { + logger.error(`formidable error: ${err}`) response.errorForbidden(res) } else { - if (config.debug) { - logger.info('SERVER received uploadimage: ' + JSON.stringify(files.image)) - } + logger.debug(`SERVER received uploadimage: ${JSON.stringify(files.image)}`) const uploadProvider = require('./' + config.imageUploadType) + logger.debug(`imageRouter: Uploading ${files.image.path} using ${config.imageUploadType}`) uploadProvider.uploadImage(files.image.path, function (err, url) { if (err !== null) { logger.error(err) return res.status(500).end('upload image error') } + logger.debug(`SERVER sending ${url} to client`) res.send({ link: url }) diff --git a/lib/web/imageRouter/lutim.js b/lib/web/imageRouter/lutim.js index 78b856c..61930ad 100644 --- a/lib/web/imageRouter/lutim.js +++ b/lib/web/imageRouter/lutim.js @@ -5,25 +5,24 @@ const logger = require('../../logger') const lutim = require('lutim') exports.uploadImage = function (imagePath, callback) { - if (!imagePath || typeof imagePath !== 'string') { - callback(new Error('Image path is missing or wrong'), null) - return - } - if (!callback || typeof callback !== 'function') { logger.error('Callback has to be a function') return } + if (!imagePath || typeof imagePath !== 'string') { + callback(new Error('Image path is missing or wrong'), null) + return + } + if (config.lutim && config.lutim.url) { lutim.setAPIUrl(config.lutim.url) + logger.debug(`Set lutim URL to ${lutim.getApiUrl()}`) } lutim.uploadImage(imagePath) .then(function (json) { - if (config.debug) { - logger.info('SERVER uploadimage success: ' + JSON.stringify(json)) - } + logger.debug(`SERVER uploadimage success: ${JSON.stringify(json)}`) callback(null, lutim.getAPIUrl() + json.msg.short) }).catch(function (err) { callback(new Error(err), null) diff --git a/lib/web/imageRouter/s3.js b/lib/web/imageRouter/s3.js index b0eca7b..4effea0 100644 --- a/lib/web/imageRouter/s3.js +++ b/lib/web/imageRouter/s3.js @@ -35,6 +35,7 @@ exports.uploadImage = function (imagePath, callback) { const mimeType = getImageMimeType(imagePath) if (mimeType) { params.ContentType = mimeType } + logger.debug(`S3 object parameters: ${JSON.stringify(params)}`) s3.putObject(params, function (err, data) { if (err) { callback(new Error(err), null) diff --git a/lib/web/statusRouter.js b/lib/web/statusRouter.js index 6f797f8..da69e62 100644 --- a/lib/web/statusRouter.js +++ b/lib/web/statusRouter.js @@ -68,9 +68,7 @@ statusRouter.post('/temp', urlencodedParser, function (req, res) { if (!data) { response.errorForbidden(res) } else { - if (config.debug) { - logger.info('SERVER received temp from [' + host + ']: ' + req.body.data) - } + logger.debug(`SERVER received temp from [${host}]: ${req.body.data}`) models.Temp.create({ data: data }).then(function (temp) { diff --git a/lib/workers/dmpWorker.js b/lib/workers/dmpWorker.js index 60db0a1..ca68b4a 100644 --- a/lib/workers/dmpWorker.js +++ b/lib/workers/dmpWorker.js @@ -4,7 +4,6 @@ var DiffMatchPatch = require('diff-match-patch') var dmp = new DiffMatchPatch() // core -var config = require('../config') var logger = require('../logger') process.on('message', function (data) { @@ -61,10 +60,8 @@ function createPatch (lastDoc, currDoc) { var patch = dmp.patch_make(lastDoc, diff) patch = dmp.patch_toText(patch) var msEnd = (new Date()).getTime() - if (config.debug) { - logger.info(patch) - logger.info((msEnd - msStart) + 'ms') - } + logger.debug(patch) + logger.debug((msEnd - msStart) + 'ms') return patch } @@ -123,9 +120,7 @@ function getRevision (revisions, count) { authorship: authorship } var msEnd = (new Date()).getTime() - if (config.debug) { - logger.info((msEnd - msStart) + 'ms') - } + logger.debug((msEnd - msStart) + 'ms') return data } From da4665c7590616dd3d17baf4488006dac98eeee4 Mon Sep 17 00:00:00 2001 From: Sheogorath Date: Sat, 8 Jun 2019 23:33:34 +0200 Subject: [PATCH 02/16] Respect DNT header Do Not Track (DNT) is an old web standard in order to notify pages that the user doesn't want to be tracked. Even while a lot of pages either ignore this header or even worse, use it for tracking purposes, the orignal intention of this header is good and should be adopted. This patch implements a respect of the DNT header by no longer including the optional Google Analytics and disqus integrations when sending a DNT header. This should reduce outside resource usage and help to stay more private. This should later-on extended towards other document content (i.e. iframe based content). The reason to not change the CDN handling is that CDNs will be deprecated with next release and removed in long term. Signed-off-by: Sheogorath --- lib/response.js | 6 ++++-- public/views/pretty.ejs | 2 +- public/views/shared/ga.ejs | 2 +- public/views/slide.ejs | 2 +- 4 files changed, 7 insertions(+), 5 deletions(-) diff --git a/lib/response.js b/lib/response.js index dd33e42..1313f9f 100644 --- a/lib/response.js +++ b/lib/response.js @@ -226,7 +226,8 @@ function showPublishNote (req, res, next) { robots: meta.robots || false, // default allow robots GA: meta.GA, disqus: meta.disqus, - cspNonce: res.locals.nonce + cspNonce: res.locals.nonce, + dnt: req.headers.dnt } return renderPublish(data, res) }).catch(function (err) { @@ -608,7 +609,8 @@ function showPublishSlide (req, res, next) { robots: meta.robots || false, // default allow robots GA: meta.GA, disqus: meta.disqus, - cspNonce: res.locals.nonce + cspNonce: res.locals.nonce, + dnt: req.headers.dnt } return renderPublishSlide(data, res) }).catch(function (err) { diff --git a/public/views/pretty.ejs b/public/views/pretty.ejs index 9604d42..1970ab2 100644 --- a/public/views/pretty.ejs +++ b/public/views/pretty.ejs @@ -63,7 +63,7 @@ - <% if(typeof disqus !== 'undefined' && disqus) { %> + <% if(typeof disqus !== 'undefined' && disqus && !dnt) { %>
<%- include shared/disqus %>
diff --git a/public/views/shared/ga.ejs b/public/views/shared/ga.ejs index 27abb74..f6b807f 100644 --- a/public/views/shared/ga.ejs +++ b/public/views/shared/ga.ejs @@ -1,4 +1,4 @@ -<% if(typeof GA !== 'undefined' && GA) { %> +<% if(typeof GA !== 'undefined' && GA && !dnt) { %>