Merge pull request #4824 from nextcloud/fix/403-on-close

Fix 403 on close

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 })

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,15 +109,15 @@ 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,
 		})
 	}
 
 	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,15 +156,15 @@ 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',
 			},
 		})
 	}
 
 	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) {

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 })

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)
+				})
+			}
+		}
+
 	}
 }