fix(realtime): compatible extractNoteIdFromSocket failed

fixed url parsed method and add testcase for extractNoteIdFromSocket

Signed-off-by: BoHong Li <a60814billy@gmail.com>

lib/realtime.js

@@ -10,6 +10,8 @@ var Chance = require('chance')
 var chance = new Chance()
 var moment = require('moment')
 
+const get = require('lodash/get')
+
 // core
 var config = require('./config')
 var logger = require('./logger')
@@ -271,25 +273,51 @@ function isReady () {
     disconnectSocketQueue.length === 0 && !isDisconnectBusy
 }
 
+function parseUrl (data) {
+  try {
+    if (url.URL) {
+      return new url.URL(data)
+    } else {
+      // fallback legacy api
+      // eslint-disable-next-line
+      return url.parse(data)
+    }
+  } catch (e) {
+  }
+  return null
+}
+
 function extractNoteIdFromSocket (socket) {
+  function extractNoteIdFromReferer (referer) {
+    if (referer) {
+      const hostUrl = parseUrl(referer)
+      if (!hostUrl) {
+        return false
+      }
+      if (config.urlPath) {
+        return hostUrl.pathname.slice(config.urlPath.length + 1, hostUrl.pathname.length).split('/')[1]
+      }
+      return hostUrl.pathname.split('/')[1]
+    }
+    return false
+  }
+
   if (!socket || !socket.handshake) {
     return false
   }
-  if (socket.handshake.query && socket.handshake.query.noteId) {
-    return socket.handshake.query.noteId
-  } else if (socket.handshake.headers) {
+
+  if (get(socket, 'handshake.query.noteId')) {
+    return decodeURIComponent(socket.handshake.query.noteId)
+  }
+
+  const referer = get(socket, 'handshake.headers.referer')
+  if (referer) {
     // this part is only for backward compatibility only; current code
     // should be using noteId query parameter instead.
-    var referer = socket.handshake.headers.referer
-    if (!referer) {
-      return false
+    return extractNoteIdFromReferer(referer)
   }
-    var hostUrl = url.URL.parse(referer)
-    var noteId = config.urlPath ? hostUrl.pathname.slice(config.urlPath.length + 1, hostUrl.pathname.length).split('/')[1] : hostUrl.pathname.split('/')[1]
-    return noteId
-  } else {
+
   return false
-  }
 }
 
 function parseNoteIdFromSocket (socket, callback) {
@@ -933,4 +961,5 @@ function connection (socket) {
   })
 }
 
-module.exports = realtime
+exports = module.exports = realtime
+exports.extractNoteIdFromSocket = extractNoteIdFromSocket

test/realtime.test.js

@@ -0,0 +1,104 @@
+'use strict'
+
+/* eslint-env node, mocha */
+
+const io = require('socket.io-client')
+const http = require('http')
+
+const mock = require('mock-require')
+const assert = require('assert')
+
+function makeMockSocket (headers, query) {
+  return {
+    handshake: {
+      headers: Object.assign({}, headers),
+      query: Object.assign({}, query)
+    }
+  }
+}
+
+describe('realtime', function () {
+  describe('extractNoteIdFromSocket', function () {
+    beforeEach(() => {
+      mock('../lib/logger', {})
+      mock('../lib/history', {})
+      mock('../lib/models', {})
+    })
+
+    afterEach(() => {
+      delete require.cache[require.resolve('../lib/realtime')]
+      mock.stopAll()
+    })
+
+    describe('urlPath not set', function () {
+      beforeEach(() => {
+        mock('../lib/config', {})
+        realtime = require('../lib/realtime')
+      })
+
+      let realtime
+
+      it('return false if socket or socket.handshake not exists', function () {
+        let noteId = realtime.extractNoteIdFromSocket()
+        assert.strictEqual(false, noteId)
+
+        noteId = realtime.extractNoteIdFromSocket({})
+        assert.strictEqual(false, noteId)
+      })
+
+      it('return false if query not set and referer not set', function () {
+        let noteId = realtime.extractNoteIdFromSocket(makeMockSocket({
+          otherHeader: 1
+        }, {
+          otherQuery: 1
+        }))
+        assert.strictEqual(false, noteId)
+      })
+
+      it('return noteId from query', function () {
+        // Arrange
+        const incomingNoteId = 'myNoteId'
+        const incomingSocket = makeMockSocket(undefined, { noteId: incomingNoteId })
+
+        // Act
+        const noteId = realtime.extractNoteIdFromSocket(incomingSocket)
+        // Assert
+        assert.strictEqual(noteId, incomingNoteId)
+      })
+
+      it('return noteId from old method (referer)', function () {
+        // Arrange
+        const incomingNoteId = 'myNoteId'
+        const incomingSocket = makeMockSocket({
+          referer: `https://localhost:3000/${incomingNoteId}`
+        })
+
+        // Act
+        const noteId = realtime.extractNoteIdFromSocket(incomingSocket)
+        // Assert
+        assert.strictEqual(noteId, incomingNoteId)
+      })
+    })
+
+    describe('urlPath is set', function () {
+      let realtime
+      it('return noteId from old method (referer) and urlPath set', function () {
+        // Arrange
+        const urlPath = 'hello'
+        mock('../lib/config', {
+          urlPath: urlPath
+        })
+        realtime = require('../lib/realtime')
+        const incomingNoteId = 'myNoteId'
+        const incomingSocket = makeMockSocket({
+          referer: `https://localhost:3000/${urlPath}/${incomingNoteId}`
+        })
+
+        // Act
+        const noteId = realtime.extractNoteIdFromSocket(incomingSocket)
+        // Assert
+        assert.strictEqual(noteId, incomingNoteId)
+      })
+    })
+  })
+})