mirror of
https://github.com/yu-i-i/overleaf-cep.git
synced 2026-11-12 13:13:45 +01:00
[clsi] address zod violations (#36199)
* [editor-core] gracefully handle no-op TextOperation * [web] avoid sending sync to pdf requests without a ?file * [validation-tools] log deeper nested violations * [editor-core] fix round trip of EditNoOperation * [web] use custom error for sync to pdf w/o file, fix tests * [validation-tools] include request params in log msg for violation * Revert "[editor-core] gracefully handle no-op TextOperation" This reverts commit c1d37261d5db72b37dbf39428c54edcfb5423f41. * [web] gracefully handle rejected sync to pdf from context menu GitOrigin-RevId: 21de7dc96b23575b0d67b6eb0c68ed9c2f86292b
This commit is contained in:
@@ -35,7 +35,8 @@ class Operation {
|
||||
if (
|
||||
'textOperation' in raw ||
|
||||
'commentId' in raw ||
|
||||
'deleteComment' in raw
|
||||
'deleteComment' in raw ||
|
||||
'noOp' in raw
|
||||
) {
|
||||
return EditFileOperation.fromRaw(raw)
|
||||
}
|
||||
|
||||
@@ -78,4 +78,18 @@ describe('EditFileOperation', function () {
|
||||
expect(editFileOperation1).to.deep.equal(edit('foo.tex', ['x']))
|
||||
})
|
||||
})
|
||||
|
||||
describe('fromRaw / toRaw', function () {
|
||||
it('round-trips an edit no-op', function () {
|
||||
const editFileOperation = Operation.editFile(
|
||||
'foo.tex',
|
||||
EditOperationBuilder.fromJSON({ noOp: true })
|
||||
)
|
||||
const raw = editFileOperation.toRaw()
|
||||
expect(raw).to.eql({ pathname: 'foo.tex', noOp: true })
|
||||
|
||||
const roundTripped = Operation.fromRaw(raw)
|
||||
expect(roundTripped).to.deep.equal(editFileOperation)
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
@@ -24,6 +24,7 @@
|
||||
"zod-validation-error": "^4.0.1"
|
||||
},
|
||||
"devDependencies": {
|
||||
"@overleaf/logger": "workspace:*",
|
||||
"@types/express": "4.17.23",
|
||||
"@types/node": "^24.5.2",
|
||||
"eslint": "^10.4.0",
|
||||
|
||||
@@ -179,7 +179,12 @@ function sanitizeIssues(issues, depth = 0) {
|
||||
path: issue.path.map(seg => String(seg).slice(0, 64)).join('.'),
|
||||
message: String(issue.message).slice(0, 200),
|
||||
}
|
||||
if (issue.code === 'invalid_union' && depth < 2) {
|
||||
// Bounded by schema shape, not user input: each level corresponds to one
|
||||
// more union nested inside a union member (e.g. rawOperation -> rawFile
|
||||
// -> rawFileMetadata -> the linked-file provider discriminatedUnion), so
|
||||
// raising this cannot be exploited to grow log volume from adversarial
|
||||
// input the way an unbounded path/array traversal could.
|
||||
if (issue.code === 'invalid_union' && depth < 5) {
|
||||
sanitized.errors = issue.errors.map(nested =>
|
||||
sanitizeIssues(nested, depth + 1)
|
||||
)
|
||||
@@ -222,7 +227,7 @@ function logSchemaFailure(req, schema, kind, issues) {
|
||||
.join('\n')
|
||||
|
||||
logger.warn(
|
||||
{ location, kind, issues: sanitizedIssues, caller },
|
||||
{ location, kind, issues: sanitizedIssues, caller, req },
|
||||
'req-validation: request failed schema in log-only rollout'
|
||||
)
|
||||
}
|
||||
|
||||
@@ -8,6 +8,7 @@ import {
|
||||
resetReqValidationLoggingForTests,
|
||||
} from '../../../parseReq'
|
||||
import { zz } from '../../../zodHelpers'
|
||||
import serializers from '@overleaf/logger/serializers'
|
||||
|
||||
const RAW_BODY = Symbol.for('overleaf.lockdown.rawBody')
|
||||
const RAW_QUERY = Symbol.for('overleaf.lockdown.rawQuery')
|
||||
@@ -56,6 +57,28 @@ function lockedRequest({
|
||||
|
||||
type WarnCall = [Record<string, any>, string]
|
||||
|
||||
// Mirrors what a real bunyan logger does to each field before writing a log
|
||||
// line -- warnMock only records the raw ctx, so assertions on the logged
|
||||
// output must run it through the same serializers to see what would actually
|
||||
// reach the log (e.g. the req serializer drops req.body).
|
||||
function serializedLogOutput(calls: WarnCall[]) {
|
||||
return JSON.stringify(
|
||||
calls.map(([ctx, msg]) => [
|
||||
Object.fromEntries(
|
||||
Object.entries(ctx).map(([key, value]) => [
|
||||
key,
|
||||
key in serializers
|
||||
? (serializers as Record<string, (v: unknown) => unknown>)[key](
|
||||
value
|
||||
)
|
||||
: value,
|
||||
])
|
||||
),
|
||||
msg,
|
||||
])
|
||||
)
|
||||
}
|
||||
|
||||
describe('parseReq log-only rollout', () => {
|
||||
// This block intentionally runs before anything else in the file calls
|
||||
// setLogger(), so the module-level logger is still unset here.
|
||||
@@ -296,7 +319,7 @@ describe('parseReq log-only rollout', () => {
|
||||
|
||||
parseReq(req, schema, { logOnly: true })
|
||||
|
||||
expect(JSON.stringify(warnMock.mock.calls)).not.toContain(
|
||||
expect(serializedLogOutput(warnMock.mock.calls)).not.toContain(
|
||||
'SECRET_VALUE_123'
|
||||
)
|
||||
})
|
||||
@@ -311,7 +334,7 @@ describe('parseReq log-only rollout', () => {
|
||||
|
||||
parseReq(req, schema, { logOnly: true })
|
||||
|
||||
const logged = JSON.stringify(warnMock.mock.calls)
|
||||
const logged = serializedLogOutput(warnMock.mock.calls)
|
||||
expect(logged).toContain('extraSecretField')
|
||||
expect(logged).not.toContain('TOP_SECRET_VALUE')
|
||||
})
|
||||
@@ -325,7 +348,7 @@ describe('parseReq log-only rollout', () => {
|
||||
|
||||
parseReq(req, schema, { logOnly: true })
|
||||
|
||||
const logged = JSON.stringify(warnMock.mock.calls)
|
||||
const logged = serializedLogOutput(warnMock.mock.calls)
|
||||
expect(logged).toContain(longKey.slice(0, 64))
|
||||
expect(logged).not.toContain(longKey)
|
||||
})
|
||||
@@ -367,10 +390,50 @@ describe('parseReq log-only rollout', () => {
|
||||
expect(Array.isArray(unionIssue.errors)).toBe(true)
|
||||
expect(unionIssue.errors.length).toBeGreaterThan(0)
|
||||
|
||||
expect(JSON.stringify(warnMock.mock.calls)).not.toContain(
|
||||
expect(serializedLogOutput(warnMock.mock.calls)).not.toContain(
|
||||
'UNION_SECRET_VALUE'
|
||||
)
|
||||
})
|
||||
|
||||
it('expands a union nested 3 levels deep inside other unions', () => {
|
||||
// Mirrors overleaf-editor-core's rawOperation -> rawFile ->
|
||||
// rawFileMetadata shape: a union of objects, one of whose fields is
|
||||
// itself a union of objects, one of whose fields is itself a union.
|
||||
const level3 = z.union([
|
||||
z.strictObject({ a: z.literal('a') }),
|
||||
z.strictObject({ b: z.literal('b') }),
|
||||
])
|
||||
const level2 = z.union([
|
||||
z.strictObject({ inner: level3 }),
|
||||
z.strictObject({ other: z.string() }),
|
||||
])
|
||||
const level1 = z.union([
|
||||
z.strictObject({ mid: level2 }),
|
||||
z.strictObject({ different: z.string() }),
|
||||
])
|
||||
const req = {
|
||||
body: { value: { mid: { inner: { c: 'nope' } } } },
|
||||
} as Request
|
||||
const schema = z.object({ body: z.object({ value: level1 }) })
|
||||
|
||||
parseReq(req, schema, { logOnly: true })
|
||||
|
||||
const [ctx] = warnMock.mock.calls[0] as WarnCall
|
||||
// Walk down: top union (value) -> level1 branch (mid) -> level2
|
||||
// union (inner) -> level3 union, whose own branch errors (unknown
|
||||
// keys "a"/"b" not satisfied by "c") must have survived.
|
||||
const top = ctx.issues.find((i: any) => i.code === 'invalid_union')
|
||||
const level1Errors = top.errors[0] // the "mid" branch
|
||||
const level2Issue = level1Errors.find(
|
||||
(i: any) => i.code === 'invalid_union'
|
||||
)
|
||||
const level2Errors = level2Issue.errors[0] // the "inner" branch
|
||||
const level3Issue = level2Errors.find(
|
||||
(i: any) => i.code === 'invalid_union'
|
||||
)
|
||||
expect(level3Issue.errors).toBeDefined()
|
||||
expect(level3Issue.errors.length).toBeGreaterThan(0)
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
|
||||
@@ -583,7 +583,7 @@ const _CompileController = {
|
||||
|
||||
async proxySyncCode(req, res) {
|
||||
const { file, line, column } = req.query
|
||||
if (file == null) {
|
||||
if (!file) {
|
||||
throw new Error('missing file parameter')
|
||||
}
|
||||
// Check that we are dealing with a simple file path (this is not
|
||||
|
||||
@@ -2066,6 +2066,7 @@
|
||||
"syncing": "",
|
||||
"synctex_error_recompile_and_try_again": "",
|
||||
"synctex_failed": "",
|
||||
"synctex_switch_files_and_try_again": "",
|
||||
"syntax_checks": "",
|
||||
"syntax_validation": "",
|
||||
"tab_connecting": "",
|
||||
|
||||
@@ -28,6 +28,12 @@ export const SynctexRequestErrorToast = () => {
|
||||
return <span>{t('synctex_error_recompile_and_try_again')}</span>
|
||||
}
|
||||
|
||||
export const SynctexNoFileToast = () => {
|
||||
const { t } = useTranslation()
|
||||
|
||||
return <span>{t('synctex_switch_files_and_try_again')}</span>
|
||||
}
|
||||
|
||||
const generators: GlobalToastGeneratorEntry[] = [
|
||||
{
|
||||
key: 'synctex:file-error',
|
||||
@@ -49,6 +55,16 @@ const generators: GlobalToastGeneratorEntry[] = [
|
||||
isDismissible: true,
|
||||
}),
|
||||
},
|
||||
{
|
||||
key: 'synctex:no-file-error',
|
||||
generator: () => ({
|
||||
content: <SynctexNoFileToast />,
|
||||
type: 'warning',
|
||||
autoHide: true,
|
||||
delay: 4000,
|
||||
isDismissible: true,
|
||||
}),
|
||||
},
|
||||
]
|
||||
|
||||
export default generators
|
||||
@@ -72,3 +88,13 @@ export const showSynctexRequestErrorToast = () => {
|
||||
})
|
||||
)
|
||||
}
|
||||
|
||||
export const showSynctexNoFileToast = () => {
|
||||
window.dispatchEvent(
|
||||
new CustomEvent('ide:show-toast', {
|
||||
detail: {
|
||||
key: 'synctex:no-file-error',
|
||||
},
|
||||
})
|
||||
)
|
||||
}
|
||||
|
||||
@@ -18,11 +18,12 @@ import { isValidTeXFile } from '@/main/is-valid-tex-file'
|
||||
import { PdfScrollPosition } from '@/shared/hooks/use-pdf-scroll-position'
|
||||
import {
|
||||
showFileErrorToast,
|
||||
showSynctexNoFileToast,
|
||||
showSynctexRequestErrorToast,
|
||||
} from '@/features/pdf-preview/components/synctex-toasts'
|
||||
|
||||
export default function useSynctex(): {
|
||||
syncToPdf: () => void
|
||||
syncToPdf: () => boolean
|
||||
syncToCode: ({ visualOffset }: { visualOffset?: number }) => void
|
||||
syncToPdfInFlight: boolean
|
||||
syncToCodeInFlight: boolean
|
||||
@@ -150,18 +151,25 @@ export default function useSynctex(): {
|
||||
|
||||
const syncToPdf = useCallback(() => {
|
||||
const file = getCurrentFilePath()
|
||||
if (!file) {
|
||||
showSynctexNoFileToast()
|
||||
return false
|
||||
}
|
||||
|
||||
if (cursorPositionRef.current) {
|
||||
const { row, column } = cursorPositionRef.current
|
||||
|
||||
const params = new URLSearchParams({
|
||||
file: file ?? '',
|
||||
file,
|
||||
line: String(row + 1),
|
||||
column: String(column),
|
||||
}).toString()
|
||||
|
||||
goToPdfLocation(params)
|
||||
return true
|
||||
}
|
||||
|
||||
return false
|
||||
}, [getCurrentFilePath, goToPdfLocation])
|
||||
|
||||
useEventListener(
|
||||
|
||||
@@ -221,10 +221,14 @@ export const useContextMenuItems = () => {
|
||||
method: 'editor-context-menu',
|
||||
direction: 'code-location-in-pdf',
|
||||
})
|
||||
requestedPdfSyncRef.current = true
|
||||
syncToPdf()
|
||||
view.focus()
|
||||
}, [syncToPdf, view, changeLayout, isEditorOnly])
|
||||
|
||||
if (syncToPdf()) {
|
||||
requestedPdfSyncRef.current = true
|
||||
view.focus()
|
||||
} else {
|
||||
closeMenu()
|
||||
}
|
||||
}, [syncToPdf, view, changeLayout, isEditorOnly, closeMenu])
|
||||
|
||||
return {
|
||||
closeMenu,
|
||||
|
||||
@@ -2651,6 +2651,7 @@
|
||||
"syncing": "Syncing",
|
||||
"synctex_error_recompile_and_try_again": "That didn’t work. Recompile and try again.",
|
||||
"synctex_failed": "Couldn’t find the corresponding source file",
|
||||
"synctex_switch_files_and_try_again": "That didn’t work. Try switching files and try again.",
|
||||
"syntax_checks": "Syntax checks",
|
||||
"syntax_validation": "Code check",
|
||||
"tab_connecting": "Connecting with the editor",
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import PdfSynctexControls from '../../../../frontend/js/features/pdf-preview/components/pdf-synctex-controls'
|
||||
import { GlobalToasts } from '../../../../frontend/js/features/ide-react/components/global-toasts'
|
||||
import { cloneDeep } from 'lodash'
|
||||
import { useDetachCompileContext as useCompileContext } from '../../../../frontend/js/shared/context/detach-compile-context'
|
||||
import { useFileTreeData } from '../../../../frontend/js/shared/context/file-tree-data-context'
|
||||
@@ -76,11 +77,11 @@ const WithSelectedEntities = ({
|
||||
return null
|
||||
}
|
||||
|
||||
function mockProviders() {
|
||||
function mockProviders(currentDocumentId: string | null = '_root_doc_id') {
|
||||
return {
|
||||
EditorOpenDocProvider: makeEditorOpenDocProvider({
|
||||
openDocName: 'main.tex',
|
||||
currentDocumentId: null,
|
||||
currentDocumentId: currentDocumentId as any,
|
||||
currentDocument: {
|
||||
doc_id: 'test-doc',
|
||||
getSnapshot: () => 'some doc content',
|
||||
@@ -162,6 +163,50 @@ describe('<PdfSynctexControls/>', function () {
|
||||
)
|
||||
})
|
||||
|
||||
it('shows an error toast and does not sync when there is no resolvable file path', function () {
|
||||
cy.interceptCompile()
|
||||
|
||||
const scope = mockScope()
|
||||
const providers = mockProviders(null)
|
||||
|
||||
cy.intercept(
|
||||
{ pathname: '/project/*/sync/code' },
|
||||
cy.spy().as('syncCodeRequest')
|
||||
)
|
||||
|
||||
cy.mount(
|
||||
<EditorProviders scope={scope} providers={providers}>
|
||||
<WithPosition mockPosition={mockPosition} />
|
||||
<WithSelectedEntities mockSelectedEntities={mockSelectedEntities} />
|
||||
<PdfSynctexControls />
|
||||
<GlobalToasts />
|
||||
</EditorProviders>
|
||||
)
|
||||
|
||||
cy.waitForCompile()
|
||||
|
||||
// mock editor cursor position update
|
||||
cy.window().then(win => {
|
||||
win.dispatchEvent(
|
||||
new CustomEvent('cursor:editor:update', {
|
||||
detail: { row: 100, column: 10 },
|
||||
})
|
||||
)
|
||||
})
|
||||
|
||||
cy.findByRole('button', { name: 'Go to code location in PDF' }).click()
|
||||
|
||||
cy.findByText(
|
||||
'That didn’t work. Try switching files and try again.'
|
||||
).should('exist')
|
||||
|
||||
cy.findByRole('button', { name: 'Go to code location in PDF' }).should(
|
||||
'not.be.disabled'
|
||||
)
|
||||
|
||||
cy.get('@syncCodeRequest').should('not.have.been.called')
|
||||
})
|
||||
|
||||
it('disables button when multiple entities are selected', function () {
|
||||
cy.interceptCompile()
|
||||
|
||||
|
||||
+68
@@ -65,6 +65,34 @@ const MockFileTreeDataProvider: FC<React.PropsWithChildren> = ({
|
||||
value={
|
||||
{
|
||||
selectedEntities: [{ type: 'doc', id: '_root_doc_id' } as any],
|
||||
fileTreeData: {
|
||||
_id: 'root-folder-id',
|
||||
name: 'rootFolder',
|
||||
docs: [{ _id: docId, name: 'test.tex' }],
|
||||
folders: [],
|
||||
fileRefs: [],
|
||||
},
|
||||
} as any
|
||||
}
|
||||
>
|
||||
{children}
|
||||
</FileTreeDataContext.Provider>
|
||||
)
|
||||
|
||||
const MockEmptyFileTreeDataProvider: FC<React.PropsWithChildren> = ({
|
||||
children,
|
||||
}) => (
|
||||
<FileTreeDataContext.Provider
|
||||
value={
|
||||
{
|
||||
selectedEntities: [{ type: 'doc', id: '_root_doc_id' } as any],
|
||||
fileTreeData: {
|
||||
_id: 'root-folder-id',
|
||||
name: 'rootFolder',
|
||||
docs: [],
|
||||
folders: [],
|
||||
fileRefs: [],
|
||||
},
|
||||
} as any
|
||||
}
|
||||
>
|
||||
@@ -1008,6 +1036,46 @@ describe('editor context menu', { scrollBehavior: false }, function () {
|
||||
})
|
||||
})
|
||||
|
||||
it('should close the menu and show an error toast when there is no resolvable file path', function () {
|
||||
const scope = mockScope()
|
||||
|
||||
cy.intercept(
|
||||
'GET',
|
||||
'/project/*/sync/code*',
|
||||
cy.spy().as('syncToPdfRequest')
|
||||
)
|
||||
|
||||
cy.mount(
|
||||
<TestContainer>
|
||||
<EditorProviders
|
||||
scope={scope}
|
||||
providers={{
|
||||
DetachCompileProvider: MockDetachCompileProvider,
|
||||
FileTreeDataProvider: MockEmptyFileTreeDataProvider,
|
||||
}}
|
||||
>
|
||||
<GlobalToasts />
|
||||
<CodeMirrorEditor />
|
||||
</EditorProviders>
|
||||
</TestContainer>
|
||||
)
|
||||
|
||||
cy.get('.cm-line').eq(10).rightclick()
|
||||
|
||||
cy.findByRole('menu').within(() => {
|
||||
cy.findByRole('menuitem', { name: /jump to location in pdf/i }).click()
|
||||
})
|
||||
|
||||
cy.findByRole('menu').should('not.exist')
|
||||
|
||||
cy.get('.global-toasts').should(
|
||||
'contain.text',
|
||||
'That didn’t work. Try switching files and try again.'
|
||||
)
|
||||
|
||||
cy.get('@syncToPdfRequest').should('not.have.been.called')
|
||||
})
|
||||
|
||||
it('should hide button when visual preview is enabled', function () {
|
||||
window.metaAttributesCache.set('ol-splitTestVariants', {
|
||||
'editor-context-menu': 'enabled',
|
||||
|
||||
@@ -7870,6 +7870,7 @@ __metadata:
|
||||
version: 0.0.0-use.local
|
||||
resolution: "@overleaf/validation-tools@workspace:libraries/validation-tools"
|
||||
dependencies:
|
||||
"@overleaf/logger": "workspace:*"
|
||||
"@overleaf/o-error": "workspace:*"
|
||||
"@types/express": "npm:4.17.23"
|
||||
"@types/node": "npm:^24.5.2"
|
||||
|
||||
Reference in New Issue
Block a user