From 095338fc484ea8855de5ca7e2192c370b2e3852f Mon Sep 17 00:00:00 2001 From: Michael Swift Date: Thu, 11 Jun 2026 11:20:52 -0400 Subject: [PATCH 1/2] Fix connector save-session garbage collection and error response The SessionManager.gc() loop never freed sessions due to two bugs: iterating the _sessions Map with `for (let session of ...)` yields [id, session] entries, so session.created and session.id were always undefined; and the body referenced the non-existent `this._session` (singular) rather than `this._sessions`. SaveSession.remove() had a related bug, using `delete map[key]` on a Map, which is a no-op. Sessions therefore accumulated for the lifetime of the process. Also fix a misplaced parenthesis in the server error handler that passed the content type and body to _requestFinished() instead of _generateResponse(), so the 500 response never included its body. --- chrome/content/zotero/xpcom/server/saveSession.js | 6 +++--- chrome/content/zotero/xpcom/server/server.js | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/chrome/content/zotero/xpcom/server/saveSession.js b/chrome/content/zotero/xpcom/server/saveSession.js index e40bad3931..de1558affe 100644 --- a/chrome/content/zotero/xpcom/server/saveSession.js +++ b/chrome/content/zotero/xpcom/server/saveSession.js @@ -49,9 +49,9 @@ Zotero.Server.Connector.SessionManager = { var ttl = this._sessions.size >= 10 ? 60 : 600; var deleteBefore = new Date() - ttl * 1000; - for (let session of this._sessions) { + for (let [id, session] of this._sessions) { if (session.created < deleteBefore) { - this._session.delete(session.id); + this._sessions.delete(id); } } } @@ -147,7 +147,7 @@ Zotero.Server.Connector.SaveSession = class { } remove() { - delete Zotero.Server.Connector.SessionManager._sessions[this.id]; + Zotero.Server.Connector.SessionManager._sessions.delete(this.id); } /** diff --git a/chrome/content/zotero/xpcom/server/server.js b/chrome/content/zotero/xpcom/server/server.js index e7b0d5c5c5..6429edc23f 100755 --- a/chrome/content/zotero/xpcom/server/server.js +++ b/chrome/content/zotero/xpcom/server/server.js @@ -502,7 +502,7 @@ Zotero.Server.RequestHandler.prototype._processEndpoint = async function (method } } catch(e) { Zotero.debug(e); - this._requestFinished(this._generateResponse(500), "text/plain", "An error occurred\n"); + this._requestFinished(this._generateResponse(500, "text/plain", "An error occurred\n")); throw e; } }; From 7b34f22ddf8ca3e5ee4ed8f639675b369b8eb11d Mon Sep 17 00:00:00 2001 From: Michael Swift Date: Thu, 11 Jun 2026 12:59:15 -0400 Subject: [PATCH 2/2] Add regression tests for connector save-session gc and remove() Cover the three previously-broken behaviors: gc() expiring sessions past the TTL, gc() retaining fresh sessions, and SaveSession#remove() deleting a session from the manager. There was no existing coverage of gc(), which is how the leak went unnoticed. --- test/tests/server_connectorTest.js | 35 ++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/test/tests/server_connectorTest.js b/test/tests/server_connectorTest.js index e5223f7051..92dd662d63 100644 --- a/test/tests/server_connectorTest.js +++ b/test/tests/server_connectorTest.js @@ -93,6 +93,41 @@ describe("Connector Server", function () { }); }); + describe("SaveSession.SessionManager", function () { + var SessionManager = Zotero.Server.Connector.SessionManager; + + it("should expire sessions older than the TTL when gc() runs", function () { + var id = "gcOld_" + Zotero.Utilities.randomString(); + var session = SessionManager.create(id, 'saveItems', {}); + // Backdate creation beyond the 10-minute TTL + session.created = new Date(Date.now() - 11 * 60 * 1000); + + SessionManager.gc(); + + assert.isUndefined(SessionManager.get(id), "stale session should be removed by gc()"); + }); + + it("should keep sessions newer than the TTL when gc() runs", function () { + var id = "gcNew_" + Zotero.Utilities.randomString(); + var session = SessionManager.create(id, 'saveItems', {}); + + SessionManager.gc(); + + assert.strictEqual(SessionManager.get(id), session, "fresh session should survive gc()"); + session.remove(); + }); + + it("should remove a session via SaveSession#remove()", function () { + var id = "remove_" + Zotero.Utilities.randomString(); + var session = SessionManager.create(id, 'saveItems', {}); + assert.strictEqual(SessionManager.get(id), session); + + session.remove(); + + assert.isUndefined(SessionManager.get(id), "remove() should delete the session from the manager"); + }); + }); + describe('/connector/getTranslatorCode', function () { it('should respond with translator code', async function () { var code = 'function detectWeb() {}\nfunction doImport() {}';