[real-time] adopt strict zod types and extend test coverage (#36031)

* [editor-core] extend coverage of zod schemas

* [real-time] adopt strict zod types and extend test coverage

* [project-history] make type check happy

* [web] make type check happy

GitOrigin-RevId: e1a65b9b3560b31ad2af817067a4b549865a5bb1
This commit is contained in:
Jakob Ackermann
2026-09-17 11:42:50 +00:00
committed by Copybot
parent 81df324bcc
commit f6bb5ce1bd
20 changed files with 1228 additions and 109 deletions
+10 -9
View File
@@ -11,7 +11,7 @@ const StringFileData = require('./file_data/string_file_data')
/**
* @import Blob from "./blob"
* @import { BlobStore, ReadonlyBlobStore, RawFileData, RawFile } from "./types"
* @import { BlobStore, ReadonlyBlobStore, RawFileData, RawFile, FileMetadata } from "./types"
* @import { StringFileRawData, CommentRawData } from "./types"
* @import CommentList from "./file_data/comment_list"
* @import TextOperation from "./operation/text_operation"
@@ -55,12 +55,13 @@ class File {
/**
* @param {FileData} data
* @param {Object} [metadata]
* @param {FileMetadata} [metadata]
*/
constructor(data, metadata) {
assert.instance(data, FileData, 'File: bad data')
this.data = data
/** @type {FileMetadata} */
this.metadata = {}
this.setMetadata(metadata || {})
}
@@ -77,7 +78,7 @@ class File {
/**
* @param {string} hash
* @param {string} [rangesHash]
* @param {Object} [metadata]
* @param {FileMetadata} [metadata]
* @return {File}
*/
static fromHash(hash, rangesHash, metadata) {
@@ -86,7 +87,7 @@ class File {
/**
* @param {string} string
* @param {Object} [metadata]
* @param {FileMetadata} [metadata]
* @return {File}
*/
static fromString(string, metadata) {
@@ -96,7 +97,7 @@ class File {
/**
* @param {number} byteLength
* @param {number} [stringLength]
* @param {Object} [metadata]
* @param {FileMetadata} [metadata]
* @return {File}
*/
static createHollow(byteLength, stringLength, metadata) {
@@ -106,7 +107,7 @@ class File {
/**
* @param {Blob} blob
* @param {Blob} [rangesBlob]
* @param {Object} [metadata]
* @param {FileMetadata} [metadata]
* @return {File}
*/
static createLazyFromBlobs(blob, rangesBlob, metadata) {
@@ -197,7 +198,7 @@ class File {
/**
* Return the metadata object for this file.
*
* @return {Object}
* @return {FileMetadata}
*/
getMetadata() {
return this.metadata
@@ -206,7 +207,7 @@ class File {
/**
* Set the metadata object for this file.
*
* @param {Object} metadata
* @param {FileMetadata} metadata
*/
setMetadata(metadata) {
assert.object(metadata, 'File: bad metadata')
@@ -281,7 +282,7 @@ class File {
}
/**
* @param {Object} metadata
* @param {FileMetadata} metadata
* @param {RawFile} raw
*/
function storeRawMetadata(metadata, raw) {
@@ -72,6 +72,223 @@ const rawEditOperation = z.union([
rawEditNoOperation,
])
const rawComment = z.strictObject({
id: zz.objectId(),
ranges: z.array(rawRange),
resolved: z.boolean().optional(),
})
const rawTrackedChange = z.strictObject({
range: rawRange,
tracking: rawTrackingProps,
})
const rawStringFileData = z.strictObject({
content: z.string(),
comments: z.array(rawComment).optional(),
trackedChanges: z.array(rawTrackedChange).optional(),
})
// linkedFileData as written by the web linked-file agents (all-scalar; see
// the per-provider _sanitizeData implementations in web). importedAt is
// always written on create/refresh but may be absent on very old documents.
const rawLinkedFileData = z.discriminatedUnion('provider', [
z.strictObject({
provider: z.literal('url'),
url: z.string(),
importedAt: z.iso.datetime().optional(),
}),
z.strictObject({
provider: z.literal('project_file'),
source_project_id: zz.objectId().optional(),
v1_source_doc_id: z.string().optional(),
source_entity_path: z.string(),
importedAt: z.iso.datetime().optional(),
}),
z.strictObject({
provider: z.literal('project_output_file'),
source_project_id: zz.objectId().optional(),
v1_source_doc_id: z.string().optional(),
source_output_file_path: z.string(),
build_id: zz.buildId().optional(),
clsiServerId: z.string().optional(),
importedAt: z.iso.datetime().optional(),
}),
z.strictObject({
provider: z.literal('mendeley'),
group_id: zz.routeSegment().optional(),
importer_id: z.string().optional(),
v1_importer_id: z.number().optional(),
importedAt: z.iso.datetime().optional(),
}),
z.strictObject({
provider: z.literal('zotero'),
// absent on files imported before the `format` field existed; agents
// default this to 'bibtex' (ZoteroAgent `_getFormat`)
format: z.enum(['bibtex', 'biblatex']).optional(),
group_id: zz.routeSegment().optional(),
importer_id: z.string().optional(),
v1_importer_id: z.number().optional(),
importedAt: z.iso.datetime().optional(),
}),
z.strictObject({
provider: z.literal('papers'),
group_id: zz.routeSegment().optional(),
importer_id: z.string().optional(),
v1_importer_id: z.number().optional(),
importedAt: z.iso.datetime().optional(),
}),
])
const rawBlobHash = z.string().regex(/^[0-9a-f]{40}$/, {
message: 'invalid blob hash',
})
// file metadata is the linked-file metadata written by web, the main-file
// marker written by the v1 history import, regular file with importedAt,
// or an empty object (metadata cleared)
const rawFileMetadata = z.union([
rawLinkedFileData,
z.strictObject({
main: z.boolean(),
importedAt: z.iso.datetime().optional(),
}),
z.strictObject({
importedAt: z.iso.datetime(),
}),
z.strictObject({}),
])
const rawHashFileData = z.strictObject({
hash: rawBlobHash,
rangesHash: rawBlobHash.optional(),
})
const rawBinaryFileData = z.strictObject({
hash: rawBlobHash,
byteLength: z.number().int().min(0),
})
const rawLazyStringFileData = z.strictObject({
hash: rawBlobHash,
stringLength: z.number().int().min(0),
rangesHash: rawBlobHash.optional(),
operations: z.array(rawEditOperation).optional(),
})
const rawHollowBinaryFileData = z.strictObject({
byteLength: z.number().int().min(0),
})
const rawHollowStringFileData = z.strictObject({
stringLength: z.number().int().min(0),
})
const rawFileData = z.union([
rawBinaryFileData,
rawHashFileData,
rawHollowBinaryFileData,
rawHollowStringFileData,
rawLazyStringFileData,
rawStringFileData,
])
// the strict variants disambiguate the union by their key sets
const rawFile = z.union([
rawBinaryFileData.extend({ metadata: rawFileMetadata.optional() }),
rawHashFileData.extend({ metadata: rawFileMetadata.optional() }),
rawHollowBinaryFileData.extend({ metadata: rawFileMetadata.optional() }),
rawHollowStringFileData.extend({ metadata: rawFileMetadata.optional() }),
rawLazyStringFileData.extend({ metadata: rawFileMetadata.optional() }),
rawStringFileData.extend({ metadata: rawFileMetadata.optional() }),
])
const rawFileMap = z.record(z.string(), rawFile)
const rawV2DocVersions = z.record(
z.string(),
z.strictObject({
pathname: z.string(),
v: z.number().int(),
})
)
const rawSnapshot = z.strictObject({
files: rawFileMap,
projectVersion: z.string().optional(),
v2DocVersions: rawV2DocVersions.nullish(),
timestamp: z.iso.datetime().optional(),
})
// mirrors Origin.fromRaw: three restore variants plus a generic kind
const rawOrigin = z.union([
z.strictObject({
kind: z.literal('restore'),
version: z.number().int(),
timestamp: z.iso.datetime(),
}),
z.strictObject({
kind: z.literal('file-restore'),
version: z.number().int(),
path: z.string(),
timestamp: z.iso.datetime(),
}),
z.strictObject({
kind: z.literal('project-restore'),
version: z.number().int(),
timestamp: z.iso.datetime(),
}),
z.strictObject({
kind: z.string(),
}),
])
// mirrors Operation.fromRaw: add file / edit file / move (or delete) file /
// set file metadata / no-op
const rawAddFileOperation = z.strictObject({
pathname: z.string(),
file: rawFile,
})
const rawMoveFileOperation = z.strictObject({
pathname: z.string(),
newPathname: z.string(),
})
const rawEditFileOperation = z.union([
rawTextOperation.extend({ pathname: z.string() }),
rawAddCommentOperation.extend({ pathname: z.string() }),
rawDeleteCommentOperation.extend({ pathname: z.string() }),
rawSetCommentStateOperation.extend({ pathname: z.string() }),
rawEditNoOperation.extend({ pathname: z.string() }),
])
const rawSetFileMetadataOperation = z.strictObject({
pathname: z.string(),
metadata: rawFileMetadata,
})
const rawNoOperation = z.strictObject({})
const rawOperation = z.union([
rawAddFileOperation,
rawEditFileOperation,
rawMoveFileOperation,
rawSetFileMetadataOperation,
rawNoOperation,
])
const rawChange = z.strictObject({
operations: z.array(rawOperation),
timestamp: z.iso.datetime(),
// null represents an anonymous author
authors: z.array(z.number().int().nullable()).optional(),
v2Authors: z.array(zz.objectId().nullable()).optional(),
origin: rawOrigin.optional(),
projectVersion: z.string().optional(),
v2DocVersions: rawV2DocVersions.optional(),
})
module.exports = {
rawTrackingProps,
rawClearTrackingProps,
@@ -86,4 +303,28 @@ module.exports = {
rawDeleteCommentOperation,
rawEditNoOperation,
rawEditOperation,
rawComment,
rawTrackedChange,
rawStringFileData,
rawLinkedFileData,
rawBlobHash,
rawFileMetadata,
rawHashFileData,
rawBinaryFileData,
rawLazyStringFileData,
rawHollowBinaryFileData,
rawHollowStringFileData,
rawFileData,
rawFile,
rawFileMap,
rawV2DocVersions,
rawSnapshot,
rawOrigin,
rawAddFileOperation,
rawMoveFileOperation,
rawEditFileOperation,
rawSetFileMetadataOperation,
rawNoOperation,
rawOperation,
rawChange,
}
+26 -62
View File
@@ -21,16 +21,9 @@ export type RangesBlob = {
export type RawRange = z.infer<typeof schemas.rawRange>
export type CommentRawData = {
id: string
ranges: RawRange[]
resolved?: boolean
}
export type CommentRawData = z.infer<typeof schemas.rawComment>
export type TrackedChangeRawData = {
range: RawRange
tracking: TrackingPropsRawData
}
export type TrackedChangeRawData = z.infer<typeof schemas.rawTrackedChange>
export type TrackingPropsRawData = z.infer<typeof schemas.rawTrackingProps>
@@ -40,37 +33,15 @@ export type ClearTrackingPropsRawData = z.infer<
export type TrackingDirective = TrackingProps | ClearTrackingProps
export type StringFileRawData = {
content: string
comments?: CommentRawData[]
trackedChanges?: TrackedChangeRawData[]
}
export type StringFileRawData = z.infer<typeof schemas.rawStringFileData>
export type RawOrigin = {
kind: string
}
export type RawOrigin = z.infer<typeof schemas.rawOrigin>
export type RawChange = {
operations: RawOperation[]
timestamp: string
authors?: (number | null)[]
v2Authors?: string[]
origin?: RawOrigin
projectVersion?: string
v2DocVersions?: RawV2DocVersions
}
export type RawChange = z.infer<typeof schemas.rawChange>
export type RawOperation =
| RawEditFileOperation
// TODO(das7pad): add types for all the other operations
| object
export type RawOperation = z.infer<typeof schemas.rawOperation>
export type RawSnapshot = {
files: RawFileMap
projectVersion?: string
v2DocVersions?: RawV2DocVersions | null
timestamp?: string
}
export type RawSnapshot = z.infer<typeof schemas.rawSnapshot>
export type RawHistory = {
snapshot: RawSnapshot
@@ -82,30 +53,25 @@ export type RawChunk = {
startVersion: number
}
export type RawFileMap = Record<string, RawFile>
export type RawFileMap = z.infer<typeof schemas.rawFileMap>
export type RawFile = { metadata?: Object } & RawFileData
export type RawFile = z.infer<typeof schemas.rawFile>
export type RawFileData =
| RawBinaryFileData
| RawHashFileData
| RawHollowBinaryFileData
| RawHollowStringFileData
| RawLazyStringFileData
| StringFileRawData
export type RawFileData = z.infer<typeof schemas.rawFileData>
export type RawHashFileData = { hash: string; rangesHash?: string }
export type RawBinaryFileData = { hash: string; byteLength: number }
export type RawLazyStringFileData = {
hash: string
stringLength: number
rangesHash?: string
operations?: RawEditOperation[]
}
export type RawHollowBinaryFileData = { byteLength: number }
export type RawHollowStringFileData = { stringLength: number }
export type RawHashFileData = z.infer<typeof schemas.rawHashFileData>
export type RawBinaryFileData = z.infer<typeof schemas.rawBinaryFileData>
export type RawLazyStringFileData = z.infer<
typeof schemas.rawLazyStringFileData
>
export type RawHollowBinaryFileData = z.infer<
typeof schemas.rawHollowBinaryFileData
>
export type RawHollowStringFileData = z.infer<
typeof schemas.rawHollowStringFileData
>
export type RawV2DocVersions = Record<string, { pathname: string; v: number }>
export type RawV2DocVersions = z.infer<typeof schemas.rawV2DocVersions>
export type RawInsertOp = z.infer<typeof schemas.rawInsertOp>
@@ -130,15 +96,13 @@ export type RawSetCommentStateOperation = z.infer<
export type RawEditNoOperation = z.infer<typeof schemas.rawEditNoOperation>
export type RawEditFileOperation = RawEditOperation & { pathname: string }
export type RawEditFileOperation = z.infer<typeof schemas.rawEditFileOperation>
export type RawEditOperation = z.infer<typeof schemas.rawEditOperation>
export type LinkedFileData = {
importedAt: string
provider: string
[other: string]: any
}
export type LinkedFileData = z.infer<typeof schemas.rawLinkedFileData>
export type FileMetadata = z.infer<typeof schemas.rawFileMetadata>
export type RawLabel = {
text: string
@@ -11,13 +11,21 @@ describe('File', function () {
let file = File.fromString('foo')
expect(file.getMetadata()).to.eql({})
// metadata passed in at construction time
// metadata passed in at construction time (the main-file marker written
// by the v1 history import)
file = File.fromString('foo', { main: true })
expect(file.getMetadata()).to.eql({ main: true })
// metadata set at runtime
file.setMetadata({ main: false })
expect(file.getMetadata()).to.eql({ main: false })
/** @type {import('../../lib/types').FileMetadata} */
const linkedFileMetadata = {
provider: 'project_file',
source_project_id: '507f1f77bcf86cd799439011',
source_entity_path: '/foo.bib',
importedAt: '2024-08-05T11:53:34.532Z',
}
file.setMetadata(linkedFileMetadata)
expect(file.getMetadata()).to.eql(linkedFileMetadata)
})
describe('toRaw', function () {
@@ -29,12 +37,17 @@ describe('File', function () {
metadata,
})
delete file.getMetadata().main
file.setMetadata({})
expect(file.toRaw()).to.eql({ hash: File.EMPTY_FILE_HASH })
})
it('returns a deep clone of metadata', function () {
const metadata = { externalFile: { id: 123 } }
/** @type {import('../../lib/types').FileMetadata} */
const metadata = {
provider: 'url',
url: 'https://example.com/foo.bib',
importedAt: '2024-08-05T11:53:34.532Z',
}
const file = File.fromHash(File.EMPTY_FILE_HASH, undefined, metadata)
const raw = file.toRaw()
const fileMetadata = file.getMetadata()
@@ -0,0 +1,156 @@
const { expect } = require('chai')
const { rawLinkedFileData, rawFileMetadata } = require('../../lib/schemas')
describe('schemas', function () {
describe('rawLinkedFileData', function () {
it('rejects an empty object', function () {
const result = rawLinkedFileData.safeParse({})
expect(result.success).to.equal(false)
})
it('rejects an unrecognized provider', function () {
const result = rawLinkedFileData.safeParse({
provider: 'not-a-provider',
})
expect(result.success).to.equal(false)
})
it('rejects a project_file provider with an unrecognized extra key', function () {
const result = rawLinkedFileData.safeParse({
provider: 'project_file',
source_entity_path: '/main.tex',
extraUnrecognizedKey: 'abcd',
})
expect(result.success).to.equal(false)
})
it('accepts a project_file provider with a valid source_project_id', function () {
const result = rawLinkedFileData.safeParse({
provider: 'project_file',
source_project_id: '507f1f77bcf86cd799439011',
source_entity_path: '/main.tex',
})
expect(result.success).to.equal(true)
})
it('accepts a project_file provider with a missing source_project_id', function () {
const result = rawLinkedFileData.safeParse({
provider: 'project_file',
source_entity_path: '/main.tex',
})
expect(result.success).to.equal(true)
})
it('rejects a project_file provider with a malformed source_project_id', function () {
const result = rawLinkedFileData.safeParse({
provider: 'project_file',
source_project_id: 'not-an-object-id',
source_entity_path: '/main.tex',
})
expect(result.success).to.equal(false)
})
it('accepts a project_output_file provider with a valid source_project_id', function () {
const result = rawLinkedFileData.safeParse({
provider: 'project_output_file',
source_project_id: '507f1f77bcf86cd799439011',
source_output_file_path: 'output.pdf',
})
expect(result.success).to.equal(true)
})
it('rejects a project_output_file provider with a malformed source_project_id', function () {
const result = rawLinkedFileData.safeParse({
provider: 'project_output_file',
source_project_id: 'not-an-object-id',
source_output_file_path: 'output.pdf',
})
expect(result.success).to.equal(false)
})
it('accepts a project_output_file provider with a valid build_id', function () {
const result = rawLinkedFileData.safeParse({
provider: 'project_output_file',
source_output_file_path: 'output.pdf',
build_id: '1234-abcd',
})
expect(result.success).to.equal(true)
})
it('rejects a project_output_file provider with a malformed build_id', function () {
const result = rawLinkedFileData.safeParse({
provider: 'project_output_file',
source_output_file_path: 'output.pdf',
build_id: 'not-a-valid-build-id',
})
expect(result.success).to.equal(false)
})
for (const provider of ['mendeley', 'zotero', 'papers']) {
it(`accepts a ${provider} provider with an opaque group_id`, function () {
const result = rawLinkedFileData.safeParse({
provider,
group_id: 'abcd',
})
expect(result.success).to.equal(true)
})
it(`accepts a ${provider} provider with a missing group_id`, function () {
const result = rawLinkedFileData.safeParse({ provider })
expect(result.success).to.equal(true)
})
it(`rejects a ${provider} provider with a group_id containing a path separator`, function () {
const result = rawLinkedFileData.safeParse({
provider,
group_id: 'abcd/../../etc/passwd',
})
expect(result.success).to.equal(false)
})
it(`rejects a ${provider} provider with a group_id of ".."`, function () {
const result = rawLinkedFileData.safeParse({
provider,
group_id: '..',
})
expect(result.success).to.equal(false)
})
}
})
describe('rawFileMetadata', function () {
it('accepts a legacy v1 main flag', function () {
const result = rawFileMetadata.safeParse({
main: true,
})
expect(result.success).to.equal(true)
})
it('accepts a legacy v1 main flag with an importedAt timestamp', function () {
const result = rawFileMetadata.safeParse({
main: true,
importedAt: '2024-01-01T00:00:00.000Z',
})
expect(result.success).to.equal(true)
})
it('accepts an importedAt timestamp with no provider', function () {
const result = rawFileMetadata.safeParse({
importedAt: '2024-01-01T00:00:00.000Z',
})
expect(result.success).to.equal(true)
})
it('rejects a malformed importedAt timestamp with no provider', function () {
const result = rawFileMetadata.safeParse({
importedAt: 'not-a-date',
})
expect(result.success).to.equal(false)
})
it('accepts an empty object', function () {
const result = rawFileMetadata.safeParse({})
expect(result.success).to.equal(true)
})
})
})
@@ -810,7 +810,7 @@ class SyncUpdateExpander {
shouldUpdate = Object.entries(file.metadata).some(
([k, v]) => metaData[k] !== v
)
} else if (metaData.provider) {
} else if ('provider' in metaData && metaData.provider) {
// overwritten by non-linked-file with same hash
// or overwritten by doc
shouldUpdate = true
+13 -1
View File
@@ -20,6 +20,7 @@ import socketIOClient from 'socket.io-client'
import http from 'node:http'
import Router from './app/js/Router.js'
import WebsocketLoadBalancer from './app/js/WebsocketLoadBalancer.js'
import { z, parseReq, handleValidationError } from '@overleaf/validation-tools'
logger.initialize('real-time')
Metrics.event_loop.monitor(logger)
@@ -153,8 +154,17 @@ app.get('/status', function (req, res) {
}
})
const debugEventsSchema = z.object({
query: z.strictObject({
count: z.coerce.number().int().optional(),
}),
})
app.get('/debug/events', function (req, res) {
Settings.debugEvents = parseInt(req.query.count, 10) || 20
const {
query: { count },
} = parseReq(req, debugEventsSchema)
Settings.debugEvents = count || 20
logger.info({ count: Settings.debugEvents }, 'starting debug mode')
res.send(`debug mode will log next ${Settings.debugEvents} events`)
})
@@ -193,6 +203,8 @@ app.use(Metrics.http.monitor(logger))
Router.configure(app, io, sessionSockets)
app.use(handleValidationError)
WebsocketLoadBalancer.listenForEditorEvents(io)
const { port } = Settings.internal.realTime
+65 -14
View File
@@ -2,10 +2,56 @@ import WebsocketLoadBalancer from './WebsocketLoadBalancer.js'
import DrainManager from './DrainManager.js'
import ConnectedUsersManager from './ConnectedUsersManager.js'
import logger from '@overleaf/logger'
import { z, zz, parseReq } from '@overleaf/validation-tools'
import { clientIdSchema } from './schemas.js'
const countConnectedClientsSchema = z.object({
params: z.strictObject({
projectId: zz.objectId(),
}),
})
// A generic relay: real-time never reads a named field from this payload,
// it forwards it verbatim to connected socket.io clients in the project's
// room (see WebsocketLoadBalancer.emitToRoom) as the named `message` event.
// The payload is a genuinely open, structured map (or an array of them, one
// message per element) -- there is no fixed shape to model here.
const sendMessageSchema = z.object({
params: z.strictObject({
project_id: zz.objectId(),
message: z.string().min(1),
}),
body: z
.array(z.record(z.string(), z.unknown()))
.or(z.record(z.string(), z.unknown())),
})
const startDrainSchema = z.object({
query: z.strictObject({
rate: z.coerce.number().default(4),
}),
})
// Rollout-temporary fallback (loosened primary schema; no zod validation
// existed for this route on main); delete when this route's
// REQ_VALIDATION_MODE instrumentation is removed.
const startDrainFallbackSchema = z.object({
query: z.object({
rate: z.coerce.number().default(4),
}),
})
const disconnectClientSchema = z.object({
params: z.strictObject({
client_id: clientIdSchema,
}),
})
export default {
countConnectedClients(req, res) {
const { projectId } = req.params
const {
params: { projectId },
} = parseReq(req, countConnectedClientsSchema, { logOnly: true })
ConnectedUsersManager.countConnectedClients(
projectId,
(err, nConnectedClients) => {
@@ -19,29 +65,32 @@ export default {
},
sendMessage(req, res) {
logger.debug({ message: req.params.message }, 'sending message')
if (Array.isArray(req.body)) {
for (const payload of req.body) {
const { params, body } = parseReq(req, sendMessageSchema, {
logOnly: true,
})
logger.debug({ message: params.message }, 'sending message')
if (Array.isArray(body)) {
for (const payload of body) {
WebsocketLoadBalancer.emitToRoom(
req.params.project_id,
req.params.message,
params.project_id,
params.message,
payload
)
}
} else {
WebsocketLoadBalancer.emitToRoom(
req.params.project_id,
req.params.message,
req.body
)
WebsocketLoadBalancer.emitToRoom(params.project_id, params.message, body)
}
res.sendStatus(204)
},
startDrain(req, res) {
const io = req.app.get('io')
let rate = req.query.rate || '4'
rate = parseFloat(rate) || 0
const {
query: { rate },
} = parseReq(req, startDrainSchema, {
logOnly: true,
fallbackSchema: startDrainFallbackSchema,
})
logger.info({ rate }, 'setting client drain rate')
DrainManager.startDrain(io, rate)
res.sendStatus(204)
@@ -49,7 +98,9 @@ export default {
disconnectClient(req, res, next) {
const io = req.app.get('io')
const { client_id: clientId } = req.params
const {
params: { client_id: clientId },
} = parseReq(req, disconnectClientSchema, { logOnly: true })
const client = io.sockets.sockets[clientId]
if (!client) {
+12 -1
View File
@@ -1,5 +1,14 @@
import { z, parseReq } from '@overleaf/validation-tools'
import { clientIdSchema } from './schemas.js'
let HttpController
const getConnectedClientSchema = z.object({
params: z.strictObject({
client_id: clientIdSchema,
}),
})
export default HttpController = {
// The code in this controller is hard to unit test because of a lot of
// dependencies on internal socket.io methods. It is not critical to the running
@@ -42,7 +51,9 @@ export default HttpController = {
},
getConnectedClient(req, res) {
const { client_id: clientId } = req.params
const {
params: { client_id: clientId },
} = parseReq(req, getConnectedClientSchema, { logOnly: true })
const io = req.app.get('io')
const ioClient = io.sockets.sockets[clientId]
if (!ioClient) {
+77 -4
View File
@@ -19,18 +19,24 @@ const HOSTNAME = os.hostname()
const SERVER_PING_INTERVAL = 15000
const SERVER_PING_LATENCY_THRESHOLD = 5000
const joinDocSchema = z.object({
const joinDocSchema = z.strictObject({
doc_id: zz.objectId(),
fromVersion: z.number().int().optional(),
options: z.object(),
// known fields read by WebsocketController.joinDoc; see
// document-container.ts's joinDoc() call sites on the frontend
options: z.strictObject({
age: z.number().optional(),
supportsHistoryOT: z.boolean().optional(),
encodeRanges: z.boolean().optional(),
}),
})
const applyOtUpdateSchema = z.object({
const applyOtUpdateSchema = z.strictObject({
doc_id: zz.objectId(),
update: z.strictObject({
doc: zz.objectId().optional(),
dupIfSource: z.array(z.string()).optional(),
hash: z.string().optional(),
hash: schemas.rawBlobHash.optional(),
lastV: z.number().optional(),
meta: z
.strictObject({
@@ -56,6 +62,9 @@ const applyOtUpdateSchema = z.object({
z.strictObject({
c: z.string(),
p: z.number().int().min(0),
// comment thread ids are usually ObjectIds, but legacy
// documents carry arbitrary strings (see docstore's ranges
// schema for the same caveat) -- kept as a plain string
t: z.string(),
u: z.boolean().optional(),
})
@@ -67,6 +76,37 @@ const applyOtUpdateSchema = z.object({
}),
})
// Fields the client actually sends; see online-users-context.tsx's
// clientTracking.updatePosition emit call. The server then mutates this
// object in place to add its own fields (id/user_id/email/name) before
// broadcasting it on, so those must NOT be accepted from the client.
const cursorDataSchema = z.strictObject({
row: z.number().optional(),
column: z.number().optional(),
doc_id: zz.objectId().nullish(),
})
// A diagnostic echo channel (see use-socket-manager.ts): the payload is
// never read by name, only reflected back to the caller, so this is a
// genuinely open map rather than a fixed shape to approximate. `data` is a
// top-level positional socket.io argument, so an omitted value is
// JSON-encoded as `null`, not `undefined` -- hence .nullish().
const debugDataSchema = z.record(z.string(), z.unknown()).nullish()
// Positional args the client echoes back from a 'serverPing' (see the
// serverPing emit below and connection-manager.ts's sendPingResponse, whose
// params are all optionally-typed); used only for logging/latency metrics.
// A socket.io array of positional args is JSON-encoded, so an omitted
// trailing value arrives as `null`, not `undefined` -- hence .nullish().
const clientPongArgsSchema = z.tuple([
z.number().nullish(),
z.number().nullish(),
z.string().nullish(),
z.string().nullish(),
z.string().nullish(),
z.string().nullish(),
])
let Router
export default Router = {
@@ -334,6 +374,21 @@ export default Router = {
clientTransport,
clientSessionId
) {
try {
clientPongArgsSchema.parse([
receivedPingId,
sentTimestamp,
serverTransport,
serverSessionId,
clientTransport,
clientSessionId,
])
} catch (error) {
// fire-and-forget message, no callback to report back to
return Router._handleError(() => {}, error, client, 'clientPong', {
validation: 1,
})
}
pongId = receivedPingId
const receivedTimestamp = Date.now()
if (
@@ -392,6 +447,13 @@ export default Router = {
if (typeof callback !== 'function') {
return Router._handleInvalidArguments(client, 'debug', arguments)
}
try {
data = debugDataSchema.parse(data)
} catch (error) {
return Router._handleError(callback, error, client, 'debug', {
validation: 1,
})
}
logger.info(
{ publicId: client.publicId, clientId: client.id },
@@ -575,6 +637,17 @@ export default Router = {
arguments
)
}
try {
cursorDataSchema.parse(cursorData)
} catch (error) {
return Router._handleError(
callback,
error,
client,
'clientTracking.updatePosition',
{ validation: 1, disconnect: 1 }
)
}
WebsocketController.updateClientPosition(
client,
+15
View File
@@ -0,0 +1,15 @@
import { z } from '@overleaf/validation-tools'
// A socket.io client id, as generated by this service's pinned socket.io
// fork's Manager.prototype.generateId (node_modules/socket.io/lib/
// manager.js): 15 random bytes, base64-encoded, with '/'->'_' and '+'->'-'
// substituted -- i.e. RFC 4648 base64url, always exactly 20 characters (15
// is a multiple of 3, so standard base64 of it never needs '=' padding).
// Used by HttpController.getConnectedClient and
// HttpApiController.disconnectClient for a dynamic-property lookup into
// socket.io's internal client map (`io.sockets.sockets[clientId]`), so both
// constraints matter: the base64url charset alone still contains
// '__proto__', 'constructor', and 'prototype' as valid strings, but none of
// those is 20 characters long, so the length check is what actually
// excludes them.
export const clientIdSchema = z.base64url().length(20)
@@ -1011,4 +1011,87 @@ describe('applyOtUpdate', function () {
return null
})
})
describe('when authorized with an invalid hash', function () {
before(function (done) {
this.update = {
op: [{ i: 'foo', p: 42 }],
v: 42,
hash: 'not-a-valid-hash',
}
return async.series(
[
cb => {
return FixturesManager.setUpProject(
{
privilegeLevel: 'readAndWrite',
},
(e, { project_id: projectId, user_id: userId }) => {
this.project_id = projectId
this.user_id = userId
return cb(e)
}
)
},
cb => {
return FixturesManager.setUpDoc(
this.project_id,
{ lines: this.lines, version: this.version, ops: this.ops },
(e, { doc_id: docId }) => {
this.doc_id = docId
return cb(e)
}
)
},
cb => {
this.client = RealTimeClient.connect(this.project_id, cb)
},
cb => {
return this.client.emit('joinDoc', this.doc_id, cb)
},
cb => {
return this.client.emit(
'applyOtUpdate',
this.doc_id,
this.update,
error => {
this.error = error
return cb()
}
)
},
],
done
)
})
it('should return an error', function () {
return expect(this.error).to.exist
})
it('should disconnect the client', function (done) {
return setTimeout(() => {
this.client.socket.connected.should.equal(false)
return done()
}, 300)
})
return it('should not put the update in redis', function (done) {
rclient.llen(
redisSettings.documentupdater.key_schema.pendingProjectUpdates({
project_id: this.project_id,
}),
(error, len) => {
if (error) return done(error)
len.should.equal(0)
return done()
}
)
return null
})
})
})
@@ -194,7 +194,7 @@ describe('clientTracking', function () {
})
})
return describe('when an anonymous client updates its cursor location', function () {
describe('when an anonymous client updates its cursor location', function () {
before(function (done) {
return async.series(
[
@@ -288,4 +288,161 @@ describe('clientTracking', function () {
])
})
})
describe('when a client updates its cursor location with no document open', function () {
describe('with doc_id set to null', function () {
before(function (done) {
return async.series(
[
cb => {
return FixturesManager.setUpProject(
{
privilegeLevel: 'owner',
project: { name: 'Test Project' },
},
(error, { user_id: userId, project_id: projectId }) => {
if (error) return done(error)
this.user_id = userId
this.project_id = projectId
return cb()
}
)
},
cb => {
this.client = RealTimeClient.connect(this.project_id, cb)
},
cb => {
return this.client.emit(
'clientTracking.updatePosition',
{
row: 42,
column: 36,
doc_id: null,
},
error => {
this.error = error
// Give a possible disconnect a chance to happen.
return setTimeout(cb, 300)
}
)
},
],
done
)
})
it('should not return an error', function () {
expect(this.error).to.not.exist
})
it('should keep the client connected', function () {
expect(this.client.socket.connected).to.equal(true)
})
})
describe('with doc_id omitted', function () {
before(function (done) {
return async.series(
[
cb => {
return FixturesManager.setUpProject(
{
privilegeLevel: 'owner',
project: { name: 'Test Project' },
},
(error, { user_id: userId, project_id: projectId }) => {
if (error) return done(error)
this.user_id = userId
this.project_id = projectId
return cb()
}
)
},
cb => {
this.client = RealTimeClient.connect(this.project_id, cb)
},
cb => {
return this.client.emit(
'clientTracking.updatePosition',
{
row: 42,
column: 36,
},
error => {
this.error = error
// Give a possible disconnect a chance to happen.
return setTimeout(cb, 300)
}
)
},
],
done
)
})
it('should not return an error', function () {
expect(this.error).to.not.exist
})
it('should keep the client connected', function () {
expect(this.client.socket.connected).to.equal(true)
})
})
describe('with an invalid doc_id', function () {
before(function (done) {
return async.series(
[
cb => {
return FixturesManager.setUpProject(
{
privilegeLevel: 'owner',
project: { name: 'Test Project' },
},
(error, { user_id: userId, project_id: projectId }) => {
if (error) return done(error)
this.user_id = userId
this.project_id = projectId
return cb()
}
)
},
cb => {
this.client = RealTimeClient.connect(this.project_id, cb)
},
cb => {
return this.client.emit(
'clientTracking.updatePosition',
{
row: 42,
column: 36,
doc_id: 'not-a-valid-id',
},
error => {
this.error = error
// Give the disconnect a chance to happen.
return setTimeout(cb, 300)
}
)
},
],
done
)
})
it('should return an invalid id error', function () {
return this.error.message.should.equal('invalid Mongo ObjectId')
})
it('should disconnect the client', function () {
return expect(this.client.socket.connected).to.equal(false)
})
})
})
})
@@ -0,0 +1,144 @@
import { expect } from 'chai'
import async from 'async'
import {
fetchNothing,
fetchString,
RequestFailedError,
} from '@overleaf/fetch-utils'
import RealTimeClient from './helpers/RealTimeClient.js'
import FixturesManager from './helpers/FixturesManager.js'
describe('sendMessage', function () {
before(function (done) {
async.series(
[
cb => {
FixturesManager.setUpProject(
{ privilegeLevel: 'owner' },
(error, { project_id: projectId }) => {
this.project_id = projectId
cb(error)
}
)
},
cb => {
this.client = RealTimeClient.connect(this.project_id, cb)
},
],
done
)
})
describe('with a well-formed object payload', function () {
before(function (done) {
this.client.once('custom:test:event', payload => {
this.received = payload
done()
})
fetchNothing(
`http://127.0.0.1:3026/project/${this.project_id}/message/custom:test:event`,
{
method: 'POST',
json: { foo: 'bar' },
}
).catch(done)
})
it('should relay the message to connected clients in the room', function () {
expect(this.received).to.deep.equal({ foo: 'bar' })
})
})
describe('with a malformed project id', function () {
it('should return 404', async function () {
try {
await fetchNothing(
'http://127.0.0.1:3026/project/not-a-valid-id/message/custom:test:event',
{
method: 'POST',
json: { foo: 'bar' },
}
)
expect.fail('should have thrown')
} catch (error) {
expect(error).to.be.instanceOf(RequestFailedError)
expect(error.response.status).to.equal(404)
}
})
})
})
describe('disconnectClient', function () {
before(function (done) {
async.series(
[
cb => {
FixturesManager.setUpProject(
{ privilegeLevel: 'owner' },
(error, { project_id: projectId }) => {
this.project_id = projectId
cb(error)
}
)
},
cb => {
this.client = RealTimeClient.connect(this.project_id, cb)
},
],
done
)
})
describe('with a malformed client id', function () {
it('should return a schema-validation 404, leaving the real client connected', async function () {
try {
await fetchNothing(
'http://127.0.0.1:3026/client/__proto__/disconnect',
{
method: 'POST',
}
)
expect.fail('should have thrown')
} catch (error) {
expect(error).to.be.instanceOf(RequestFailedError)
expect(error.response.status).to.equal(404)
// The domain-level 404 for an already-disconnected client has no
// JSON body -- a JSON body with statusCode/error here proves this
// request was rejected by the client_id schema (InvalidParamsError
// via handleValidationError), not by the "already disconnected"
// check in HttpApiController.disconnectClient.
const body = JSON.parse(error.body)
expect(body).to.have.property('statusCode', 404)
expect(body.error).to.be.a('string')
}
expect(this.client.socket.connected).to.equal(true)
})
})
describe('with a real client id', function () {
it('should disconnect the client', function (done) {
this.client.on('disconnect', () => done())
fetchNothing(
`http://127.0.0.1:3026/client/${this.client.socket.sessionid}/disconnect`,
{ method: 'POST' }
).catch(done)
})
})
})
describe('GET /debug/events', function () {
it('should accept a numeric count', async function () {
const body = await fetchString('http://127.0.0.1:3026/debug/events?count=5')
expect(body).to.equal('debug mode will log next 5 events')
})
it('should reject a non-numeric count', async function () {
try {
await fetchNothing('http://127.0.0.1:3026/debug/events?count=abc')
expect.fail('should have thrown')
} catch (error) {
expect(error).to.be.instanceOf(RequestFailedError)
expect(error.response.status).to.equal(400)
}
})
})
@@ -27,6 +27,25 @@ describe('HttpControllerTests', function () {
expect(error.response.status).to.equal(404)
}
})
it('should return a schema-validation 404 for a __proto__-shaped client id', async function () {
const clientId = '__proto__'
try {
await fetchNothing(`http://127.0.0.1:3026/clients/${clientId}`)
expect.fail('request should have failed')
} catch (error) {
expect(error).to.be.instanceof(RequestFailedError)
expect(error.response.status).to.equal(404)
// The domain-level 404 (`res.sendStatus(404)` for an id that parses
// but has no matching client) has no JSON body. Asserting a JSON
// body with a statusCode/error here proves this request was
// rejected by the client_id schema (InvalidParamsError via
// handleValidationError), not by the "client not found" check.
const body = JSON.parse(error.body)
expect(body).to.have.property('statusCode', 404)
expect(body.error).to.be.a('string')
}
})
})
describe('with a user and after joining a project', function () {
@@ -83,8 +83,8 @@ describe('leaveDoc', function () {
})
})
return it('should return an error', function () {
return expect(this.error).to.exist
return it('should return an invalid id error', function () {
return this.error.message.should.equal('invalid Mongo ObjectId')
})
})
})
@@ -403,6 +403,11 @@ describe('MatrixTests', function () {
})
describe('send updates', function () {
// Fake blob hashes, distinguishable from each other, standing in for
// whichever client sent the update.
const USER_HASH = '1111111111111111111111111111111111111111'
const PRIVATE_HASH = '2222222222222222222222222222222222222222'
let receivedArgs, submittedUpdates, update
beforeEach(function cleanup(done) {
@@ -427,7 +432,7 @@ describe('MatrixTests', function () {
return this.skip()
}
const userUpdate = Object.assign({}, update.op, {
hash: 'user',
hash: USER_HASH,
})
client.emit(
@@ -443,7 +448,7 @@ describe('MatrixTests', function () {
beforeEach(function sendAsPrivateUserForReferenceOp(done) {
const privateUpdate = Object.assign({}, update.op, {
hash: 'private',
hash: PRIVATE_HASH,
})
privateClient.emit(
@@ -477,7 +482,7 @@ describe('MatrixTests', function () {
it('should submit the private users message only', function () {
expect(submittedUpdates).to.have.length(1)
const update = JSON.parse(submittedUpdates[0])
expect(update.hash).to.equal('private')
expect(update.hash).to.equal(PRIVATE_HASH)
})
})
})
@@ -0,0 +1,137 @@
import { expect } from 'chai'
import async from 'async'
import RealTimeClient from './helpers/RealTimeClient.js'
import FixturesManager from './helpers/FixturesManager.js'
describe('debug message', function () {
before(function (done) {
async.series(
[
cb => {
FixturesManager.setUpProject(
{ privilegeLevel: 'owner' },
(error, { project_id: projectId }) => {
this.project_id = projectId
cb(error)
}
)
},
cb => {
this.client = RealTimeClient.connect(this.project_id, cb)
},
],
done
)
})
describe('with a well-formed data payload', function () {
before(function (done) {
this.client.emit('debug', { time: 123 }, (response, ...rest) => {
this.response = response
this.returnedArgs = rest
done()
})
})
it('should echo the data back without an error', function () {
expect(this.returnedArgs).to.deep.equal([])
expect(this.response.data).to.deep.equal({ time: 123 })
})
it('should include the client view', function () {
expect(this.response.client.publicId).to.equal(this.client.publicId)
})
})
describe('with no data payload', function () {
before(function (done) {
this.client.emit('debug', undefined, (response, ...rest) => {
this.response = response
this.returnedArgs = rest
done()
})
})
it('should echo back with no error', function () {
expect(this.returnedArgs).to.deep.equal([])
expect(this.response.data).to.not.exist
})
})
describe('with a non-object data payload', function () {
before(function (done) {
this.client.emit('debug', 'not-an-object', error => {
this.error = error
done()
})
})
it('should return a validation error', function () {
expect(this.error).to.exist
})
})
})
describe('clientPong message', function () {
before(function (done) {
async.series(
[
cb => {
FixturesManager.setUpProject(
{ privilegeLevel: 'owner' },
(error, { project_id: projectId }) => {
this.project_id = projectId
cb(error)
}
)
},
cb => {
this.client = RealTimeClient.connect(this.project_id, cb)
},
],
done
)
})
describe('with well-formed pong args', function () {
before(function (done) {
this.client.emit(
'clientPong',
1,
Date.now(),
'websocket',
this.client.socket.sessionid,
'websocket',
this.client.socket.sessionid
)
// clientPong has no ack; give the server a moment to process it, then
// confirm the connection is still healthy via a normal RPC.
setTimeout(() => {
this.client.emit('clientTracking.getConnectedUsers', error => {
this.error = error
done()
})
}, 50)
})
it('should not disrupt the connection', function () {
expect(this.error).to.not.exist
})
})
describe('with malformed pong args', function () {
before(function (done) {
this.client.emit('clientPong', 'not-a-number', 'not-a-number-either')
setTimeout(() => {
this.client.emit('clientTracking.getConnectedUsers', error => {
this.error = error
done()
})
}, 50)
})
it('should be rejected but not disrupt the connection', function () {
expect(this.error).to.not.exist
})
})
})
@@ -11,9 +11,31 @@
*/
import sinon from 'sinon'
import express from 'express'
import { z, zz, parseReq } from '@overleaf/validation-tools'
let MockDocUpdaterServer
// Mirrors the request shape of document-updater's real routes (see
// services/document-updater/app/js/HttpController.js).
const getDocumentRequestSchema = z.object({
params: z.strictObject({
project_id: zz.objectId(),
doc_id: zz.objectId(),
}),
query: z.strictObject({
fromVersion: z.coerce.number().int().optional(),
// sent by real-time's DocumentUpdaterManager.getDocument alongside
// fromVersion; unused here since this mock only reads fromVersion.
historyOTSupport: z.stringbool().optional(),
}),
})
const deleteProjectRequestSchema = z.object({
params: z.strictObject({
project_id: zz.objectId(),
}),
})
export default MockDocUpdaterServer = {
docs: {},
@@ -31,9 +53,9 @@ export default MockDocUpdaterServer = {
deleteProject: sinon.stub().callsArg(1),
getDocumentRequest(req, res, next) {
const { project_id: projectId, doc_id: docId } = req.params
let { fromVersion } = req.query
fromVersion = parseInt(fromVersion, 10)
const { params, query } = parseReq(req, getDocumentRequestSchema)
const { project_id: projectId, doc_id: docId } = params
const { fromVersion } = query
return MockDocUpdaterServer.getDocument(
projectId,
docId,
@@ -51,7 +73,8 @@ export default MockDocUpdaterServer = {
},
deleteProjectRequest(req, res, next) {
const { project_id: projectId } = req.params
const { params } = parseReq(req, deleteProjectRequestSchema)
const { project_id: projectId } = params
return MockDocUpdaterServer.deleteProject(projectId, error => {
if (error != null) {
return next(error)
@@ -11,9 +11,22 @@
*/
import sinon from 'sinon'
import express from 'express'
import { z, zz, parseReq } from '@overleaf/validation-tools'
let MockWebServer
// Mirrors the request shape of web's real POST /project/:project_id/join route (see WebApiManager.joinProject).
const joinProjectRequestSchema = z.object({
params: z.strictObject({
project_id: zz.objectId(),
}),
body: z.strictObject({
// 'anonymous-user' is a valid sentinel, not always a real user id
userId: zz.objectId().or(z.literal('anonymous-user')),
anonymousAccessToken: z.string().optional(),
}),
})
export default MockWebServer = {
projects: {},
privileges: {},
@@ -43,8 +56,9 @@ export default MockWebServer = {
},
joinProjectRequest(req, res, next) {
const { project_id: projectId } = req.params
const { anonymousAccessToken, userId } = req.body
const { params, body } = parseReq(req, joinProjectRequestSchema)
const { project_id: projectId } = params
const { anonymousAccessToken, userId } = body
if (projectId === '404404404404404404404404') {
// not-found
return res.status(404).send()