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 5c75a509d0..497b054ea8 100644 Binary files a/libraries/overleaf-editor-core/test/unit/build_set_content_operations.test.js and b/libraries/overleaf-editor-core/test/unit/build_set_content_operations.test.js differ 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