From d17bc9a8db6524be40e8aec958ace79ec77e7211 Mon Sep 17 00:00:00 2001 From: Max Date: Tue, 26 Sep 2023 10:42:23 +0200 Subject: [PATCH 1/2] fix(api): prevent sending requests with closed session. Signed-off-by: Max --- cypress/e2e/api/SessionApi.spec.js | 13 ++++++++++++ src/services/SessionApi.js | 33 +++++++++++++++++++++++------- 2 files changed, 39 insertions(+), 7 deletions(-) diff --git a/cypress/e2e/api/SessionApi.spec.js b/cypress/e2e/api/SessionApi.spec.js index d98c81727ec..36d8f94e1e5 100644 --- a/cypress/e2e/api/SessionApi.spec.js +++ b/cypress/e2e/api/SessionApi.spec.js @@ -301,6 +301,19 @@ describe('The session Api', function() { }) }) + it('signals closing connection', function() { + cy.then(() => { + return new Promise((resolve, reject) => { + connection.close() + connection.push({ steps: [messages.update], version, awareness: '' }) + .then( + () => reject(new Error('Push should have thrown ConnectionClosed()')), + resolve, + ) + }) + }) + }) + it('sends initial content if other session is alive but did not push any steps', function() { let joining cy.createTextSession(undefined, { filePath: '', shareToken }) diff --git a/src/services/SessionApi.js b/src/services/SessionApi.js index 2260dd4a328..7380401c962 100644 --- a/src/services/SessionApi.js +++ b/src/services/SessionApi.js @@ -22,6 +22,14 @@ import axios from '@nextcloud/axios' import { generateUrl } from '@nextcloud/router' +export class ConnectionClosedError extends Error { + + constructor(message = 'Close has already been called on the connection', ...rest) { + super(message, ...rest) + } + +} + class SessionApi { #options @@ -50,6 +58,7 @@ class SessionApi { export class Connection { #content + #closed #documentState #document #session @@ -66,6 +75,7 @@ export class Connection { this.#content = content this.#documentState = documentState this.#options = options + this.closed = false } get session() { @@ -99,7 +109,7 @@ export class Connection { } sync({ version }) { - return axios.post(this.#url('session/sync'), { + return this.#post(this.#url('session/sync'), { ...this.#defaultParams, filePath: this.#options.filePath, version, @@ -107,7 +117,7 @@ export class Connection { } save({ version, autosaveContent, documentState, force, manualSave }) { - return axios.post(this.#url('session/save'), { + return this.#post(this.#url('session/save'), { ...this.#defaultParams, filePath: this.#options.filePath, version, @@ -119,7 +129,7 @@ export class Connection { } push({ steps, version, awareness }) { - return axios.post(this.#url('session/push'), { + return this.#post(this.#url('session/push'), { ...this.#defaultParams, filePath: this.#options.filePath, steps, @@ -130,7 +140,7 @@ export class Connection { // TODO: maybe return a new connection here so connections have immutable state update(guestName) { - return axios.post(this.#url('session'), { + return this.#post(this.#url('session'), { ...this.#defaultParams, guestName, }).then(({ data }) => { @@ -146,7 +156,7 @@ export class Connection { + '&sessionId=' + encodeURIComponent(this.#session.id) + '&sessionToken=' + encodeURIComponent(this.#session.token) + '&shareToken=' + encodeURIComponent(this.#options.shareToken || '') - return axios.post(url, formData, { + return this.#post(url, formData, { headers: { 'Content-Type': 'multipart/form-data', }, @@ -154,7 +164,7 @@ export class Connection { } insertAttachmentFile(filePath) { - return axios.post(_endpointUrl('attachment/filepath'), { + return this.#post(_endpointUrl('attachment/filepath'), { documentId: this.#document.id, sessionId: this.#session.id, sessionToken: this.#session.token, @@ -163,7 +173,16 @@ export class Connection { } close() { - return axios.post(this.#url('session/close'), this.#defaultParams) + const promise = this.#post(this.#url('session/close'), this.#defaultParams) + this.closed = true + return promise + } + + #post(...args) { + if (this.closed) { + return Promise.reject(new ConnectionClosedError()) + } + return axios.post(...args) } #url(endpoint) { From d89971462e6233fa0d24be39bfa903c79f6cb6d2 Mon Sep 17 00:00:00 2001 From: Max Date: Tue, 26 Sep 2023 10:44:22 +0200 Subject: [PATCH 2/2] fix(api): send remaining steps before closing connection y.js will send a last awareness update when closing the websocket. Try to get this out before closing the sync service connection. Signed-off-by: Max --- src/services/SyncService.js | 8 ++++---- src/services/WebSocketPolyfill.js | 22 ++++++++++++++++++++-- 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/src/services/SyncService.js b/src/services/SyncService.js index bfa4c8f3c34..902d9c60cb5 100644 --- a/src/services/SyncService.js +++ b/src/services/SyncService.js @@ -157,16 +157,16 @@ class SyncService { return new Promise((resolve, reject) => { this.#sendIntervalId = setInterval(() => { if (this.connection && !this.sending) { - clearInterval(this.#sendIntervalId) - this.#sendIntervalId = null - this._sendSteps(getSendable).then(resolve).catch(reject) + this.sendStepsNow(getSendable).then(resolve).catch(reject) } }, 200) }) } - _sendSteps(getSendable) { + sendStepsNow(getSendable) { this.sending = true + clearInterval(this.#sendIntervalId) + this.#sendIntervalId = null const data = getSendable() if (data.steps.length > 0) { this.emit('stateChange', { dirty: true }) diff --git a/src/services/WebSocketPolyfill.js b/src/services/WebSocketPolyfill.js index 082a07d836e..8bb8a0c5fd5 100644 --- a/src/services/WebSocketPolyfill.js +++ b/src/services/WebSocketPolyfill.js @@ -85,7 +85,7 @@ export default function initWebSocketPolyfill(syncService, fileId, initialSessio this.#queue.push(...data) let outbox = [] - syncService.sendSteps(() => { + return syncService.sendSteps(() => { outbox = [...this.#queue] const data = { steps: this.#steps, @@ -112,7 +112,8 @@ export default function initWebSocketPolyfill(syncService, fileId, initialSessio .findLast(s => s > 'AQ') || '' } - close() { + async close() { + await this.#sendRemainingSteps() Object.entries(this.#handlers) .forEach(([key, value]) => syncService.off(key, value)) this.#handlers = [] @@ -122,5 +123,22 @@ export default function initWebSocketPolyfill(syncService, fileId, initialSessio logger.debug('Websocket closed') } + #sendRemainingSteps() { + if (this.#queue.length) { + return syncService.sendStepsNow(() => { + const data = { + steps: this.#steps, + awareness: this.#awareness, + version: this.#version, + } + this.#queue = [] + logger.debug('sending final steps ', data) + return data + })?.catch(err => { + logger.error(err) + }) + } + } + } }