From 4dfecce966f95e5176c9bb1d7ad2135e88afa513 Mon Sep 17 00:00:00 2001
From: Nick Krecklow <hello@github.nklow.com>
Date: Fri, 8 May 2020 04:47:10 -0500
Subject: [PATCH] serve favicons over hashed paths for improved caching

---
 assets/js/servers.js |  8 +++++++-
 lib/app.js           |  2 +-
 lib/server.js        | 45 +++++++++++++++++++++++++++++++++++---------
 lib/servers.js       | 36 ++++++++++++++++++++++++++++-------
 4 files changed, 73 insertions(+), 18 deletions(-)

diff --git a/assets/js/servers.js b/assets/js/servers.js
index 1c8cc08..3b4bd62 100644
--- a/assets/js/servers.js
+++ b/assets/js/servers.js
@@ -210,7 +210,13 @@ export class ServerRegistration {
 
     // An updated favicon has been sent, update the src
     if (ping.favicon) {
-      document.getElementById('favicon_' + this.serverId).setAttribute('src', ping.favicon)
+      const faviconElement = document.getElementById('favicon_' + this.serverId)
+
+      // Since favicons may be URLs, only update the attribute when it has changed
+      // Otherwise the browser may send multiple requests to the same URL
+      if (faviconElement.getAttribute('src') !== ping.favicon) {
+        faviconElement.setAttribute('src', ping.favicon)
+      }
     }
   }
 
diff --git a/lib/app.js b/lib/app.js
index 7a330fc..22127ad 100644
--- a/lib/app.js
+++ b/lib/app.js
@@ -14,7 +14,7 @@ class App {
   constructor () {
     this.mojangUpdater = new MojangUpdater(this)
     this.pingController = new PingController(this)
-    this.server = new Server(this.handleClientConnection)
+    this.server = new Server(this)
     this.timeTracker = new TimeTracker(this)
   }
 
diff --git a/lib/server.js b/lib/server.js
index 9a6b371..49cf340 100644
--- a/lib/server.js
+++ b/lib/server.js
@@ -11,9 +11,11 @@ function getRemoteAddr (req) {
 }
 
 class Server {
-  constructor (clientSocketHandler) {
+  constructor (app) {
+    this._app = app
+
     this.createHttpServer()
-    this.createWebSocketServer(clientSocketHandler)
+    this.createWebSocketServer()
   }
 
   createHttpServer () {
@@ -23,15 +25,40 @@ class Server {
     this._http = http.createServer((req, res) => {
       logger.log('info', '%s requested: %s', getRemoteAddr(req), req.url)
 
-      // Attempt to handle req using distServeStatic, otherwise fail over to faviconServeStatic
-      // If faviconServeStatic fails, pass to finalHttpHandler to terminate
-      distServeStatic(req, res, () => {
-        faviconsServeStatic(req, res, finalHttpHandler(req, res))
-      })
+      if (req.url.startsWith('/hashedfavicon?')) {
+        this.handleFaviconRequest(req, res)
+      } else {
+        // Attempt to handle req using distServeStatic, otherwise fail over to faviconServeStatic
+        // If faviconServeStatic fails, pass to finalHttpHandler to terminate
+        distServeStatic(req, res, () => {
+          faviconsServeStatic(req, res, finalHttpHandler(req, res))
+        })
+      }
     })
   }
 
-  createWebSocketServer (proxyClientSocketHandler) {
+  handleFaviconRequest = (req, res) => {
+    const hash = req.url.split('?')[1]
+
+    for (const serverRegistration of this._app.serverRegistrations) {
+      if (serverRegistration.faviconHash && serverRegistration.faviconHash === hash) {
+        const buf = Buffer.from(serverRegistration.lastFavicon.split(',')[1], 'base64')
+
+        res.writeHead(200, {
+          'Content-Type': 'image/png',
+          'Content-Length': buf.length
+        }).end(buf)
+
+        return
+      }
+    }
+
+    // Terminate request, no match
+    res.writeHead(404)
+    res.end()
+  }
+
+  createWebSocketServer () {
     this._wss = new WebSocket.Server({
       server: this._http
     })
@@ -45,7 +72,7 @@ class Server {
       })
 
       // Pass client off to proxy handler
-      proxyClientSocketHandler(client)
+      this._app.handleClientConnection(client)
     })
   }
 
diff --git a/lib/servers.js b/lib/servers.js
index 5db95dd..61777e2 100644
--- a/lib/servers.js
+++ b/lib/servers.js
@@ -1,3 +1,5 @@
+const crypto = require('crypto')
+
 const TimeTracker = require('./time')
 
 const config = require('../config')
@@ -61,11 +63,8 @@ class ServerRegistration {
         update.recordData = this.recordData
       }
 
-      // Compare against this.data.favicon to support favicon overrides
-      const newFavicon = resp.favicon || this.data.favicon
-      if (this.updateFavicon(newFavicon)) {
-        // Append an updated favicon
-        update.favicon = newFavicon
+      if (this.updateFavicon(resp.favicon)) {
+        update.favicon = this.getFaviconUrl()
       }
 
       // Append a result object
@@ -101,7 +100,7 @@ class ServerRegistration {
       const payload = {
         versions: this.versions,
         recordData: this.recordData,
-        favicon: this.lastFavicon
+        favicon: this.getFaviconUrl()
       }
 
       // Only append graphPeakData if defined
@@ -134,7 +133,8 @@ class ServerRegistration {
         message: 'Pinging...'
       },
       recordData: this.recordData,
-      graphPeakData: this.getGraphPeak()
+      graphPeakData: this.getGraphPeak(),
+      favicon: this.data.favicon
     }
   }
 
@@ -216,13 +216,35 @@ class ServerRegistration {
   }
 
   updateFavicon (favicon) {
+    // If data.favicon is defined, then a favicon override is present
+    // Disregard the incoming favicon, regardless if it is different
+    if (this.data.favicon) {
+      return false
+    }
+
     if (favicon && favicon !== this.lastFavicon) {
       this.lastFavicon = favicon
+
+      // Generate an updated hash
+      // This is used by #getFaviconUrl
+      if (!this._faviconHasher) {
+        this._faviconHasher = crypto.createHash('md5')
+      }
+      this.faviconHash = this._faviconHasher.update(favicon).digest('hex').toString()
+
       return true
     }
     return false
   }
 
+  getFaviconUrl () {
+    if (this.faviconHash) {
+      return '/hashedfavicon?' + this.faviconHash
+    } else if (this.data.favicon) {
+      return this.data.favicon
+    }
+  }
+
   updateProtocolVersionCompat (incomingId, outgoingId, protocolIndex) {
     // If the result version matches the attempted version, the version is supported
     const isSuccess = incomingId === outgoingId