From 04871d60493cea07dd4bab6fe745d0d94f7e34f8 Mon Sep 17 00:00:00 2001 From: Jakob Ackermann Date: Thu, 27 Aug 2026 12:16:01 +0200 Subject: [PATCH] [editor-core] extract more shared utils (#36954) * [editor-core] share the blob hash and the doc-vs-file rule The git blob hash and the rule that decides whether a file is a doc or a binary file both have to reach the same answer in every service that writes to history, so they move into overleaf-editor-core rather than being spelled out again on each side. blob_utils takes over lib/blob_string_length.js: hashing and the string-length rule answered two halves of the same question -- what history needs to know about content before it can store it -- and each carried the same warning about staying off index.js, which is bundled for the browser. One module states it once, and blobForFile joins them, describing content on disk as the blob it amounts to. The hashing functions are flat rather than a namespace of their own: in a module that also holds the string-length rule, a bare fromStream says nothing about what it makes. history-v1's blob store and recover_zip follow the module to its new name. file_type_detector holds the doc-vs-file rule, Node-only and deep-imported for the same reason. It takes textExtensions, editableFilenames and maxDocLength as configuration rather than reading settings: the limit is web's 2 MiB and not editor-core's own 3 MiB, and the surrogate check covers both halves of a pair where editor-core's covers the high half. buildSetContentOperations' test spelled the blob header out for itself, with the separator as a literal NUL byte, which had git reading the file as binary. It hashes through blobHashFromString now, so it does not spell the header at all. Co-Authored-By: Claude Opus 5 (1M context) * [editor-core] name the client a change came from on the origin itself `EditorOrigin` carried an `editorId` that only an editor change could have. What the id is for -- recognising a change again, so history does not apply a resend twice and the client does not apply its own change back -- is a property of the client that submitted it rather than of what it did, so it moves onto `Origin` as an optional `historyClientId` and `EditorOrigin` goes. - `Origin` takes `(kind, historyClientId)` and answers `getHistoryClientId()`; `dropHistoryClientId()` is what the chunk store uses to strip the id from all but a client's latest change, which now leaves the rest of the origin alone -- a restore that a client submitted stays a restore. - `toRaw` is built a layer at a time: the base writes the kind and the id, a subclass spreads that and adds the fields it owns. - The named kinds are their own schemas and discriminate on `kind`, so a payload claiming one is checked against that shape and the error says what it got wrong. The catch-all cannot join them -- its kind is any string -- so it sits alongside and is refined to refuse the named kinds, which it used to swallow: `{kind: 'restore'}` validated as a kind and nothing else, then read back as a restore that recorded nothing about what it restored. - `lib/origin/*.js` gains `// @ts-check`. Without it the annotations in there were decorative: the lazily required subclasses were `any`, so nothing handed to them was checked, and `fromRaw` narrows with `in` to drop the catch-all variant, which a kind check cannot. The callers that stamp the id -- real-time's handshake flag, web's applyChange body -- come with the rest of the branch. Co-Authored-By: Claude Opus 5 (1M context) * [editor-core] recognise a change a writer that is not an editor placed The counterpart of `isSameEditorChange` for writers that are not an editor client: an integration that commits a batch on a user's behalf has to recognise that batch coming back, because a lost response does not say whether the commit landed and resubmitting one that did applies its removes twice. It compares the same three fields, and for the same reason -- a rebase rewrites a change's operations but leaves its origin, authors and timestamp alone -- with the timestamp compared as a point in time rather than as a string, so that two spellings of the same instant cannot read as different changes. github-sync is the first caller, and comes with the rest of the branch. Co-Authored-By: Claude Opus 5 (1M context) * [editor-core] load an editable file's content, or say it has none Only an editable file has content: any other kind is bytes addressed by hash, and hollow data records the size of content it does not have. Both answer `getContent()` with null, so every caller that wants the content of a doc loaded it, checked the data's class, and reached for its own error when the check failed. `File#loadEager` does that once and answers the data that always has the content. `buildSetContentOperations` takes a `ReadWriteBlobStore` -- everything but looking a blob up by hash, which is all it takes to build operations over content -- so a caller holding a store that cannot answer `getBlob` can still use it. Its metadata argument is `FileMetadata` rather than `Object`, which is what the schema already says a file's metadata is. Co-Authored-By: Claude Opus 5 (1M context) * [editor-core] say that a pair of operations that do not fit is unprocessable compose and transform threw a bare Error when the two operations do not describe the same content. Everything that answers a client for these -- history-v1's import routes, the git-bridge push, project-history's resync -- decides on UnprocessableError, so a bare Error reached them as an internal failure: a 500, which reads as worth retrying, for a pair that fails the same way every time. A client that resubmits on a 500 resubmits forever. The guards that cannot be reached through the API keep their bare Error: a TextOperation composed with something that is not one, and the branch whose message says it should not happen. Co-Authored-By: Claude Opus 5 * [history-v1] hash blobs with the rule editor-core holds storage/lib/blob_hash.js was a bluebird-era implementation of the same git blob hash editor-core holds, exported from the storage barrel and used across the storage layer, the api and the tests. All of it reads the shared rule now, and the test that existed only to check the copy is gone -- editor-core's suite covers the same non-ASCII case. Co-Authored-By: Claude Opus 5 (1M context) --------- Co-authored-by: Claude Opus 5 (1M context) GitOrigin-RevId: 5a0c1e9a06170a0164c01a26a7bb2d205726d531 --- libraries/overleaf-editor-core/index.js | 4 +- libraries/overleaf-editor-core/lib/blob.js | 2 +- .../lib/blob_string_length.js | 54 --- .../overleaf-editor-core/lib/blob_utils.js | 157 +++++++++ .../lib/build_set_content_operations.js | 9 +- .../lib/change_identity.js | 69 +++- libraries/overleaf-editor-core/lib/file.js | 20 ++ .../lib/file_type_detector.js | 205 ++++++++++++ .../lib/operation/text_operation.js | 16 +- .../lib/origin/editor_origin.js | 55 --- .../overleaf-editor-core/lib/origin/index.js | 94 ++++-- .../lib/origin/restore_file_origin.js | 25 +- .../lib/origin/restore_origin.js | 24 +- .../lib/origin/restore_project_origin.js | 24 +- libraries/overleaf-editor-core/lib/schemas.js | 87 +++-- .../lib/text_file_defaults.js | 13 +- libraries/overleaf-editor-core/lib/types.ts | 16 + .../test/unit/blob_string_length.test.js | 104 ------ .../test/unit/blob_utils.test.js | 313 ++++++++++++++++++ .../unit/build_set_content_operations.test.js | Bin 15536 -> 15434 bytes .../test/unit/change_identity.test.js | 99 +++++- .../test/unit/editor_origin.test.js | 102 ------ .../test/unit/file.test.js | 80 +++++ .../test/unit/file_type_detector.test.js | 247 ++++++++++++++ .../test/unit/origin.test.js | 212 ++++++++++++ .../test/unit/text_operation.test.js | 21 ++ .../history-v1/api/controllers/projects.js | 4 +- services/history-v1/storage/index.js | 1 - .../history-v1/storage/lib/backupVerifier.mjs | 4 +- services/history-v1/storage/lib/blob_hash.js | 80 ----- .../storage/lib/blob_store/index.js | 9 +- .../storage/lib/hash_check_blob_store.js | 4 +- .../history-v1/storage/scripts/backup.mjs | 2 +- .../history-v1/storage/scripts/recover_zip.js | 4 +- .../test/acceptance/js/api/end_to_end.test.js | 8 +- .../js/api/project_hashed_content.test.js | 6 +- .../acceptance/js/api/project_updates.test.js | 4 +- .../test/acceptance/js/api/projects.test.js | 9 +- .../js/storage/backup_generator.test.mjs | 10 +- .../acceptance/js/storage/blob_hash.test.js | 15 - .../js/storage/support/test_files.js | 3 +- 41 files changed, 1666 insertions(+), 549 deletions(-) delete mode 100644 libraries/overleaf-editor-core/lib/blob_string_length.js create mode 100644 libraries/overleaf-editor-core/lib/blob_utils.js create mode 100644 libraries/overleaf-editor-core/lib/file_type_detector.js delete mode 100644 libraries/overleaf-editor-core/lib/origin/editor_origin.js delete mode 100644 libraries/overleaf-editor-core/test/unit/blob_string_length.test.js create mode 100644 libraries/overleaf-editor-core/test/unit/blob_utils.test.js delete mode 100644 libraries/overleaf-editor-core/test/unit/editor_origin.test.js create mode 100644 libraries/overleaf-editor-core/test/unit/file_type_detector.test.js create mode 100644 libraries/overleaf-editor-core/test/unit/origin.test.js delete mode 100644 services/history-v1/storage/lib/blob_hash.js delete mode 100644 services/history-v1/test/acceptance/js/storage/blob_hash.test.js diff --git a/libraries/overleaf-editor-core/index.js b/libraries/overleaf-editor-core/index.js index dce4aa2f00..e496547d2b 100644 --- a/libraries/overleaf-editor-core/index.js +++ b/libraries/overleaf-editor-core/index.js @@ -27,13 +27,13 @@ const RestoreOrigin = require('./lib/origin/restore_origin') const RestoreFileOrigin = require('./lib/origin/restore_file_origin') const Origin = require('./lib/origin') const { EDITOR_ORIGIN_KIND } = require('./lib/origin') -const EditorOrigin = require('./lib/origin/editor_origin') const OtClient = require('./lib/ot_client') const rebaseChanges = require('./lib/rebase') const { editorChangeIdentity, editorChangeIdentityOf, isSameEditorChange, + isChangeFrom, } = require('./lib/change_identity') const TextOperation = require('./lib/operation/text_operation') const EditOperation = require('./lib/operation/edit_operation') @@ -96,13 +96,13 @@ exports.Operation = Operation exports.RestoreOrigin = RestoreOrigin exports.RestoreFileOrigin = RestoreFileOrigin exports.Origin = Origin -exports.EditorOrigin = EditorOrigin exports.EDITOR_ORIGIN_KIND = EDITOR_ORIGIN_KIND exports.OtClient = OtClient exports.rebaseChanges = rebaseChanges exports.editorChangeIdentity = editorChangeIdentity exports.editorChangeIdentityOf = editorChangeIdentityOf exports.isSameEditorChange = isSameEditorChange +exports.isChangeFrom = isChangeFrom exports.TextOperation = TextOperation exports.EditOperation = EditOperation exports.safePathname = safePathname diff --git a/libraries/overleaf-editor-core/lib/blob.js b/libraries/overleaf-editor-core/lib/blob.js index 0226cf62c4..478cf7d58d 100644 --- a/libraries/overleaf-editor-core/lib/blob.js +++ b/libraries/overleaf-editor-core/lib/blob.js @@ -41,7 +41,7 @@ class Blob { /** * The hash of empty content, which is the same for every project. * - * git's hash of an empty blob, as `blobHash` computes it: it is a property of the + * git's hash of an empty blob, as `blobHashFromString` computes it: it is a property of the * content rather than of anything stored, so it can be recognised without looking * anything up. `File.EMPTY_FILE_HASH` is the same value under the name a file's * content goes by. diff --git a/libraries/overleaf-editor-core/lib/blob_string_length.js b/libraries/overleaf-editor-core/lib/blob_string_length.js deleted file mode 100644 index 513e191f73..0000000000 --- a/libraries/overleaf-editor-core/lib/blob_string_length.js +++ /dev/null @@ -1,54 +0,0 @@ -// @ts-check -'use strict' - -/** - * The rule that decides whether the content of a blob counts as editable text, - * and if so what string length to record on the {@link Blob}. - * - * This module is Node-only: it reads files and validates UTF-8 with a native - * addon. It must never be reachable from index.js, because overleaf-editor-core - * is bundled for the browser. Consumers deep import it, which the package - * exports allow: - * - * require('overleaf-editor-core/lib/blob_string_length') - */ - -const fs = require('node:fs') -const isValidUtf8 = require('utf-8-validate') - -const Blob = require('./blob') -const TextOperation = require('./operation/text_operation') -const { containsNonBmpChars } = require('./util') - -/** - * @param {Buffer} buffer - * @return {number | null} the string length, or null when the content is not - * editable text - */ -function getStringLengthOfBuffer(buffer) { - if (!isValidUtf8(buffer)) return null - const data = buffer.toString() - if (data.length > TextOperation.MAX_STRING_LENGTH) return null - // We cannot edit files containing non-BMP or null characters. - if (containsNonBmpChars(data)) return null - if (data.indexOf('\x00') !== -1) return null - return data.length -} - -/** - * @param {number} byteLength - * @param {string} pathname - * @return {Promise} the string length, or null when the content - * is not editable text - */ -async function getStringLengthOfFile(byteLength, pathname) { - // We have to read the file into memory to get its UTF-8 length, so don't - // bother for files that are too large for us to edit anyway. - if (byteLength > Blob.MAX_EDITABLE_BYTE_LENGTH_BOUND) { - return null - } - - return getStringLengthOfBuffer(await fs.promises.readFile(pathname)) -} - -module.exports = { getStringLengthOfBuffer, getStringLengthOfFile } diff --git a/libraries/overleaf-editor-core/lib/blob_utils.js b/libraries/overleaf-editor-core/lib/blob_utils.js new file mode 100644 index 0000000000..363fdd7ac7 --- /dev/null +++ b/libraries/overleaf-editor-core/lib/blob_utils.js @@ -0,0 +1,157 @@ +// @ts-check +'use strict' + +/** + * What history needs to know about a piece of content before it can store it: + * the hash it is addressed by, whether its bytes count as editable text, and, + * for content already written to disk, both at once as a {@link Blob}. + * + * This module is Node-only: it hashes with node:crypto, reads files and + * validates UTF-8 with a native addon. It must never be reachable from + * index.js, because overleaf-editor-core is bundled for the browser. Consumers + * deep import it, which the package exports allow: + * + * require('overleaf-editor-core/lib/blob_utils') + */ + +const crypto = require('node:crypto') +const fs = require('node:fs') +const { pipeline } = require('node:stream/promises') +const isValidUtf8 = require('utf-8-validate') + +const Blob = require('./blob') +const TextOperation = require('./operation/text_operation') +const { containsNonBmpChars } = require('./util') + +/** + * @param {number} byteLength + * @return {crypto.Hash} + */ +function createBlobHash(byteLength) { + if (!Number.isInteger(byteLength) || byteLength < 0) { + // Without this the header carries "blob NaN\0" and the digest is a + // plausible-looking hash of nothing in particular. + throw new TypeError(`blobHash: bad byteLength: ${byteLength}`) + } + const hash = crypto.createHash('sha1') + hash.setEncoding('hex') + hash.update('blob ' + byteLength + '\x00') + return hash +} + +/** + * The git blob hash: sha1("blob " + byteLength + "\0" + content). + * + * This is how history-v1 addresses blobs, and it is byte-for-byte git's own + * blob object hash, so the hash history stores for a file is also the sha that + * file has in a git repository. {@link File.EMPTY_FILE_HASH} is git's empty-blob + * hash for the same reason. + * + * @param {string} string + * @return {string} hexadecimal SHA-1 hash + */ +function blobHashFromString(string) { + // The header counts bytes, not characters. Hashing the string length instead + // makes every non-ASCII file disagree with git and with history. + const hash = createBlobHash(Buffer.byteLength(string)) + hash.update(string, 'utf8') + hash.end() + return /** @type {string} */ (hash.read()) +} + +/** + * @param {Buffer} buffer + * @return {string} hexadecimal SHA-1 hash + */ +function blobHashFromBuffer(buffer) { + const hash = createBlobHash(buffer.byteLength) + hash.update(buffer) + hash.end() + return /** @type {string} */ (hash.read()) +} + +/** + * @param {number} byteLength + * @param {NodeJS.ReadableStream} stream the interface, not the class: what a + * caller has is often whatever its storage handed it + * @return {Promise} hexadecimal SHA-1 hash + */ +async function blobHashFromStream(byteLength, stream) { + const hash = createBlobHash(byteLength) + // pipeline ends the hash stream, which flushes the digest to its readable + // side. Ending it again here would emit ERR_STREAM_ALREADY_FINISHED. + await pipeline(stream, hash) + return /** @type {string} */ (hash.read()) +} + +/** + * @param {string} pathname + * @return {Promise} hexadecimal SHA-1 hash + */ +async function blobHashFromFile(pathname) { + // A file can be tens of megabytes, so the content is streamed rather than + // read into memory. The header needs the byte length up front, hence the stat. + const { size } = await fs.promises.stat(pathname) + return await blobHashFromStream(size, fs.createReadStream(pathname)) +} + +/** + * The rule that decides whether the content of a blob counts as editable text, + * and if so what string length to record on the {@link Blob}. + * + * @param {Buffer} buffer + * @return {number | null} the string length, or null when the content is not + * editable text + */ +function getStringLengthOfBuffer(buffer) { + if (!isValidUtf8(buffer)) return null + const data = buffer.toString() + if (data.length > TextOperation.MAX_STRING_LENGTH) return null + // We cannot edit files containing non-BMP or null characters. + if (containsNonBmpChars(data)) return null + if (data.indexOf('\x00') !== -1) return null + return data.length +} + +/** + * @param {number} byteLength + * @param {string} pathname + * @return {Promise} the string length, or null when the content + * is not editable text + */ +async function getStringLengthOfFile(byteLength, pathname) { + // We have to read the file into memory to get its UTF-8 length, so don't + // bother for files that are too large for us to edit anyway. + if (byteLength > Blob.MAX_EDITABLE_BYTE_LENGTH_BOUND) { + return null + } + + return getStringLengthOfBuffer(await fs.promises.readFile(pathname)) +} + +/** + * The blob a local file amounts to: the hash its content is addressed by, its + * byte length, and the string length, left off when the content is not editable + * text. + * + * @param {string} pathname + * @return {Promise} + */ +async function blobForFile(pathname) { + const { size: byteLength } = await fs.promises.stat(pathname) + const [hash, stringLength] = await Promise.all([ + blobHashFromStream(byteLength, fs.createReadStream(pathname)), + getStringLengthOfFile(byteLength, pathname), + ]) + return new Blob(hash, byteLength, stringLength ?? undefined) +} + +module.exports = { + blobHashFromString, + blobHashFromBuffer, + blobHashFromStream, + blobHashFromFile, + getStringLengthOfBuffer, + getStringLengthOfFile, + blobForFile, +} diff --git a/libraries/overleaf-editor-core/lib/build_set_content_operations.js b/libraries/overleaf-editor-core/lib/build_set_content_operations.js index d3d7ac31d6..83659648bd 100644 --- a/libraries/overleaf-editor-core/lib/build_set_content_operations.js +++ b/libraries/overleaf-editor-core/lib/build_set_content_operations.js @@ -11,7 +11,7 @@ const { diffAsTextOperation } = require('./diff_as_text_operation') /** * @import Blob from './blob' * @import StringFileData from './file_data/string_file_data' - * @import { BlobStore, RangesBlob } from './types' + * @import { FileMetadata, ReadWriteBlobStore, RangesBlob } from './types' */ /** @@ -29,11 +29,12 @@ const { diffAsTextOperation } = require('./diff_as_text_operation') * @param {string} args.pathname * @param {string} [args.content] new doc content * @param {Blob} [args.blob] blob with the new file content - * @param {Object} [args.metadata] metadata for the file; the file's metadata - * is always replaced with this value (defaults to empty metadata) + * @param {FileMetadata} [args.metadata] metadata for the file; the file's + * metadata is always replaced with this value (defaults to empty + * metadata) * @param {{userId: string, ts: Date}} [args.tracking] record the change as * tracked changes (ignored when replacing with a blob) - * @param {BlobStore} args.blobStore + * @param {ReadWriteBlobStore} args.blobStore * @return {Promise<{operations: Operation[], status: 'applied' | 'noop'}>} */ async function buildSetContentOperations({ diff --git a/libraries/overleaf-editor-core/lib/change_identity.js b/libraries/overleaf-editor-core/lib/change_identity.js index 42ed6f65cf..74888bf759 100644 --- a/libraries/overleaf-editor-core/lib/change_identity.js +++ b/libraries/overleaf-editor-core/lib/change_identity.js @@ -27,22 +27,22 @@ const { EDITOR_ORIGIN_KIND } = require('./origin') * * - the origin kind keeps other writers out, so a Dropbox sync or a resync is * never mistaken for a client's change. - * - `editorId` separates editors that share an author. Nothing coordinates + * - `historyClientId` separates editors that share an author. Nothing coordinates * timestamps between two tabs of one user, or between anonymous editors, who * share an empty author — without it, two clients colliding on a millisecond * look identical. - * - the author makes a forged `editorId` unusable: it arrives from the client, but - * a change belonging to another account can never match. + * - the author makes a forged `historyClientId` unusable: it arrives from the + * client, but a change belonging to another account can never match. * - the timestamp separates successive changes from one editor, which the client * keeps distinct and stable across resends. * - * @typedef {{editorId: string, author: (string|null), timestamp: number}} EditorChangeIdentity + * @typedef {{historyClientId: string, author: (string|null), timestamp: number}} EditorChangeIdentity */ /** * The identity of a raw change, or null if it is not an identifiable editor - * change — another writer's, or one whose `editorId` was dropped when its chunk - * was written. + * change — another writer's, or one whose `historyClientId` was dropped when its + * chunk was written. * * The timestamp is compared as a point in time rather than as a string, so that * two spellings of the same instant cannot read as different changes. @@ -54,10 +54,11 @@ function editorChangeIdentity(raw) { const origin = raw?.origin if (!origin || origin.kind !== EDITOR_ORIGIN_KIND) return null - // The origin union's catch-all variant is `{kind: string}`, which also admits - // this kind, so checking the kind cannot narrow to the variant carrying the id. - const { editorId } = /** @type {{editorId?: string}} */ (origin) - if (!editorId) return null + // Absent on a change nothing has to recognise again, and on one whose id was + // dropped when its chunk was written. Neither is identifiable, and an absent id + // must never read as a match. + const { historyClientId } = origin + if (!historyClientId) return null // real-time stamps exactly one author on every change it forwards, so anything // else did not come from this path. @@ -67,21 +68,25 @@ function editorChangeIdentity(raw) { const timestamp = new Date(raw.timestamp).getTime() if (Number.isNaN(timestamp)) return null - return { editorId, author: authors[0] ?? null, timestamp } + return { historyClientId, author: authors[0] ?? null, timestamp } } /** * Build an identity from a client's own record of a change it submitted, for * comparing against what comes back from history. * - * @param {Object} params - * @param {string} params.editorId - * @param {string | null} params.author - * @param {Date} params.timestamp + * The timestamp is still a Date here: the client holds the change it submitted, + * and only the comparison works in milliseconds. + * + * @param {{historyClientId: string, author: string | null, timestamp: Date}} params * @return {EditorChangeIdentity} */ -function editorChangeIdentityOf({ editorId, author, timestamp }) { - return { editorId, author: author ?? null, timestamp: timestamp.getTime() } +function editorChangeIdentityOf({ historyClientId, author, timestamp }) { + return { + historyClientId, + author: author ?? null, + timestamp: timestamp.getTime(), + } } /** @@ -95,14 +100,42 @@ function editorChangeIdentityOf({ editorId, author, timestamp }) { function isSameEditorChange(a, b) { if (!a || !b) return false return ( - a.editorId === b.editorId && + a.historyClientId === b.historyClientId && a.author === b.author && a.timestamp === b.timestamp ) } +/** + * Whether a raw change is one a given writer placed. + * + * The counterpart of the above for writers that are not an editor client: an + * integration that commits a batch on a user's behalf and then has to recognise + * that batch coming back, because a lost response does not say whether the + * commit landed and resubmitting one that did applies its removes twice. + * + * Compares the same three fields, and for the same reason -- a rebase rewrites + * a change's operations but leaves its origin, authors and timestamp alone -- + * with the timestamp compared as a point in time rather than as a string, so + * that two spellings of the same instant cannot read as different changes. + * + * @param {RawChange} [raw] + * @param {{originKind: string, author: string, timestamp: Date}} [expected] + * @return {boolean} + */ +function isChangeFrom(raw, expected) { + if (!raw || !expected) return false + if (raw.origin?.kind !== expected.originKind) return false + if (!Array.isArray(raw.v2Authors)) return false + if (!raw.v2Authors.includes(expected.author)) return false + const timestamp = new Date(raw.timestamp).getTime() + if (Number.isNaN(timestamp)) return false + return timestamp === expected.timestamp.getTime() +} + module.exports = { editorChangeIdentity, editorChangeIdentityOf, isSameEditorChange, + isChangeFrom, } diff --git a/libraries/overleaf-editor-core/lib/file.js b/libraries/overleaf-editor-core/lib/file.js index c24990c8da..52ded36dc0 100644 --- a/libraries/overleaf-editor-core/lib/file.js +++ b/libraries/overleaf-editor-core/lib/file.js @@ -270,6 +270,26 @@ class File { return this } + /** + * Load the content of an editable file, answering with the data holding it. + * + * Only an editable file has content: any other kind is bytes addressed by + * hash, and hollow data records the size of content it does not have. Both + * answer {@link File#getContent} with null, which this rules out for its + * caller -- what it answers with is the data that always has the content. + * + * @param {ReadonlyBlobStore} blobStore + * @return {Promise} the loaded data of this file + * @throws {NotEditableError} where the file is not an editable doc + */ + async loadEager(blobStore) { + await this.load('eager', blobStore) + if (!(this.data instanceof StringFileData)) { + throw new File.NotEditableError() + } + return this.data + } + /** * Store the file's content in the blob store and return a raw file with * the corresponding hash. As a side effect, make this object consistent with diff --git a/libraries/overleaf-editor-core/lib/file_type_detector.js b/libraries/overleaf-editor-core/lib/file_type_detector.js new file mode 100644 index 0000000000..8ef37b573b --- /dev/null +++ b/libraries/overleaf-editor-core/lib/file_type_detector.js @@ -0,0 +1,205 @@ +// @ts-check +'use strict' + +/** + * Whether a file is an editable doc or a binary file, at a given pathname. + * + * The rules mirror web's Uploads/FileTypeManager and the type decision in web's + * ThirdPartyDataStore/UpdateMerger, which together classify a file that travels + * through web's update endpoints. Every integration that writes to history + * directly has to reach the same verdict for the same bytes at the same + * pathname, otherwise a project changes the type of a file just by crossing the + * otMigrationStage boundary. + * + * This is a different question from the one lib/blob_utils.js answers. + * That one says whether history *can* hold content as text: it caps at + * TextOperation.MAX_STRING_LENGTH (3 MiB) and looks for high surrogates only. + * This one says whether a file *should be* a doc: it caps at web's + * max_doc_length (2 MiB) and rejects either half of a surrogate pair. That is + * why the limit and the lists are injected rather than taken from editor-core's + * own constants — they are web's rule, and editor-core has no + * @overleaf/settings dependency and must not gain one. + * + * This module is Node-only: it reads files and validates UTF-8 with a native + * addon. It must never be reachable from index.js, because overleaf-editor-core + * is bundled for the browser. Consumers deep import it, which the package + * exports allow: + * + * require('overleaf-editor-core/lib/file_type_detector') + */ + +const fs = require('node:fs') +const Path = require('node:path') +const isValidUtf8 = require('utf-8-validate') + +/** + * @typedef {{kind: 'text', content: string} | {kind: 'binary'}} DetectedType + * @typedef {'doc'|'file'|null|undefined} ExistingType + */ + +/** + * @typedef {object} FileTypeConfig + * @property {ReadonlyArray} textExtensions extensions without a leading + * dot, as web's settings.textExtensions carries them + * @property {ReadonlyArray} editableFilenames whole file names, lower + * case + * @property {number} maxDocLength web's max_doc_length, in characters + */ + +/** + * @param {FileTypeConfig} config + */ +function validateConfig(config) { + if ( + !config || + !Array.isArray(config.textExtensions) || + !Array.isArray(config.editableFilenames) || + !Number.isFinite(config.maxDocLength) + ) { + // A missing maxDocLength would make every length comparison below false, + // which reads as "no limit" rather than as a misconfiguration. + throw new TypeError('fileTypeDetector: bad config') + } +} + +/** + * @param {string} pathname + * @param {FileTypeConfig} config + * @return {boolean} whether the name alone allows the file to be a doc + */ +function isTextFilename(pathname, config) { + const basename = Path.basename(pathname).toLowerCase() + const extension = Path.extname(pathname).toLowerCase() + const extensions = new Set(config.textExtensions.map(ext => `.${ext}`)) + return ( + extensions.has(extension) || config.editableFilenames.includes(basename) + ) +} + +/** + * Whether the content has to be read before the file can be classified. + * + * `false` is a verdict, not a maybe: every case that gets here is a binary file, + * decided from the name and the size alone. + * + * @param {string} pathname target pathname, project-relative + * @param {number|null} byteLength null when the size is not known yet, which + * skips the size gate + * @param {ExistingType} existingType type of what is at the pathname now, if + * anything + * @param {FileTypeConfig} config + * @return {boolean} + */ +function needsContent(pathname, byteLength, existingType, config) { + validateConfig(config) + + // A binary file stays a binary file whatever the update looks like: no update + // to an existing file comes out of web's UpdateMerger._determineFileType as + // anything but a file. + if (existingType === 'file') { + return false + } + + // A file that is already a doc stays eligible to be one whatever its name + // says, which is how web keeps a doc a doc after a rename to a name that is + // not in the text-extension list. + if (existingType !== 'doc' && !isTextFilename(pathname, config)) { + return false + } + + // A character takes at most three bytes in the only encoding that can yield an + // editable doc, so anything above this cannot come in under the character + // limit below. + if (byteLength !== null && byteLength > 3 * config.maxDocLength) { + return false + } + + return true +} + +/** + * Whether a decoded string can be stored as a doc. + * + * @param {string} content + * @param {FileTypeConfig} config + * @return {boolean} + */ +function isEditableString(content, config) { + if (content.length >= config.maxDocLength) { + return false + } + if (content.includes('\x00')) { + return false + } + // High and low surrogate code units, i.e. non-BMP characters. Only paired ones + // can reach here from valid UTF-8: a lone surrogate is not valid UTF-8, so + // those bytes have already been turned away. Kept whole because web applies + // the same rule to content it decoded as UTF-16, where a lone one can occur. + if (/[\uD800-\uDFFF]/.test(content)) { + return false + } + return true +} + +/** + * Classify content already held in memory. + * + * A `kind: 'text'` verdict promises the bytes are valid UTF-8, so the content + * can be handed back to where it came from as the bytes that arrived. + * + * @param {Buffer} buffer + * @param {{pathname: string, existingType?: ExistingType}} target + * @param {FileTypeConfig} config + * @return {DetectedType} + */ +function detectBuffer(buffer, { pathname, existingType }, config) { + if (!needsContent(pathname, buffer.byteLength, existingType, config)) { + return { kind: 'binary' } + } + + // Anything that is not UTF-8 cannot be kept in sync, so web treats it as + // binary regardless of what it decodes to (UpdateMerger.mjs, isBinary). The + // verdict is acted on here rather than returned alongside the decoded content: + // carrying it for a caller to honour is what let a latin1 file become a doc + // with its bytes re-serialised as UTF-8. + if (!isValidUtf8(buffer)) { + return { kind: 'binary' } + } + + const content = buffer.toString('utf-8') + if (!isEditableString(content, config)) { + return { kind: 'binary' } + } + return { kind: 'text', content } +} + +/** + * Classify the file spooled at `localPath` as it would be classified at + * `pathname`. + * + * @param {string} pathname target pathname, project-relative + * @param {string} localPath spooled copy of the file's content + * @param {ExistingType} existingType type of what is at the pathname now, if + * anything + * @param {FileTypeConfig} config + * @return {Promise} + */ +async function detectFile(pathname, localPath, existingType, config) { + // The name can decide on its own, before the file is even stat'ed. + if (!needsContent(pathname, null, existingType, config)) { + return { kind: 'binary' } + } + + const stat = await fs.promises.stat(localPath) + if (!needsContent(pathname, stat.size, existingType, config)) { + return { kind: 'binary' } + } + + return detectBuffer( + await fs.promises.readFile(localPath), + { pathname, existingType }, + config + ) +} + +module.exports = { needsContent, detectBuffer, detectFile } diff --git a/libraries/overleaf-editor-core/lib/operation/text_operation.js b/libraries/overleaf-editor-core/lib/operation/text_operation.js index 76a738c428..b0fce6f842 100644 --- a/libraries/overleaf-editor-core/lib/operation/text_operation.js +++ b/libraries/overleaf-editor-core/lib/operation/text_operation.js @@ -501,7 +501,7 @@ class TextOperation extends EditOperation { } const operation1 = this if (operation1.targetLength !== operation2.baseLength) { - throw new Error( + throw new UnprocessableError( 'The base length of the second operation has to be the ' + 'target length of the first operation' ) @@ -537,12 +537,12 @@ class TextOperation extends EditOperation { } if (typeof op1 === 'undefined') { - throw new Error( + throw new UnprocessableError( 'Cannot compose operations: first operation is too short.' ) } if (typeof op2 === 'undefined') { - throw new Error( + throw new UnprocessableError( 'Cannot compose operations: first operation is too long.' ) } @@ -651,7 +651,9 @@ class TextOperation extends EditOperation { */ static transform(operation1, operation2) { if (operation1.baseLength !== operation2.baseLength) { - throw new Error('Both operations have to have the same base length') + throw new UnprocessableError( + 'Both operations have to have the same base length' + ) } const operation1prime = new TextOperation() @@ -695,12 +697,12 @@ class TextOperation extends EditOperation { } if (typeof op1 === 'undefined') { - throw new Error( + throw new UnprocessableError( 'Cannot compose operations: first operation is too short.' ) } if (typeof op2 === 'undefined') { - throw new Error( + throw new UnprocessableError( 'Cannot compose operations: first operation is too long.' ) } @@ -781,7 +783,7 @@ class TextOperation extends EditOperation { } operation2prime.remove(minl) } else { - throw new Error("The two operations aren't compatible") + throw new UnprocessableError("The two operations aren't compatible") } } diff --git a/libraries/overleaf-editor-core/lib/origin/editor_origin.js b/libraries/overleaf-editor-core/lib/origin/editor_origin.js deleted file mode 100644 index d8bdcfbff4..0000000000 --- a/libraries/overleaf-editor-core/lib/origin/editor_origin.js +++ /dev/null @@ -1,55 +0,0 @@ -'use strict' - -const assert = require('check-types').assert - -const Origin = require('.') - -/** - * An Origin for a change an editor client submitted over the `applyHistoryOt` - * rpc. - * - * The `editorId` identifies the editor instance the change came from. It is what - * scopes the change's timestamp: a client keeps its timestamps distinct, but two - * clients of the same user do not coordinate, so the pair is what identifies a - * submission. history-v1 uses it to recognise a change it has already applied, - * rather than applying a resend twice. - * - * It is only needed while a change can still be resent, so it is dropped from - * all but the latest change of an editor when a chunk is written to storage. The - * plain {@link Origin} with this kind is what remains, and it keeps recording - * that the change came from the editor. - */ -class EditorOrigin extends Origin { - /** - * @param {string} editorId a UUID identifying the editor instance - */ - constructor(editorId) { - assert.nonEmptyString(editorId, 'EditorOrigin: bad editorId') - - super(EditorOrigin.KIND) - this.editorId = editorId - } - - static fromRaw(raw) { - return new EditorOrigin(raw.editorId) - } - - /** @inheritdoc */ - toRaw() { - return { - kind: EditorOrigin.KIND, - editorId: this.editorId, - } - } - - /** - * @return {string} - */ - getEditorId() { - return this.editorId - } -} - -EditorOrigin.KIND = Origin.EDITOR_ORIGIN_KIND - -module.exports = EditorOrigin diff --git a/libraries/overleaf-editor-core/lib/origin/index.js b/libraries/overleaf-editor-core/lib/origin/index.js index 878a3d4ce2..8700408bb3 100644 --- a/libraries/overleaf-editor-core/lib/origin/index.js +++ b/libraries/overleaf-editor-core/lib/origin/index.js @@ -1,13 +1,23 @@ +// @ts-check + 'use strict' const assert = require('check-types').assert +/** + * @import { RawBaseOrigin, RawOrigin, RawRestoreOrigin } from '../types' + * @import { RawRestoreFileOrigin, RawRestoreProjectOrigin } from '../types' + */ + // Dependencies are loaded at the bottom of the file to mitigate circular -// dependency -let RestoreOrigin = null -let RestoreFileOrigin = null -let RestoreProjectOrigin = null -let EditorOrigin = null +// dependency. Typed here, or they would be `any` and nothing they are handed +// would be checked. +/** @type {typeof import('./restore_origin')} */ +let RestoreOrigin +/** @type {typeof import('./restore_file_origin')} */ +let RestoreFileOrigin +/** @type {typeof import('./restore_project_origin')} */ +let RestoreProjectOrigin /** * The origin kind of a change the editor submitted over the `applyHistoryOt` @@ -25,41 +35,58 @@ const EDITOR_ORIGIN_KIND = 'editor' class Origin { /** * @param {string} kind + * @param {string} [historyClientId] identifies the client that submitted the + * change, where it is one that has to be recognised again; see + * {@link Origin#getHistoryClientId} */ - constructor(kind) { + constructor(kind, historyClientId) { assert.string(kind, 'Origin: bad kind') + assert.maybe.nonEmptyString(historyClientId, 'Origin: bad historyClientId') this.kind = kind + this.historyClientId = historyClientId } /** * Create an Origin from its raw form. * - * @param {Object} [raw] + * The kind tells the variants apart, but only up to a point: the union's last + * variant is a kind on its own, and its `kind` is any string, so it admits the + * kinds below as well. Checking the kind rules out the other restore variants + * and leaves that one, hence the assertion at each branch. + * + * @param {RawOrigin} [raw] * @return {Origin | null} */ static fromRaw(raw) { if (!raw) return null - if (raw.kind === RestoreOrigin.KIND) return RestoreOrigin.fromRaw(raw) - if (raw.kind === RestoreFileOrigin.KIND) + if (raw.kind === RestoreOrigin.KIND && 'version' in raw) { + return RestoreOrigin.fromRaw(raw) + } + if (raw.kind === RestoreFileOrigin.KIND && 'path' in raw) { return RestoreFileOrigin.fromRaw(raw) - if (raw.kind === RestoreProjectOrigin.KIND) + } + if (raw.kind === RestoreProjectOrigin.KIND && 'version' in raw) { return RestoreProjectOrigin.fromRaw(raw) - // Only an editor change that still carries its editorId is an EditorOrigin. - // The id is dropped from all but an editor's latest change when a chunk is - // written, so most editor changes read back from storage are plain origins. - if (raw.kind === EditorOrigin.KIND && raw.editorId) - return EditorOrigin.fromRaw(raw) - return new Origin(raw.kind) + } + return new Origin(raw.kind, raw.historyClientId) } /** * Convert the Origin to raw form for storage or transmission. * - * @return {Object} + * Built up a layer at a time: this one writes what every origin has, and a + * subclass spreads it and adds the fields it owns. + * + * @return {RawBaseOrigin} */ toRaw() { - return { kind: this.kind } + /** @type {RawBaseOrigin} */ + const raw = { kind: this.kind } + if (this.historyClientId) { + raw.historyClientId = this.historyClientId + } + return raw } /** @@ -68,6 +95,36 @@ class Origin { getKind() { return this.kind } + + /** + * The client that submitted this change, where there is one. + * + * It is what scopes the change's timestamp: a client keeps its own timestamps + * distinct, but two clients of the same user do not coordinate, so the pair is + * what identifies a submission. history-v1 uses it to recognise a change it has + * already applied, rather than applying a resend twice, and the client that + * sent it uses it to recognise its own change coming back. + * + * Absent on a change nothing has to recognise again, and on one whose id was + * dropped once it could no longer be resent -- see + * {@link Origin#dropHistoryClientId}. + * + * @return {string | undefined} + */ + getHistoryClientId() { + return this.historyClientId + } + + /** + * Forget which client submitted this change, keeping the rest of the origin. + * + * The id is only worth anything while the change could still be resent, so the + * chunk store drops it from all but the latest change of each client when a + * chunk is written. + */ + dropHistoryClientId() { + this.historyClientId = undefined + } } module.exports = Origin @@ -76,4 +133,3 @@ module.exports.EDITOR_ORIGIN_KIND = EDITOR_ORIGIN_KIND RestoreOrigin = require('./restore_origin') RestoreFileOrigin = require('./restore_file_origin') RestoreProjectOrigin = require('./restore_project_origin') -EditorOrigin = require('./editor_origin') diff --git a/libraries/overleaf-editor-core/lib/origin/restore_file_origin.js b/libraries/overleaf-editor-core/lib/origin/restore_file_origin.js index 6e42467b9c..2a1156820c 100644 --- a/libraries/overleaf-editor-core/lib/origin/restore_file_origin.js +++ b/libraries/overleaf-editor-core/lib/origin/restore_file_origin.js @@ -1,34 +1,50 @@ +// @ts-check + 'use strict' const assert = require('check-types').assert const Origin = require('.') +/** + * @import { RawRestoreFileOrigin } from '../types' + */ + class RestoreFileOrigin extends Origin { /** * @param {number} version that was restored * @param {string} path that was restored * @param {Date} timestamp from the restored version + * @param {string} [historyClientId] see {@link Origin} */ - constructor(version, path, timestamp) { + constructor(version, path, timestamp, historyClientId) { assert.integer(version, 'RestoreFileOrigin: bad version') assert.string(path, 'RestoreFileOrigin: bad path') assert.date(timestamp, 'RestoreFileOrigin: bad timestamp') - super(RestoreFileOrigin.KIND) + super(RestoreFileOrigin.KIND, historyClientId) this.version = version this.path = path this.timestamp = timestamp } + /** + * @param {RawRestoreFileOrigin} raw + * @return {RestoreFileOrigin} + */ static fromRaw(raw) { - return new RestoreFileOrigin(raw.version, raw.path, new Date(raw.timestamp)) + return new RestoreFileOrigin( + raw.version, + raw.path, + new Date(raw.timestamp), + raw.historyClientId + ) } /** @inheritdoc */ toRaw() { return { - kind: RestoreFileOrigin.KIND, + ...super.toRaw(), version: this.version, path: this.path, timestamp: this.timestamp.toISOString(), @@ -57,6 +73,7 @@ class RestoreFileOrigin extends Origin { } } +/** @type {'file-restore'} */ RestoreFileOrigin.KIND = 'file-restore' module.exports = RestoreFileOrigin diff --git a/libraries/overleaf-editor-core/lib/origin/restore_origin.js b/libraries/overleaf-editor-core/lib/origin/restore_origin.js index 15bb4258f6..6f08bec110 100644 --- a/libraries/overleaf-editor-core/lib/origin/restore_origin.js +++ b/libraries/overleaf-editor-core/lib/origin/restore_origin.js @@ -1,9 +1,15 @@ +// @ts-check + 'use strict' const assert = require('check-types').assert const Origin = require('./') +/** + * @import { RawRestoreOrigin } from '../types' + */ + /** * When a {@link Change} is generated by restoring a previous version, this * records the original version. We also store the timestamp of the restored @@ -19,24 +25,33 @@ class RestoreOrigin extends Origin { /** * @param {number} version that was restored * @param {Date} timestamp from the restored version + * @param {string} [historyClientId] see {@link Origin} */ - constructor(version, timestamp) { + constructor(version, timestamp, historyClientId) { assert.integer(version, 'RestoreOrigin: bad version') assert.date(timestamp, 'RestoreOrigin: bad timestamp') - super(RestoreOrigin.KIND) + super(RestoreOrigin.KIND, historyClientId) this.version = version this.timestamp = timestamp } + /** + * @param {RawRestoreOrigin} raw + * @return {RestoreOrigin} + */ static fromRaw(raw) { - return new RestoreOrigin(raw.version, new Date(raw.timestamp)) + return new RestoreOrigin( + raw.version, + new Date(raw.timestamp), + raw.historyClientId + ) } /** @inheritdoc */ toRaw() { return { - kind: RestoreOrigin.KIND, + ...super.toRaw(), version: this.version, timestamp: this.timestamp.toISOString(), } @@ -57,6 +72,7 @@ class RestoreOrigin extends Origin { } } +/** @type {'restore'} */ RestoreOrigin.KIND = 'restore' module.exports = RestoreOrigin diff --git a/libraries/overleaf-editor-core/lib/origin/restore_project_origin.js b/libraries/overleaf-editor-core/lib/origin/restore_project_origin.js index 9db14b1eb7..aff34f1747 100644 --- a/libraries/overleaf-editor-core/lib/origin/restore_project_origin.js +++ b/libraries/overleaf-editor-core/lib/origin/restore_project_origin.js @@ -1,31 +1,46 @@ +// @ts-check + 'use strict' const assert = require('check-types').assert const Origin = require('.') +/** + * @import { RawRestoreProjectOrigin } from '../types' + */ + class RestoreProjectOrigin extends Origin { /** * @param {number} version that was restored * @param {Date} timestamp from the restored version + * @param {string} [historyClientId] see {@link Origin} */ - constructor(version, timestamp) { + constructor(version, timestamp, historyClientId) { assert.integer(version, 'RestoreProjectOrigin: bad version') assert.date(timestamp, 'RestoreProjectOrigin: bad timestamp') - super(RestoreProjectOrigin.KIND) + super(RestoreProjectOrigin.KIND, historyClientId) this.version = version this.timestamp = timestamp } + /** + * @param {RawRestoreProjectOrigin} raw + * @return {RestoreProjectOrigin} + */ static fromRaw(raw) { - return new RestoreProjectOrigin(raw.version, new Date(raw.timestamp)) + return new RestoreProjectOrigin( + raw.version, + new Date(raw.timestamp), + raw.historyClientId + ) } /** @inheritdoc */ toRaw() { return { - kind: RestoreProjectOrigin.KIND, + ...super.toRaw(), version: this.version, timestamp: this.timestamp.toISOString(), } @@ -46,6 +61,7 @@ class RestoreProjectOrigin extends Origin { } } +/** @type {'project-restore'} */ RestoreProjectOrigin.KIND = 'project-restore' module.exports = RestoreProjectOrigin diff --git a/libraries/overleaf-editor-core/lib/schemas.js b/libraries/overleaf-editor-core/lib/schemas.js index cb19c7507a..cefd921056 100644 --- a/libraries/overleaf-editor-core/lib/schemas.js +++ b/libraries/overleaf-editor-core/lib/schemas.js @@ -241,34 +241,63 @@ const rawSnapshot = z.strictObject({ }) // mirrors Origin.fromRaw: three restore variants plus a generic kind +// +// Every variant may carry the id of the client that submitted the change, which +// is what lets history recognise a resend and the client recognise its own change +// coming back. It is optional throughout: most changes are ones nothing has to +// recognise again, and the id is dropped from the rest once they can no longer be +// resent, so a bare {kind} is the common shape. +const historyClientId = z.uuid().optional() + +// The kinds that have a shape of their own, which the variants below describe. A +// payload claiming one of them has to match that shape rather than falling through +// to the catch-all, where it would validate as a kind and nothing else and then be +// read back as a change that has lost what the kind promised. +const namedOriginKinds = ['restore', 'file-restore', 'project-restore'] + +// The fields every origin has, which is also what an origin with nothing but a kind +// serialises to -- Origin's own raw form, and the union's catch-all below. +const rawBaseOrigin = z.strictObject({ + kind: z.string().refine(kind => !namedOriginKinds.includes(kind), { + message: 'origin kind has a shape of its own', + }), + historyClientId, +}) + +const rawRestoreOrigin = z.strictObject({ + kind: z.literal('restore'), + version: z.number().int(), + timestamp: z.iso.datetime(), + historyClientId, +}) + +const rawRestoreFileOrigin = z.strictObject({ + kind: z.literal('file-restore'), + version: z.number().int(), + path: z.string(), + timestamp: z.iso.datetime(), + historyClientId, +}) + +const rawRestoreProjectOrigin = z.strictObject({ + kind: z.literal('project-restore'), + version: z.number().int(), + timestamp: z.iso.datetime(), + historyClientId, +}) + +// The named kinds discriminate on `kind`, so a payload claiming one of them is +// checked against that shape alone and the error says which field it got wrong. The +// catch-all cannot join them -- its kind is any string, which zod rejects as a +// discriminator -- so it sits alongside, and its refinement above is what stops it +// swallowing a named 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(), - }), - // An editor change that still carries the id of the editor instance it came - // from. Once a chunk is written the id is dropped from all but that editor's - // latest change, leaving a bare {kind: 'editor'} that the catch-all below - // accepts. - z.strictObject({ - kind: z.literal('editor'), - editorId: z.uuid(), - }), - z.strictObject({ - kind: z.string(), - }), + z.discriminatedUnion('kind', [ + rawRestoreOrigin, + rawRestoreFileOrigin, + rawRestoreProjectOrigin, + ]), + rawBaseOrigin, ]) // mirrors Operation.fromRaw: add file / edit file / move (or delete) file / @@ -347,6 +376,10 @@ module.exports = { rawFileMap, rawV2DocVersions, rawSnapshot, + rawBaseOrigin, + rawRestoreOrigin, + rawRestoreFileOrigin, + rawRestoreProjectOrigin, rawOrigin, rawAddFileOperation, rawMoveFileOperation, diff --git a/libraries/overleaf-editor-core/lib/text_file_defaults.js b/libraries/overleaf-editor-core/lib/text_file_defaults.js index ac0501c028..0225559ac4 100644 --- a/libraries/overleaf-editor-core/lib/text_file_defaults.js +++ b/libraries/overleaf-editor-core/lib/text_file_defaults.js @@ -4,12 +4,13 @@ /** * The file names that may hold editable text, as data. * - * These lists decide whether a file becomes a doc or a binary file, so every - * service that classifies a file has to work from the same list: one carrying a - * shorter copy turns the same bytes into a binary file, and a project changes - * the type of a file just by taking a different path into history. Consumers - * read them from here instead of carrying a copy. web layers the - * ADDITIONAL_TEXT_EXTENSIONS override on top for Server CE/Pro. + * These lists decide whether a file becomes a doc or a binary file (see + * lib/file_type_detector.js), so every service that classifies a file has to + * work from the same list: one carrying a shorter copy turns the same bytes into + * a binary file, and a project changes the type of a file just by taking a + * different path into history. Consumers read them from here instead of carrying + * a copy. web layers the ADDITIONAL_TEXT_EXTENSIONS override on top for Server + * CE/Pro. * * Frozen because they are shared: copy them (`.concat(...)`, `.slice()`) before * handing them to anything that merges configuration into its own objects. diff --git a/libraries/overleaf-editor-core/lib/types.ts b/libraries/overleaf-editor-core/lib/types.ts index db35999099..423b88c18c 100644 --- a/libraries/overleaf-editor-core/lib/types.ts +++ b/libraries/overleaf-editor-core/lib/types.ts @@ -14,6 +14,12 @@ export type BlobStore = { export type ReadonlyBlobStore = Pick +/** + * Everything but looking a blob up by its hash: reading content and storing + * content, which is all it takes to build operations over content. + */ +export type ReadWriteBlobStore = Omit + export type RangesBlob = { comments: CommentRawData[] trackedChanges: TrackedChangeRawData[] @@ -35,6 +41,16 @@ export type TrackingDirective = TrackingProps | ClearTrackingProps export type StringFileRawData = z.infer +export type RawBaseOrigin = z.infer + +export type RawRestoreOrigin = z.infer + +export type RawRestoreFileOrigin = z.infer + +export type RawRestoreProjectOrigin = z.infer< + typeof schemas.rawRestoreProjectOrigin +> + export type RawOrigin = z.infer export type RawChange = z.infer diff --git a/libraries/overleaf-editor-core/test/unit/blob_string_length.test.js b/libraries/overleaf-editor-core/test/unit/blob_string_length.test.js deleted file mode 100644 index 2f4d5d14f8..0000000000 --- a/libraries/overleaf-editor-core/test/unit/blob_string_length.test.js +++ /dev/null @@ -1,104 +0,0 @@ -// @ts-check -'use strict' - -const { expect } = require('chai') -const fs = require('node:fs') -const os = require('node:os') -const path = require('node:path') - -const { - getStringLengthOfBuffer, - getStringLengthOfFile, -} = require('../../lib/blob_string_length') -const Blob = require('../../lib/blob') -const TextOperation = require('../../lib/operation/text_operation') - -describe('blob string length', function () { - /** @type {string} */ - let tmpDir - - beforeEach(async function () { - tmpDir = await fs.promises.mkdtemp( - path.join(os.tmpdir(), 'blob-string-length-') - ) - }) - - afterEach(async function () { - await fs.promises.rm(tmpDir, { recursive: true, force: true }) - }) - - /** - * @param {Buffer} content - * @return {Promise} - */ - async function writeFile(content) { - const pathname = path.join(tmpDir, 'blob') - await fs.promises.writeFile(pathname, content) - return pathname - } - - describe('getStringLengthOfBuffer', function () { - it('counts the characters of plain ASCII', function () { - expect(getStringLengthOfBuffer(Buffer.from('hello'))).to.equal(5) - }) - - it('counts characters rather than bytes for multi-byte UTF-8', function () { - // 'é' is two bytes, 'ᚠ' is three. - const buffer = Buffer.from('héllᚠ') - expect(buffer.byteLength).to.equal(8) - expect(getStringLengthOfBuffer(buffer)).to.equal(5) - }) - - it('rejects bytes that are not valid UTF-8', function () { - // 0x80 is a continuation byte with nothing to continue. - expect(getStringLengthOfBuffer(Buffer.from([0x61, 0x80, 0x62]))).to.be - .null - }) - - it('rejects a NUL character', function () { - expect(getStringLengthOfBuffer(Buffer.from('a\0b'))).to.be.null - }) - - it('rejects non-BMP characters', function () { - expect(getStringLengthOfBuffer(Buffer.from('a 🙂 face'))).to.be.null - }) - - it('rejects content longer than a text operation can hold', function () { - const content = 'a'.repeat(TextOperation.MAX_STRING_LENGTH + 1) - expect(getStringLengthOfBuffer(Buffer.from(content))).to.be.null - }) - - it('accepts content at the maximum length', function () { - const content = 'a'.repeat(TextOperation.MAX_STRING_LENGTH) - expect(getStringLengthOfBuffer(Buffer.from(content))).to.equal( - TextOperation.MAX_STRING_LENGTH - ) - }) - }) - - describe('getStringLengthOfFile', function () { - it('counts the characters of an editable file', async function () { - const content = Buffer.from('héllᚠ') - const pathname = await writeFile(content) - expect( - await getStringLengthOfFile(content.byteLength, pathname) - ).to.equal(5) - }) - - it('rejects a file that is not editable text', async function () { - const content = Buffer.from([0x61, 0x00, 0x62]) - const pathname = await writeFile(content) - expect(await getStringLengthOfFile(content.byteLength, pathname)).to.be - .null - }) - - it('does not read a file too large to be editable', async function () { - // A pathname that cannot be read at all: reaching the read would throw. - const result = await getStringLengthOfFile( - Blob.MAX_EDITABLE_BYTE_LENGTH_BOUND + 1, - path.join(tmpDir, 'does-not-exist') - ) - expect(result).to.be.null - }) - }) -}) diff --git a/libraries/overleaf-editor-core/test/unit/blob_utils.test.js b/libraries/overleaf-editor-core/test/unit/blob_utils.test.js new file mode 100644 index 0000000000..4d9a109f9f --- /dev/null +++ b/libraries/overleaf-editor-core/test/unit/blob_utils.test.js @@ -0,0 +1,313 @@ +// @ts-check +'use strict' + +const { expect } = require('chai') +const fs = require('node:fs') +const os = require('node:os') +const path = require('node:path') +const { Readable } = require('node:stream') + +const { + blobHashFromString, + blobHashFromBuffer, + blobHashFromStream, + blobHashFromFile, + getStringLengthOfBuffer, + getStringLengthOfFile, + blobForFile, +} = require('../../lib/blob_utils') +const Blob = require('../../lib/blob') +const File = require('../../lib/file') +const TextOperation = require('../../lib/operation/text_operation') + +// `git hash-object` output for the same content. +const HELLO_WORLD = 'hello world\n' +const HELLO_WORLD_HASH = '3b18e512dba79e4c8300dd08aeb37f8e728b8dad' +const EMPTY_HASH = 'e69de29bb2d1d6434b8b29ae775ad8c2e48c5391' + +describe('blob utils', function () { + /** @type {string} */ + let tmpDir + + beforeEach(async function () { + tmpDir = await fs.promises.mkdtemp(path.join(os.tmpdir(), 'blob-hash-')) + }) + + afterEach(async function () { + await fs.promises.rm(tmpDir, { recursive: true, force: true }) + }) + + /** + * @param {string | Buffer} content + * @return {Promise} + */ + async function writeFile(content) { + const pathname = path.join(tmpDir, 'file') + await fs.promises.writeFile(pathname, content) + return pathname + } + + /** + * chai 3 has no chai-as-promised here. + * + * @param {Promise} promise + * @return {Promise} + */ + async function rejection(promise) { + try { + await promise + } catch (err) { + return /** @type {Error} */ (err) + } + throw new Error('expected the promise to reject') + } + + describe('blobHashFromString', function () { + it('produces the git blob hash of the string', function () { + expect(blobHashFromString(HELLO_WORLD)).to.equal(HELLO_WORLD_HASH) + }) + + it('produces the empty file hash for the empty string', function () { + expect(blobHashFromString('')).to.equal(EMPTY_HASH) + expect(blobHashFromString('')).to.equal(File.EMPTY_FILE_HASH) + }) + + it('hashes the utf8 bytes of the string', function () { + // 'héllo\n' is 7 bytes in utf8 and 6 characters, so a hash over the + // string length would not match git. + expect(blobHashFromString('héllo\n')).to.equal( + '5fb50d3c93474f139362304b663fe44e9d17a26e' + ) + }) + + // The shas real GitHub reported for these strings, recorded in + // services/github-sync/test/acceptance/js/ExportingAProject.test.js. They are + // the whole premise of pushing a history snapshot to GitHub without + // re-uploading blobs: the hash history stores is the sha GitHub has. + it('agrees with the shas GitHub reported for an exported project', function () { + expect(blobHashFromString('Hello world')).to.equal( + '70c379b63ffa0795fdbfbc128e5a2818397b7ef8' + ) + expect(blobHashFromString('Chapter 1 content')).to.equal( + 'ae3a11ab13155c7d114fa05590a1c1e23b1c0913' + ) + expect(blobHashFromString('Chapter 2 content')).to.equal( + '6986b1ac5bf8e4aa890b57b71c1d215f8f9a27e5' + ) + expect(blobHashFromString('')).to.equal( + 'e69de29bb2d1d6434b8b29ae775ad8c2e48c5391' + ) + }) + }) + + describe('blobHashFromBuffer', function () { + it('produces the git blob hash of the content', function () { + expect(blobHashFromBuffer(Buffer.from(HELLO_WORLD))).to.equal( + HELLO_WORLD_HASH + ) + }) + + it('produces the empty file hash for an empty buffer', function () { + expect(blobHashFromBuffer(Buffer.alloc(0))).to.equal(EMPTY_HASH) + }) + + it('agrees with blobHashFromString for multi-byte characters', function () { + const content = 'héllo wörld\n' + expect(blobHashFromBuffer(Buffer.from(content))).to.equal( + blobHashFromString(content) + ) + }) + + it('hashes bytes that are not text', function () { + expect( + blobHashFromBuffer(Buffer.from([0x00, 0x01, 0xff, 0xfe])) + ).to.equal('ad2f38543fc2bba3468a77f36137c23378420463') + }) + }) + + describe('blobHashFromStream', function () { + it('produces the git blob hash of the content', async function () { + const content = Buffer.from(HELLO_WORLD) + expect( + await blobHashFromStream(content.byteLength, Readable.from([content])) + ).to.equal(HELLO_WORLD_HASH) + }) + + it('agrees with blobHashFromString across chunk boundaries', async function () { + const content = 'x'.repeat(128 * 1024) + 'end\n' + expect( + await blobHashFromStream( + Buffer.byteLength(content), + Readable.from([Buffer.from(content)]) + ) + ).to.equal(blobHashFromString(content)) + }) + + it('rejects a byte length that is not an integer', async function () { + // The header would read "blob NaN\0" and the digest would look like a + // hash rather than like a failure. + const err = await rejection( + blobHashFromStream( + // @ts-expect-error deliberately bad input + undefined, + Readable.from([Buffer.from(HELLO_WORLD)]) + ) + ) + expect(err).to.be.an.instanceof(TypeError) + }) + }) + + describe('blobHashFromFile', function () { + it('produces the git blob hash of the file content', async function () { + const pathname = await writeFile(HELLO_WORLD) + expect(await blobHashFromFile(pathname)).to.equal(HELLO_WORLD_HASH) + }) + + it('produces the empty file hash for an empty file', async function () { + const pathname = await writeFile('') + expect(await blobHashFromFile(pathname)).to.equal(EMPTY_HASH) + }) + + it('agrees with blobHashFromString for multi-byte characters', async function () { + const content = 'héllo wörld\n' + const pathname = await writeFile(content) + expect(await blobHashFromFile(pathname)).to.equal( + blobHashFromString(content) + ) + }) + + it('agrees with blobHashFromString for content larger than one chunk', async function () { + const content = 'x'.repeat(128 * 1024) + 'end\n' + const pathname = await writeFile(content) + expect(await blobHashFromFile(pathname)).to.equal( + blobHashFromString(content) + ) + }) + + it('hashes binary content', async function () { + const pathname = await writeFile(Buffer.from([0x00, 0x01, 0xff, 0xfe])) + expect(await blobHashFromFile(pathname)).to.equal( + 'ad2f38543fc2bba3468a77f36137c23378420463' + ) + }) + + it('rejects when the file is missing', async function () { + const err = await rejection( + blobHashFromFile(path.join(tmpDir, 'does-not-exist')) + ) + expect(err).to.have.property('code', 'ENOENT') + }) + }) + describe('getStringLengthOfBuffer', function () { + it('counts the characters of plain ASCII', function () { + expect(getStringLengthOfBuffer(Buffer.from('hello'))).to.equal(5) + }) + + it('counts characters rather than bytes for multi-byte UTF-8', function () { + // 'é' is two bytes, 'ᚠ' is three. + const buffer = Buffer.from('héllᚠ') + expect(buffer.byteLength).to.equal(8) + expect(getStringLengthOfBuffer(buffer)).to.equal(5) + }) + + it('rejects bytes that are not valid UTF-8', function () { + // 0x80 is a continuation byte with nothing to continue. + expect(getStringLengthOfBuffer(Buffer.from([0x61, 0x80, 0x62]))).to.be + .null + }) + + it('rejects a NUL character', function () { + expect(getStringLengthOfBuffer(Buffer.from('a\0b'))).to.be.null + }) + + it('rejects non-BMP characters', function () { + expect(getStringLengthOfBuffer(Buffer.from('a 🙂 face'))).to.be.null + }) + + it('rejects content longer than a text operation can hold', function () { + const content = 'a'.repeat(TextOperation.MAX_STRING_LENGTH + 1) + expect(getStringLengthOfBuffer(Buffer.from(content))).to.be.null + }) + + it('accepts content at the maximum length', function () { + const content = 'a'.repeat(TextOperation.MAX_STRING_LENGTH) + expect(getStringLengthOfBuffer(Buffer.from(content))).to.equal( + TextOperation.MAX_STRING_LENGTH + ) + }) + }) + + describe('getStringLengthOfFile', function () { + it('counts the characters of an editable file', async function () { + const content = Buffer.from('héllᚠ') + const pathname = await writeFile(content) + expect( + await getStringLengthOfFile(content.byteLength, pathname) + ).to.equal(5) + }) + + it('rejects a file that is not editable text', async function () { + const content = Buffer.from([0x61, 0x00, 0x62]) + const pathname = await writeFile(content) + expect(await getStringLengthOfFile(content.byteLength, pathname)).to.be + .null + }) + + it('does not read a file too large to be editable', async function () { + // A pathname that cannot be read at all: reaching the read would throw. + const result = await getStringLengthOfFile( + Blob.MAX_EDITABLE_BYTE_LENGTH_BOUND + 1, + path.join(tmpDir, 'does-not-exist') + ) + expect(result).to.be.null + }) + }) + describe('blobForFile', function () { + it('describes an editable file', async function () { + const pathname = await writeFile(HELLO_WORLD) + + const blob = await blobForFile(pathname) + + expect(blob.getHash()).to.equal(HELLO_WORLD_HASH) + expect(blob.getByteLength()).to.equal(HELLO_WORLD.length) + expect(blob.getStringLength()).to.equal(HELLO_WORLD.length) + }) + + it('counts characters rather than bytes for multi-byte UTF-8', async function () { + const content = 'ol\u00e9\n' + const pathname = await writeFile(content) + + const blob = await blobForFile(pathname) + + expect(blob.getByteLength()).to.equal(5) + expect(blob.getStringLength()).to.equal(4) + }) + + it('leaves the string length off a file that is not editable text', async function () { + // What tells history to hold the content as a file rather than as a doc. + const pathname = await writeFile(Buffer.from([0xff, 0xfe, 0x00])) + + const blob = await blobForFile(pathname) + + expect(blob.getStringLength()).to.be.undefined + expect(blob.getByteLength()).to.equal(3) + }) + + it('describes an empty file', async function () { + const pathname = await writeFile('') + + const blob = await blobForFile(pathname) + + expect(blob.getHash()).to.equal(File.EMPTY_FILE_HASH) + expect(blob.getByteLength()).to.equal(0) + expect(blob.getStringLength()).to.equal(0) + }) + + it('rejects when the file is missing', async function () { + const err = await rejection( + blobForFile(path.join(tmpDir, 'does-not-exist')) + ) + expect(err).to.have.property('code', 'ENOENT') + }) + }) +}) diff --git a/libraries/overleaf-editor-core/test/unit/build_set_content_operations.test.js b/libraries/overleaf-editor-core/test/unit/build_set_content_operations.test.js index 5c75a509d0f47a173c90059c7d67ba2b5825f2ed..497b054ea8c7bb9916305251fd2070cc89b26409 100644 GIT binary patch delta 254 zcmdl`d8%T9-Nck6v1)~+octt@#NrIMqWs+8lA_GKbcNc9AH^mK;xaTLM zhNo|Ue^7|4OT1%|m4T_AnYl%j`s8&?!JA(*_c3xI zjGNre>9%=4>ltPwvCZ9_H<8tBPT^iA#^^S=S6Y0sgwiBVE(IuopK#O=501|9a^#A|> delta 431 zcmY+A&q~8U5XP+r!FUs`2NB0u*hFbW5UdrYsEA-6!9(+Bn*`!+-E6R-1Ye=D2d|z4 zPbxWh^zKX4zJNY~;5I2rPs4oQH@_L?qx+Kdk2V}iA`T8qI-J5GFnk>c4C{zc7uO5< z?f}0IzDwd~x_R5^Sj@VDUc^~IycCM6!X`8r<}oAxmKX49#FG$%9a0;7EAnM-TW!b0 z;Q=MNd@fbJfEdtIK^~A(ip{bIFdc?1j`O6ABH!9UMq$JqR~mXN=T~~ial4I~X(t>v zG4c2xIGt)uPK*~86gTj2#C7E3t^}u9W9Gh;4V48&43)F&WKNW_PxY}Z`s)v3U(;%% zWEfJop)7QfVHhT-rhJN#+9^F(B`A*=oM&}F@;iQPBQYpfMQd3V@4Fz@ Twl2lnUS-1nGFGzL+Ntsj`+||# diff --git a/libraries/overleaf-editor-core/test/unit/change_identity.test.js b/libraries/overleaf-editor-core/test/unit/change_identity.test.js index 0adc6578fd..3845ea6e09 100644 --- a/libraries/overleaf-editor-core/test/unit/change_identity.test.js +++ b/libraries/overleaf-editor-core/test/unit/change_identity.test.js @@ -5,6 +5,7 @@ const { editorChangeIdentity, editorChangeIdentityOf, isSameEditorChange, + isChangeFrom, EDITOR_ORIGIN_KIND, } = require('../..') @@ -23,7 +24,7 @@ describe('editor change identity', function () { origin: 'origin' in overrides ? overrides.origin - : { kind: EDITOR_ORIGIN_KIND, editorId: EDITOR_A }, + : { kind: EDITOR_ORIGIN_KIND, historyClientId: EDITOR_A }, } } @@ -40,18 +41,20 @@ describe('editor change identity', function () { // change is treated as a duplicate and silently discarded. it('a different editor sharing the author and timestamp', function () { - // The case editorId exists for: two tabs of one user, or two anonymous + // The case historyClientId exists for: two tabs of one user, or two anonymous // editors, can produce changes on the same millisecond. expect( same( raw(), - raw({ origin: { kind: EDITOR_ORIGIN_KIND, editorId: EDITOR_B } }) + raw({ + origin: { kind: EDITOR_ORIGIN_KIND, historyClientId: EDITOR_B }, + }) ) ).to.be.false }) - it('a different author sharing the editorId', function () { - // editorId comes from the client, so a forged one must not let one account + it('a different author sharing the historyClientId', function () { + // historyClientId comes from the client, so a forged one must not let one account // suppress another's change. expect(same(raw(), raw({ v2Authors: [USER_B] }))).to.be.false }) @@ -69,7 +72,7 @@ describe('editor change identity', function () { expect(same(raw(), raw({ origin: undefined }))).to.be.false }) - it('a change whose editorId was stripped when its chunk was written', function () { + it('a change whose historyClientId was stripped when its chunk was written', function () { expect(same(raw(), raw({ origin: { kind: EDITOR_ORIGIN_KIND } }))).to.be .false }) @@ -91,14 +94,14 @@ describe('editor change identity', function () { }) it('still separates two anonymous editors', function () { - // They share an empty author, so the editorId is the only thing keeping + // They share an empty author, so the historyClientId is the only thing keeping // their changes apart. expect( same( raw({ v2Authors: [null] }), raw({ v2Authors: [null], - origin: { kind: EDITOR_ORIGIN_KIND, editorId: EDITOR_B }, + origin: { kind: EDITOR_ORIGIN_KIND, historyClientId: EDITOR_B }, }) ) ).to.be.false @@ -131,7 +134,7 @@ describe('editor change identity', function () { describe('editorChangeIdentityOf', function () { it('matches a raw change describing the same submission', function () { const mine = editorChangeIdentityOf({ - editorId: EDITOR_A, + historyClientId: EDITOR_A, author: USER_A, timestamp: new Date(TIMESTAMP), }) @@ -141,7 +144,7 @@ describe('editor change identity', function () { it('treats a missing author as anonymous', function () { const mine = editorChangeIdentityOf({ - editorId: EDITOR_A, + historyClientId: EDITOR_A, author: null, timestamp: new Date(TIMESTAMP), }) @@ -160,3 +163,79 @@ describe('editor change identity', function () { expect(isSameEditorChange(editorChangeIdentity(raw()), null)).to.be.false }) }) + +describe('isChangeFrom', function () { + const KIND = 'github' + + function raw(overrides = {}) { + return { + operations: [], + timestamp: overrides.timestamp || TIMESTAMP, + v2Authors: 'v2Authors' in overrides ? overrides.v2Authors : [USER_A], + origin: 'origin' in overrides ? overrides.origin : { kind: KIND }, + } + } + + const expected = { + originKind: KIND, + author: USER_A, + timestamp: new Date(TIMESTAMP), + } + + it('recognises a change this writer placed', function () { + expect(isChangeFrom(raw(), expected)).to.be.true + }) + + it('recognises it among several authors', function () { + expect(isChangeFrom(raw({ v2Authors: [USER_B, USER_A] }), expected)).to.be + .true + }) + + it('compares the timestamp as a point in time', function () { + // two spellings of the same instant are the same change + expect( + isChangeFrom( + raw({ timestamp: '2025-01-02T04:04:05.678+01:00' }), + expected + ) + ).to.be.true + }) + + describe('what it refuses to match', function () { + // a false match here has the writer take a commit it never landed as landed, and the removes it holds are never applied + + it('a kind another writer stamps', function () { + expect(isChangeFrom(raw({ origin: { kind: 'dropbox' } }), expected)).to.be + .false + }) + + it('a change with no origin', function () { + expect(isChangeFrom(raw({ origin: undefined }), expected)).to.be.false + }) + + it('a change placed on behalf of another user', function () { + expect(isChangeFrom(raw({ v2Authors: [USER_B] }), expected)).to.be.false + }) + + it('a change with no authors recorded', function () { + expect(isChangeFrom(raw({ v2Authors: undefined }), expected)).to.be.false + expect(isChangeFrom(raw({ v2Authors: [] }), expected)).to.be.false + }) + + it('a change from another moment', function () { + expect( + isChangeFrom(raw({ timestamp: '2025-01-02T03:04:05.679Z' }), expected) + ).to.be.false + }) + + it('an unparseable timestamp', function () { + expect(isChangeFrom(raw({ timestamp: 'not a date' }), expected)).to.be + .false + }) + + it('nothing at all on either side', function () { + expect(isChangeFrom(undefined, expected)).to.be.false + expect(isChangeFrom(raw(), undefined)).to.be.false + }) + }) +}) diff --git a/libraries/overleaf-editor-core/test/unit/editor_origin.test.js b/libraries/overleaf-editor-core/test/unit/editor_origin.test.js deleted file mode 100644 index 7dbfb0ab5a..0000000000 --- a/libraries/overleaf-editor-core/test/unit/editor_origin.test.js +++ /dev/null @@ -1,102 +0,0 @@ -'use strict' - -const { expect } = require('chai') -const { Change, EditorOrigin, Origin, EDITOR_ORIGIN_KIND } = require('../..') -const schemas = require('../../lib/schemas') - -describe('EditorOrigin', function () { - const EDITOR_ID = '3f2504e0-4f89-41d3-9a0c-0305e82c3301' - - it('round trips through raw form', function () { - const origin = new EditorOrigin(EDITOR_ID) - expect(origin.toRaw()).to.deep.equal({ - kind: EDITOR_ORIGIN_KIND, - editorId: EDITOR_ID, - }) - - const parsed = Origin.fromRaw(origin.toRaw()) - expect(parsed).to.be.an.instanceof(EditorOrigin) - expect(parsed.getEditorId()).to.equal(EDITOR_ID) - expect(parsed.getKind()).to.equal(EDITOR_ORIGIN_KIND) - }) - - it('rejects a missing editorId', function () { - expect(() => new EditorOrigin()).to.throw() - expect(() => new EditorOrigin('')).to.throw() - }) - - describe('once the editorId has been stripped', function () { - // Writing a chunk drops the editorId from all but an editor's latest change, - // leaving a bare {kind: 'editor'}. Reading one of those back has to keep - // working, so it must not be treated as an EditorOrigin. - it('parses as a plain Origin rather than throwing', function () { - const parsed = Origin.fromRaw({ kind: EDITOR_ORIGIN_KIND }) - - expect(parsed).to.be.an.instanceof(Origin) - expect(parsed).to.not.be.an.instanceof(EditorOrigin) - expect(parsed.getKind()).to.equal(EDITOR_ORIGIN_KIND) - expect(parsed.toRaw()).to.deep.equal({ kind: EDITOR_ORIGIN_KIND }) - }) - - it('survives a Change round trip', function () { - const change = Change.fromRaw({ - operations: [], - timestamp: '2025-01-02T03:04:05.678Z', - origin: { kind: EDITOR_ORIGIN_KIND }, - }) - - expect(change.getOrigin().getKind()).to.equal(EDITOR_ORIGIN_KIND) - expect(change.toRaw().origin).to.deep.equal({ kind: EDITOR_ORIGIN_KIND }) - }) - }) - - it('is carried through a Change round trip while it has an id', function () { - const change = Change.fromRaw({ - operations: [], - timestamp: '2025-01-02T03:04:05.678Z', - origin: { kind: EDITOR_ORIGIN_KIND, editorId: EDITOR_ID }, - }) - - expect(change.getOrigin()).to.be.an.instanceof(EditorOrigin) - expect(change.getOrigin().getEditorId()).to.equal(EDITOR_ID) - expect(change.toRaw().origin).to.deep.equal({ - kind: EDITOR_ORIGIN_KIND, - editorId: EDITOR_ID, - }) - }) - - describe('rawOrigin schema', function () { - it('accepts an editor origin with a uuid', function () { - expect( - schemas.rawOrigin.safeParse({ - kind: EDITOR_ORIGIN_KIND, - editorId: EDITOR_ID, - }).success - ).to.be.true - }) - - it('accepts the stripped form', function () { - expect(schemas.rawOrigin.safeParse({ kind: EDITOR_ORIGIN_KIND }).success) - .to.be.true - }) - - it('rejects an editorId that is not a uuid', function () { - expect( - schemas.rawOrigin.safeParse({ - kind: EDITOR_ORIGIN_KIND, - editorId: 'not-a-uuid', - }).success - ).to.be.false - }) - - it('rejects unknown fields alongside the editorId', function () { - expect( - schemas.rawOrigin.safeParse({ - kind: EDITOR_ORIGIN_KIND, - editorId: EDITOR_ID, - somethingElse: 1, - }).success - ).to.be.false - }) - }) -}) diff --git a/libraries/overleaf-editor-core/test/unit/file.test.js b/libraries/overleaf-editor-core/test/unit/file.test.js index 648f736631..5867780922 100644 --- a/libraries/overleaf-editor-core/test/unit/file.test.js +++ b/libraries/overleaf-editor-core/test/unit/file.test.js @@ -4,6 +4,7 @@ const { expect } = require('chai') const FakeBlobStore = require('./support/fake_blob_store') const ot = require('../..') const File = ot.File +const Blob = ot.Blob describe('File', function () { it('can have attached metadata', function () { @@ -93,6 +94,85 @@ describe('File', function () { }) }) + describe('loadEager', function () { + const HASH = 'a'.repeat(40) + + /** + * A store of blobs named by what they hold. A blob with no string length is + * what history stores content it cannot hold as a doc under. + * + * @param {Record} blobs + */ + function blobStoreOf(blobs) { + return { + async getBlob(hash) { + const blob = blobs[hash] + if (blob == null) return null + return new Blob( + hash, + Buffer.byteLength(blob.content), + blob.stringLength + ) + }, + async getString(hash) { + const blob = blobs[hash] + if (blob == null) throw new Error(`no blob: ${hash}`) + return blob.content + }, + async getObject(hash) { + return JSON.parse(await this.getString(hash)) + }, + } + } + + /** + * chai 3 has no chai-as-promised here. + * + * @param {Promise} promise + * @return {Promise} + */ + async function rejection(promise) { + try { + await promise + } catch (err) { + return err + } + throw new Error('expected the promise to reject') + } + + it('answers with the data holding the content', async function () { + // What the caller gets is the kind of data that has content, rather than + // the file it has to ask again and handle a null from. + const file = File.fromHash(HASH, undefined, undefined, 'hello'.length) + + const data = await file.loadEager( + blobStoreOf({ [HASH]: { content: 'hello', stringLength: 5 } }) + ) + + expect(data.getContent()).to.equal('hello') + expect(file.getContent()).to.equal('hello') + }) + + it('answers with the data of a file that already has its content', async function () { + const file = File.fromString('foo') + + const data = await file.loadEager(new FakeBlobStore()) + + expect(data.getContent()).to.equal('foo') + }) + + it('refuses a file that is not an editable doc', async function () { + // Bytes history addresses by hash: there is no content to answer with. + const file = File.fromHash(HASH) + + const err = await rejection( + file.loadEager(blobStoreOf({ [HASH]: { content: 'hello' } })) + ) + + expect(err).to.be.an.instanceof(File.NotEditableError) + }) + }) + describe('with hollow string data', function () { it('can be cloned', function () { const file = File.createHollow(null, 0) diff --git a/libraries/overleaf-editor-core/test/unit/file_type_detector.test.js b/libraries/overleaf-editor-core/test/unit/file_type_detector.test.js new file mode 100644 index 0000000000..1edc5a1278 --- /dev/null +++ b/libraries/overleaf-editor-core/test/unit/file_type_detector.test.js @@ -0,0 +1,247 @@ +// @ts-check +'use strict' + +const { expect } = require('chai') +const fs = require('node:fs') +const os = require('node:os') +const path = require('node:path') + +const { + needsContent, + detectBuffer, + detectFile, +} = require('../../lib/file_type_detector') + +// Small enough to exercise the size and length limits with tiny fixtures. +const MAX_DOC_LENGTH = 32 + +/** @type {import('../../lib/file_type_detector').FileTypeConfig} */ +const CONFIG = { + textExtensions: ['tex', 'txt'], + editableFilenames: ['latexmkrc', '.latexmkrc', 'makefile'], + maxDocLength: MAX_DOC_LENGTH, +} + +describe('file type detector', function () { + /** @type {string} */ + let tmpDir + + beforeEach(async function () { + tmpDir = await fs.promises.mkdtemp( + path.join(os.tmpdir(), 'file-type-detector-') + ) + }) + + afterEach(async function () { + await fs.promises.rm(tmpDir, { recursive: true, force: true }) + }) + + /** + * The two entry points have to agree on every row of the table, so each case + * below runs through both: detectFile with the content spooled to disk, and + * detectBuffer with it in memory. + * + * @param {string | Buffer} bytes + * @param {string} pathname + * @param {import('../../lib/file_type_detector').ExistingType} existingType + * @return {Promise} + */ + async function detect(bytes, pathname, existingType) { + const buffer = Buffer.isBuffer(bytes) ? bytes : Buffer.from(bytes) + const localPath = path.join(tmpDir, 'spooled') + await fs.promises.writeFile(localPath, buffer) + + const fromFile = await detectFile(pathname, localPath, existingType, CONFIG) + const fromBuffer = detectBuffer(buffer, { pathname, existingType }, CONFIG) + expect(fromBuffer).to.deep.equal(fromFile) + return fromFile + } + + describe('the filename', function () { + it('accepts a listed extension', async function () { + expect(await detect('hello', 'dir/main.tex', null)).to.deep.equal({ + kind: 'text', + content: 'hello', + }) + }) + + it('accepts a listed extension in upper case', async function () { + expect(await detect('hello', 'MAIN.TeX', null)).to.have.property( + 'kind', + 'text' + ) + }) + + it('rejects an extension that is not listed', async function () { + expect(await detect('hello', 'main.png', null)).to.deep.equal({ + kind: 'binary', + }) + }) + + it('accepts a name from the editable filenames list', async function () { + expect(await detect('hello', 'dir/Makefile', null)).to.have.property( + 'kind', + 'text' + ) + }) + + it('skips the name check for a file that is already a doc', async function () { + expect(await detect('hello', 'main.png', 'doc')).to.have.property( + 'kind', + 'text' + ) + }) + + it('still applies the name check for a file that is not a doc', async function () { + expect(await detect('hello', 'main.png', 'file')).to.deep.equal({ + kind: 'binary', + }) + }) + + it('keeps a binary file binary at a text pathname', async function () { + expect(await detect('hello', 'main.tex', 'file')).to.deep.equal({ + kind: 'binary', + }) + }) + }) + + describe('the content', function () { + it('rejects a file that is too large to be worth reading', async function () { + const bytes = 'a'.repeat(3 * MAX_DOC_LENGTH + 1) + expect(await detect(bytes, 'main.tex', 'doc')).to.deep.equal({ + kind: 'binary', + }) + }) + + it('rejects content at the character limit', async function () { + const bytes = 'a'.repeat(MAX_DOC_LENGTH) + expect(await detect(bytes, 'main.tex', null)).to.deep.equal({ + kind: 'binary', + }) + }) + + it('accepts content just below the character limit', async function () { + const bytes = 'a'.repeat(MAX_DOC_LENGTH - 1) + expect(await detect(bytes, 'main.tex', null)).to.have.property( + 'kind', + 'text' + ) + }) + + it('rejects content with a NUL byte', async function () { + const bytes = Buffer.from('ab\u0000cd', 'utf8') + expect(await detect(bytes, 'main.tex', null)).to.deep.equal({ + kind: 'binary', + }) + }) + + it('rejects bytes that encode a lone surrogate', async function () { + // U+D800, an unpaired high surrogate, in the three-byte form a UTF-8 + // encoder would use for it. + const bytes = Buffer.from([0xed, 0xa0, 0x80]) + expect(await detect(bytes, 'main.tex', null)).to.deep.equal({ + kind: 'binary', + }) + }) + + it('rejects a character outside the basic multilingual plane', async function () { + // Valid UTF-8 can only ever produce surrogates in pairs, so this row is + // caught whether the rule tests high surrogates or both halves. The wider + // range matters for content decoded as UTF-16, which web does and this + // module does not — see the note in the module header. + const bytes = Buffer.from('a\u{1F600}b', 'utf8') + expect(await detect(bytes, 'main.tex', null)).to.deep.equal({ + kind: 'binary', + }) + }) + + it('rejects little-endian utf-16 announced by a byte order mark', async function () { + // A byte order mark followed by "hi": text a reader can make sense of, + // which still cannot be stored as text without losing the bytes. + const bytes = Buffer.from([0xff, 0xfe, 0x68, 0x00, 0x69, 0x00]) + expect(await detect(bytes, 'main.tex', null)).to.deep.equal({ + kind: 'binary', + }) + }) + + it('rejects bytes that are not valid utf-8', async function () { + // Every byte here stands for a printable latin1 character, so the encoding + // is the only thing that makes this a binary file. + const bytes = Buffer.from([0x68, 0xe9, 0x69]) + expect(await detect(bytes, 'main.tex', null)).to.deep.equal({ + kind: 'binary', + }) + }) + + it('accepts multi-byte utf-8', async function () { + expect( + await detect(Buffer.from('héi', 'utf8'), 'main.tex', null) + ).to.deep.equal({ kind: 'text', content: 'héi' }) + }) + + it('accepts an empty file', async function () { + expect(await detect('', 'main.tex', null)).to.deep.equal({ + kind: 'text', + content: '', + }) + }) + }) + + describe('needsContent', function () { + it('is false for an existing binary file', function () { + expect(needsContent('main.tex', 10, 'file', CONFIG)).to.equal(false) + }) + + it('is false for a name that cannot hold text', function () { + expect(needsContent('main.png', 10, null, CONFIG)).to.equal(false) + }) + + it('is true for a name that cannot hold text but is already a doc', function () { + expect(needsContent('main.png', 10, 'doc', CONFIG)).to.equal(true) + }) + + it('is false above three times the character limit', function () { + expect( + needsContent('main.tex', 3 * MAX_DOC_LENGTH + 1, null, CONFIG) + ).to.equal(false) + }) + + it('is true at three times the character limit', function () { + expect( + needsContent('main.tex', 3 * MAX_DOC_LENGTH, null, CONFIG) + ).to.equal(true) + }) + + it('skips the size gate when the size is not known', function () { + expect(needsContent('main.tex', null, null, CONFIG)).to.equal(true) + }) + }) + + describe('the config', function () { + it('rejects a config without a document length', function () { + // Every length comparison against undefined is false, which reads as "no + // limit" rather than as a misconfiguration. + expect(() => + needsContent( + 'main.tex', + 10, + null, + // @ts-expect-error deliberately bad input + { textExtensions: [], editableFilenames: [] } + ) + ).to.throw(TypeError) + }) + + it('rejects a config without the extension lists', function () { + expect(() => + needsContent( + 'main.tex', + 10, + null, + // @ts-expect-error deliberately bad input + { maxDocLength: MAX_DOC_LENGTH } + ) + ).to.throw(TypeError) + }) + }) +}) diff --git a/libraries/overleaf-editor-core/test/unit/origin.test.js b/libraries/overleaf-editor-core/test/unit/origin.test.js new file mode 100644 index 0000000000..02ba3ab77e --- /dev/null +++ b/libraries/overleaf-editor-core/test/unit/origin.test.js @@ -0,0 +1,212 @@ +'use strict' + +const { expect } = require('chai') +const { + Change, + Origin, + RestoreFileOrigin, + EDITOR_ORIGIN_KIND, +} = require('../..') +const schemas = require('../../lib/schemas') + +describe('Origin', function () { + const CLIENT_ID = '3f2504e0-4f89-41d3-9a0c-0305e82c3301' + + it('round trips a kind on its own', function () { + const origin = new Origin('dropbox') + // No key for an id nothing has: the shape of the stored origin is unchanged + // for every writer that does not need recognising. + expect(origin.toRaw()).to.deep.equal({ kind: 'dropbox' }) + expect(origin.getHistoryClientId()).to.be.undefined + + const parsed = Origin.fromRaw(origin.toRaw()) + expect(parsed.getKind()).to.equal('dropbox') + expect(parsed.getHistoryClientId()).to.be.undefined + }) + + it('round trips a kind with the client that submitted it', function () { + const origin = new Origin(EDITOR_ORIGIN_KIND, CLIENT_ID) + expect(origin.toRaw()).to.deep.equal({ + kind: EDITOR_ORIGIN_KIND, + historyClientId: CLIENT_ID, + }) + + const parsed = Origin.fromRaw(origin.toRaw()) + expect(parsed.getKind()).to.equal(EDITOR_ORIGIN_KIND) + expect(parsed.getHistoryClientId()).to.equal(CLIENT_ID) + }) + + it('rejects a historyClientId that is not a non-empty string', function () { + expect(() => new Origin(EDITOR_ORIGIN_KIND, '')).to.throw() + expect(() => new Origin(EDITOR_ORIGIN_KIND, 42)).to.throw() + }) + + it('carries it on a kind of its own, not just the editor kind', function () { + // What the id is for is recognising a change again, which is a property of + // the client that submitted it rather than of what it did. + const origin = new RestoreFileOrigin( + 7, + 'main.tex', + new Date('2025-01-02T03:04:05.678Z'), + CLIENT_ID + ) + + // Built up a layer at a time: the base writes the kind and the id, this one + // adds what it owns. + expect(origin.toRaw()).to.deep.equal({ + kind: 'file-restore', + historyClientId: CLIENT_ID, + version: 7, + path: 'main.tex', + timestamp: '2025-01-02T03:04:05.678Z', + }) + + const parsed = Origin.fromRaw(origin.toRaw()) + expect(parsed).to.be.an.instanceof(RestoreFileOrigin) + expect(parsed.getHistoryClientId()).to.equal(CLIENT_ID) + expect(parsed.getPath()).to.equal('main.tex') + }) + + describe('dropHistoryClientId', function () { + it('leaves the rest of the origin behind', function () { + const origin = new Origin(EDITOR_ORIGIN_KIND, CLIENT_ID) + + origin.dropHistoryClientId() + + expect(origin.getHistoryClientId()).to.be.undefined + expect(origin.toRaw()).to.deep.equal({ kind: EDITOR_ORIGIN_KIND }) + }) + + it('keeps the fields a subclass owns', function () { + // What the chunk store does to every change but a client's latest, so a + // restore that was submitted by a client stays a restore afterwards. + const origin = new RestoreFileOrigin( + 7, + 'main.tex', + new Date('2025-01-02T03:04:05.678Z'), + CLIENT_ID + ) + + origin.dropHistoryClientId() + + expect(origin.toRaw()).to.deep.equal({ + kind: 'file-restore', + version: 7, + path: 'main.tex', + timestamp: '2025-01-02T03:04:05.678Z', + }) + }) + }) + + describe('through a Change', function () { + function changeWithOrigin(origin) { + return Change.fromRaw({ + operations: [], + timestamp: '2025-01-02T03:04:05.678Z', + origin, + }) + } + + it('carries the id while there is one', function () { + const change = changeWithOrigin({ + kind: EDITOR_ORIGIN_KIND, + historyClientId: CLIENT_ID, + }) + + expect(change.getOrigin().getHistoryClientId()).to.equal(CLIENT_ID) + expect(change.toRaw().origin).to.deep.equal({ + kind: EDITOR_ORIGIN_KIND, + historyClientId: CLIENT_ID, + }) + }) + + it('reads back a change whose id was stripped', function () { + // Writing a chunk drops the id from all but a client's latest change, + // leaving a bare {kind: 'editor'}, which still records where it came from. + const change = changeWithOrigin({ kind: EDITOR_ORIGIN_KIND }) + + expect(change.getOrigin().getKind()).to.equal(EDITOR_ORIGIN_KIND) + expect(change.getOrigin().getHistoryClientId()).to.be.undefined + expect(change.toRaw().origin).to.deep.equal({ kind: EDITOR_ORIGIN_KIND }) + }) + }) + + describe('rawOrigin schema', function () { + it('accepts a kind with a uuid', function () { + expect( + schemas.rawOrigin.safeParse({ + kind: EDITOR_ORIGIN_KIND, + historyClientId: CLIENT_ID, + }).success + ).to.be.true + }) + + it('accepts the stripped form', function () { + expect(schemas.rawOrigin.safeParse({ kind: EDITOR_ORIGIN_KIND }).success) + .to.be.true + }) + + it('accepts one on a restore origin', function () { + expect( + schemas.rawOrigin.safeParse({ + kind: 'file-restore', + version: 7, + path: 'main.tex', + timestamp: '2025-01-02T03:04:05.678Z', + historyClientId: CLIENT_ID, + }).success + ).to.be.true + }) + + it('rejects a kind with a shape of its own that does not match it', function () { + // Without this the catch-all would take it: a change would store as a restore + // that records nothing about what it restored, and be read back that way. + expect(schemas.rawOrigin.safeParse({ kind: 'restore' }).success).to.be + .false + expect( + schemas.rawOrigin.safeParse({ kind: 'file-restore', version: 1 }) + .success + ).to.be.false + }) + + it('says which fields the kind a payload claims is missing', function () { + // The named kinds discriminate on the kind, so a payload claiming one is + // checked against that shape alone: the error is about what it got wrong, + // rather than one guess per variant in the union. + const result = schemas.rawOrigin.safeParse({ + kind: 'file-restore', + version: 1, + }) + + const [claimed] = result.error.issues[0].errors + expect(claimed.map(issue => issue.path.join('.'))).to.deep.equal([ + 'path', + 'timestamp', + ]) + }) + + it('accepts a kind that has no shape of its own', function () { + expect(schemas.rawOrigin.safeParse({ kind: 'dropbox' }).success).to.be + .true + }) + + it('rejects a historyClientId that is not a uuid', function () { + expect( + schemas.rawOrigin.safeParse({ + kind: EDITOR_ORIGIN_KIND, + historyClientId: 'not-a-uuid', + }).success + ).to.be.false + }) + + it('rejects unknown fields alongside the historyClientId', function () { + expect( + schemas.rawOrigin.safeParse({ + kind: EDITOR_ORIGIN_KIND, + historyClientId: CLIENT_ID, + somethingElse: 1, + }).success + ).to.be.false + }) + }) +}) diff --git a/libraries/overleaf-editor-core/test/unit/text_operation.test.js b/libraries/overleaf-editor-core/test/unit/text_operation.test.js index 3ec3846c8a..779650283a 100644 --- a/libraries/overleaf-editor-core/test/unit/text_operation.test.js +++ b/libraries/overleaf-editor-core/test/unit/text_operation.test.js @@ -391,6 +391,13 @@ describe('TextOperation', function () { }) describe('compose', function () { + it('rejects a second operation built on different content', function () { + const a = new TextOperation().retain(4) + const b = new TextOperation().retain(7) + + expect(() => a.compose(b)).to.throw(TextOperation.UnprocessableError) + }) + it( 'composes (randomised)', random.test(numTrials, () => { @@ -728,6 +735,20 @@ describe('TextOperation', function () { }) describe('transform', function () { + it('rejects operations built on different content', function () { + // Two operations that do not describe the same starting content cannot be + // transformed against each other. That is the pair being wrong, not this + // process, and a caller that answers a client has to be able to tell the + // difference -- an internal error reads as worth retrying, and the same + // pair fails the same way every time. + const a = new TextOperation().retain(4) + const b = new TextOperation().retain(7) + + expect(() => TextOperation.transform(a, b)).to.throw( + TextOperation.UnprocessableError + ) + }) + it( 'transforms (randomised)', random.test(numTrials, () => { diff --git a/services/history-v1/api/controllers/projects.js b/services/history-v1/api/controllers/projects.js index c8042ba6f3..e1b2cc69e8 100644 --- a/services/history-v1/api/controllers/projects.js +++ b/services/history-v1/api/controllers/projects.js @@ -13,10 +13,10 @@ const { parseReq } = require('@overleaf/validation-tools') const logger = require('@overleaf/logger') const { Chunk, ChunkResponse, Blob } = require('overleaf-editor-core') +const { blobHashFromFile } = require('overleaf-editor-core/lib/blob_utils') const { BlobStore, BatchBlobStore, - blobHash, chunkStore, redisBuffer, HashCheckBlobStore, @@ -392,7 +392,7 @@ async function createProjectBlob(req, res, next) { ) return render.requestEntityTooLarge(res) } - const hash = await blobHash.fromFile(tmpPath) + const hash = await blobHashFromFile(tmpPath) if (hash !== expectedHash) { logger.warn({ projectId, hash, expectedHash }, 'Hash mismatch') return render.conflict(res, 'File hash mismatch') diff --git a/services/history-v1/storage/index.js b/services/history-v1/storage/index.js index 5424315892..b484fd7c5d 100644 --- a/services/history-v1/storage/index.js +++ b/services/history-v1/storage/index.js @@ -1,5 +1,4 @@ exports.BatchBlobStore = require('./lib/batch_blob_store') -exports.blobHash = require('./lib/blob_hash') exports.HashCheckBlobStore = require('./lib/hash_check_blob_store') exports.chunkStore = require('./lib/chunk_store') exports.redisBuffer = require('./lib/chunk_store/redis') diff --git a/services/history-v1/storage/lib/backupVerifier.mjs b/services/history-v1/storage/lib/backupVerifier.mjs index 85f13a8c66..a2191a3900 100644 --- a/services/history-v1/storage/lib/backupVerifier.mjs +++ b/services/history-v1/storage/lib/backupVerifier.mjs @@ -8,7 +8,7 @@ import { } from './backupPersistor.mjs' import { Blob, Chunk, History } from 'overleaf-editor-core' import { BlobStore, GLOBAL_BLOBS, makeProjectKey } from './blob_store/index.js' -import blobHash from './blob_hash.js' +import { blobHashFromStream } from 'overleaf-editor-core/lib/blob_utils.js' import { NotFoundError } from '@overleaf/object-persistor/src/Errors.js' import logger from '@overleaf/logger' import path from 'node:path' @@ -78,7 +78,7 @@ export async function verifyBlobs(historyId, hashes, projectCache) { } throw err } - const backupHash = await blobHash.fromStream(blob.getByteLength(), stream) + const backupHash = await blobHashFromStream(blob.getByteLength(), stream) if (backupHash !== hash) { throw new BackupCorruptedInvalidBlobError( 'hash mismatch for backed up blob', diff --git a/services/history-v1/storage/lib/blob_hash.js b/services/history-v1/storage/lib/blob_hash.js deleted file mode 100644 index 10ac64b87b..0000000000 --- a/services/history-v1/storage/lib/blob_hash.js +++ /dev/null @@ -1,80 +0,0 @@ -/** @module */ -'use strict' - -const BPromise = require('bluebird') -const fs = BPromise.promisifyAll(require('node:fs')) -const crypto = require('node:crypto') -const { pipeline } = require('node:stream') -const assert = require('./assert') - -function getGitBlobHeader(byteLength) { - return 'blob ' + byteLength + '\x00' -} - -function getBlobHash(byteLength) { - const hash = crypto.createHash('sha1') - hash.setEncoding('hex') - hash.update(getGitBlobHeader(byteLength)) - return hash -} - -/** - * Compute the git blob hash for a blob from a readable stream of its content. - * - * @function - * @param {number} byteLength - * @param {stream.Readable} stream - * @return {Promise.} hexadecimal SHA-1 hash - */ -exports.fromStream = BPromise.method( - function blobHashFromStream(byteLength, stream) { - assert.integer(byteLength, 'blobHash: bad byteLength') - assert.object(stream, 'blobHash: bad stream') - - const hash = getBlobHash(byteLength) - return new BPromise(function (resolve, reject) { - pipeline(stream, hash, function (err) { - if (err) { - reject(err) - } else { - hash.end() - resolve(hash.read()) - } - }) - }) - } -) - -/** - * Compute the git blob hash for a blob with the given string content. - * - * @param {string} string - * @return {string} hexadecimal SHA-1 hash - */ -exports.fromString = function blobHashFromString(string) { - assert.string(string, 'blobHash: bad string') - const hash = getBlobHash(Buffer.byteLength(string)) - hash.update(string, 'utf8') - hash.end() - return hash.read() -} - -/** - * Compute the git blob hash for the content of a file - * - * @param {string} filePath - * @return {Promise} hexadecimal SHA-1 hash - */ -exports.fromFile = function blobHashFromFile(pathname) { - assert.string(pathname, 'blobHash: bad pathname') - - function getByteLengthOfFile() { - return fs.statAsync(pathname).then(stat => stat.size) - } - - const fromStream = this.fromStream - return getByteLengthOfFile(pathname).then(function (byteLength) { - const stream = fs.createReadStream(pathname) - return fromStream(byteLength, stream) - }) -} diff --git a/services/history-v1/storage/lib/blob_store/index.js b/services/history-v1/storage/lib/blob_store/index.js index dc65abc008..63d0f928ae 100644 --- a/services/history-v1/storage/lib/blob_store/index.js +++ b/services/history-v1/storage/lib/blob_store/index.js @@ -6,14 +6,15 @@ const { ReadableString } = require('@overleaf/stream-utils') const core = require('overleaf-editor-core') const { + blobHashFromStream, + blobHashFromString, getStringLengthOfFile, -} = require('overleaf-editor-core/lib/blob_string_length') +} = require('overleaf-editor-core/lib/blob_utils') const objectPersistor = require('@overleaf/object-persistor') const OError = require('@overleaf/o-error') const Blob = core.Blob const assert = require('../assert') -const blobHash = require('../blob_hash') const mongodb = require('../mongodb') const persistor = require('../persistor') const projectKey = require('@overleaf/object-persistor/src/ProjectKey.js') @@ -116,7 +117,7 @@ function getBackend(projectId) { async function makeBlobForFile(pathname) { const { size: byteLength } = await fs.promises.stat(pathname) - const hash = await blobHash.fromStream( + const hash = await blobHashFromStream( byteLength, fs.createReadStream(pathname) ) @@ -221,7 +222,7 @@ class BlobStore extends core.BlobStoreBase { */ async putString(string) { assert.string(string, 'bad string') - const hash = blobHash.fromString(string) + const hash = blobHashFromString(string) const existingBlob = await this._findBlobBeforeInsert(hash) if (existingBlob != null) { diff --git a/services/history-v1/storage/lib/hash_check_blob_store.js b/services/history-v1/storage/lib/hash_check_blob_store.js index 597b4b2de0..7e4fb71052 100644 --- a/services/history-v1/storage/lib/hash_check_blob_store.js +++ b/services/history-v1/storage/lib/hash_check_blob_store.js @@ -1,5 +1,5 @@ const { Blob, BlobStoreBase } = require('overleaf-editor-core') -const blobHash = require('./blob_hash') +const { blobHashFromString } = require('overleaf-editor-core/lib/blob_utils') // We want to simulate applying all of the operations so we can return the // resulting hashes to the caller for them to check. To do this, we need to be @@ -34,7 +34,7 @@ class HashCheckBlobStore extends BlobStoreBase { */ async putString(string) { return new Blob( - blobHash.fromString(string), + blobHashFromString(string), Buffer.byteLength(string), string.length ) diff --git a/services/history-v1/storage/scripts/backup.mjs b/services/history-v1/storage/scripts/backup.mjs index 7a99a5a85f..2df5e3ebc3 100644 --- a/services/history-v1/storage/scripts/backup.mjs +++ b/services/history-v1/storage/scripts/backup.mjs @@ -51,7 +51,7 @@ import { } from '@overleaf/mongo-utils/batchedUpdate.js' import { createGunzip } from 'node:zlib' import { text } from 'node:stream/consumers' -import { fromStream as blobHashFromStream } from '../lib/blob_hash.js' +import { blobHashFromStream } from 'overleaf-editor-core/lib/blob_utils.js' import { NotFoundError } from '@overleaf/object-persistor/src/Errors.js' // Create a singleton promise that loads global blobs once diff --git a/services/history-v1/storage/scripts/recover_zip.js b/services/history-v1/storage/scripts/recover_zip.js index 820c0c0b03..3a6018380a 100644 --- a/services/history-v1/storage/scripts/recover_zip.js +++ b/services/history-v1/storage/scripts/recover_zip.js @@ -42,9 +42,7 @@ function createStorage() { } const core = require('overleaf-editor-core') -const { - getStringLengthOfFile, -} = require('overleaf-editor-core/lib/blob_string_length') +const { getStringLengthOfFile } = require('overleaf-editor-core/lib/blob_utils') const projectKey = require('@overleaf/object-persistor/src/ProjectKey.js') const streams = require('../lib/streams') diff --git a/services/history-v1/test/acceptance/js/api/end_to_end.test.js b/services/history-v1/test/acceptance/js/api/end_to_end.test.js index f3ad144335..83171266ed 100644 --- a/services/history-v1/test/acceptance/js/api/end_to_end.test.js +++ b/services/history-v1/test/acceptance/js/api/end_to_end.test.js @@ -20,7 +20,7 @@ const Operation = core.Operation const Snapshot = core.Snapshot const TextOperation = core.TextOperation -const blobHash = require('../../../../storage').blobHash +const { blobHashFromString } = require('overleaf-editor-core/lib/blob_utils') describe('overleaf ot', function () { beforeEach(cleanup.everything) @@ -236,7 +236,7 @@ describe('overleaf ot', function () { testFiles.GRAPH_PNG_HASH ) expect(snapshot.getFile('main.tex').getHash()).to.equal( - blobHash.fromString('hello') + blobHashFromString('hello') ) return projectId }) @@ -283,7 +283,7 @@ describe('overleaf ot', function () { const snapshot = Snapshot.fromRaw(response.obj) expect(snapshot.countFiles()).to.equal(3) expect(snapshot.getFile('main.tex').getHash()).to.equal( - blobHash.fromString('hi world') + blobHashFromString('hi world') ) return projectId }) @@ -321,7 +321,7 @@ describe('overleaf ot', function () { const snapshot = Snapshot.fromRaw(response.obj) expect(snapshot.countFiles()).to.equal(3) expect(snapshot.getFile('intro.tex').getHash()).to.equal( - blobHash.fromString('hi world') + blobHashFromString('hi world') ) return projectId }) diff --git a/services/history-v1/test/acceptance/js/api/project_hashed_content.test.js b/services/history-v1/test/acceptance/js/api/project_hashed_content.test.js index b32514630d..fb8bd16688 100644 --- a/services/history-v1/test/acceptance/js/api/project_hashed_content.test.js +++ b/services/history-v1/test/acceptance/js/api/project_hashed_content.test.js @@ -12,7 +12,7 @@ const Change = core.Change const File = core.File const Operation = core.Operation -const blobHash = require('../../../../storage/lib/blob_hash') +const { blobHashFromString } = require('overleaf-editor-core/lib/blob_utils') const persistChanges = require('../../../../storage/lib/persist_changes') describe('project hashed content', function () { @@ -72,7 +72,7 @@ describe('project hashed content', function () { const file = response.obj.files['tracked.tex'] // The content is hashed as it stands, tracked delete included, and the ranges // are hashed separately. - expect(file.hash).to.equal(blobHash.fromString(content)) + expect(file.hash).to.equal(blobHashFromString(content)) expect(file.rangesHash).to.match(/^[0-9a-f]{40}$/) }) @@ -92,7 +92,7 @@ describe('project hashed content', function () { expect(response.status).to.equal(200) const file = response.obj.files['main.tex'] - expect(file.hash).to.equal(blobHash.fromString(content)) + expect(file.hash).to.equal(blobHashFromString(content)) expect(file.rangesHash).to.be.undefined }) }) diff --git a/services/history-v1/test/acceptance/js/api/project_updates.test.js b/services/history-v1/test/acceptance/js/api/project_updates.test.js index 3d4a41c97b..5d9f907606 100644 --- a/services/history-v1/test/acceptance/js/api/project_updates.test.js +++ b/services/history-v1/test/acceptance/js/api/project_updates.test.js @@ -22,7 +22,7 @@ const TextOperation = core.TextOperation const V2DocVersions = core.V2DocVersions const knex = require('../../../../storage').knex -const blobHash = require('../../../../storage/lib/blob_hash') +const { blobHashFromString } = require('overleaf-editor-core/lib/blob_utils') describe('history import', function () { beforeEach(cleanup.everything) @@ -537,7 +537,7 @@ describe('history import', function () { }, ], }) - const testRangesHash = blobHash.fromString(rangesContent) + const testRangesHash = blobHashFromString(rangesContent) const testFile = File.fromHash(testFiles.HELLO_TXT_HASH, testRangesHash) const [contentResponse, rangesResponse] = await Promise.all([ diff --git a/services/history-v1/test/acceptance/js/api/projects.test.js b/services/history-v1/test/acceptance/js/api/projects.test.js index 226366083b..b682261cbe 100644 --- a/services/history-v1/test/acceptance/js/api/projects.test.js +++ b/services/history-v1/test/acceptance/js/api/projects.test.js @@ -16,8 +16,11 @@ const { BlobStore, persistChanges, redisBuffer, - blobHash, } = require('../../../../storage') +const { + blobHashFromStream, + blobHashFromString, +} = require('overleaf-editor-core/lib/blob_utils') const { expectHttpError } = require('./support/expect_response') const testServer = require('./support/test_server') @@ -453,7 +456,7 @@ describe('project controller', function () { for (let i = 0; i < nTextBlobs; i++) { const content = `text blob ${i}` - const hash = blobHash.fromString(content) + const hash = blobHashFromString(content) blobHashes.push(hash) expectedTextBytes += content.length const res = await fetch( @@ -469,7 +472,7 @@ describe('project controller', function () { for (let i = 0; i < nBinaryBlobs; i++) { const content = Buffer.from([0, i, i + 1, i + 2]) - const hash = await blobHash.fromStream( + const hash = await blobHashFromStream( content.length, Readable.from(content) ) diff --git a/services/history-v1/test/acceptance/js/storage/backup_generator.test.mjs b/services/history-v1/test/acceptance/js/storage/backup_generator.test.mjs index 3c0e8f1e67..6c99c540f4 100644 --- a/services/history-v1/test/acceptance/js/storage/backup_generator.test.mjs +++ b/services/history-v1/test/acceptance/js/storage/backup_generator.test.mjs @@ -13,7 +13,7 @@ import { ObjectId } from 'mongodb' import testFiles from './support/test_files.js' import { BlobStore } from '../../../../storage/lib/blob_store/index.js' import fs from 'node:fs' -import blobHash from '../../../../storage/lib/blob_hash.js' +import { blobHashFromString } from 'overleaf-editor-core/lib/blob_utils.js' const scenarios = [ { @@ -197,7 +197,7 @@ for (const scenario of scenarios) { expect(results[1].chunkRecord.endVersion).to.equal(20) expect(results[1].blobsToBackup).to.have.deep.members([ { - hash: blobHash.fromString('a'.repeat(7)), + hash: blobHashFromString('a'.repeat(7)), byteLength: 7, stringLength: 7, }, @@ -213,7 +213,7 @@ for (const scenario of scenarios) { expect(results[2].chunkRecord.endVersion).to.equal(24) expect(results[2].blobsToBackup).to.have.deep.members([ { - hash: blobHash.fromString('a'.repeat(16)), + hash: blobHashFromString('a'.repeat(16)), byteLength: 16, stringLength: 16, }, @@ -270,7 +270,7 @@ for (const scenario of scenarios) { { version: 11, blob: { - hash: blobHash.fromString('a'.repeat(7)), + hash: blobHashFromString('a'.repeat(7)), byteLength: 7, stringLength: 7, }, @@ -291,7 +291,7 @@ for (const scenario of scenarios) { { version: 21, blob: { - hash: blobHash.fromString('a'.repeat(16)), + hash: blobHashFromString('a'.repeat(16)), byteLength: 16, stringLength: 16, }, diff --git a/services/history-v1/test/acceptance/js/storage/blob_hash.test.js b/services/history-v1/test/acceptance/js/storage/blob_hash.test.js deleted file mode 100644 index 77e4cc3a0e..0000000000 --- a/services/history-v1/test/acceptance/js/storage/blob_hash.test.js +++ /dev/null @@ -1,15 +0,0 @@ -'use strict' - -const { expect } = require('chai') -const storage = require('../../../../storage') -const blobHash = storage.blobHash - -describe('blobHash', function () { - it('can hash non-ASCII strings', function () { - // checked with git hash-object - const testString = 'å\n' - const testHash = 'aad321caf77ca6c5ab09e6c638c237705f93b001' - - expect(blobHash.fromString(testString)).to.equal(testHash) - }) -}) diff --git a/services/history-v1/test/acceptance/js/storage/support/test_files.js b/services/history-v1/test/acceptance/js/storage/support/test_files.js index 5632aea753..2e507fb369 100644 --- a/services/history-v1/test/acceptance/js/storage/support/test_files.js +++ b/services/history-v1/test/acceptance/js/storage/support/test_files.js @@ -1,4 +1,5 @@ const path = require('node:path') +const { File } = require('overleaf-editor-core') exports.path = function (pathname) { return path.join(__dirname, '..', 'files', pathname) @@ -19,7 +20,7 @@ exports.NON_BMP_TXT_BYTE_LENGTH = 57 exports.NULL_CHARACTERS_TXT_HASH = '4227ca4e8736af63036e7457e2db376ddf7e5795' exports.NULL_CHARACTERS_TXT_BYTE_LENGTH = 3 -exports.EMPTY_FILE_HASH = 'e69de29bb2d1d6434b8b29ae775ad8c2e48c5391' +exports.EMPTY_FILE_HASH = File.EMPTY_FILE_HASH exports.EMPTY_FILE_BYTE_LENGTH = 0 // git hashes of some short strings for testing