From a21006b702fa4b70f9adf9efd8fbb7c81c3745a2 Mon Sep 17 00:00:00 2001 From: Carlos Scheidegger Date: Mon, 31 Mar 2025 16:52:38 -0400 Subject: [PATCH 1/3] do not allow !expr tags outside of knitr execution --- src/core/lib/yaml-validation/validator.ts | 67 ++++++++++++++++++- src/core/schema/validate-document.ts | 40 ++++++----- tests/docs/yaml/issue-11967.qmd | 5 ++ tests/smoke/smoke-all.test.ts | 10 ++- .../yaml-intelligence/issue-11967.test.ts | 27 ++++++++ 5 files changed, 131 insertions(+), 18 deletions(-) create mode 100644 tests/docs/yaml/issue-11967.qmd create mode 100644 tests/smoke/yaml-intelligence/issue-11967.test.ts diff --git a/src/core/lib/yaml-validation/validator.ts b/src/core/lib/yaml-validation/validator.ts index 8cc50ba5375..2d20942303e 100644 --- a/src/core/lib/yaml-validation/validator.ts +++ b/src/core/lib/yaml-validation/validator.ts @@ -30,12 +30,69 @@ import { import { resolveSchema } from "./resolve.ts"; -import { MappedString } from "../text-types.ts"; -import { createLocalizedError } from "./errors.ts"; +import { MappedString, StringMapResult } from "../text-types.ts"; +import { createLocalizedError, createSourceContext } from "./errors.ts"; import { InternalError } from "../error.ts"; +import { mappedIndexToLineCol } from "../mapped-text.ts"; +import { TidyverseError } from "../errors-types.ts"; //////////////////////////////////////////////////////////////////////////////// +function createNiceError(obj: { + violatingObject: AnnotatedParse; + source: MappedString; + message: string; +}): TidyverseError { + const { + violatingObject, + source, + message, + } = obj; + const locF = mappedIndexToLineCol(source); + + let location; + try { + location = { + start: locF(violatingObject.start), + end: locF(violatingObject.end), + }; + } catch (_e) { + location = { + start: { line: 0, column: 0 }, + end: { line: 0, column: 0 }, + }; + } + + const mapResult = source.map(violatingObject.start); + const fileName = mapResult ? mapResult.originalString.fileName : undefined; + return { + heading: message, + error: [], + info: {}, + fileName, + location: location!, + sourceContext: createSourceContext(violatingObject.source, { + start: violatingObject.start, + end: violatingObject.end, + }), + }; +} + +export class NoExprTag extends Error { + constructor(violatingObject: AnnotatedParse, source: MappedString) { + super(`Unexpected !expr tag`); + this.name = "NoExprTag"; + this.niceError = createNiceError({ + violatingObject, + source, + message: + "!expr tags are not allowed in Quarto outside of knitr code cells.", + }); + } + + niceError: TidyverseError; +} + class ValidationContext { instancePath: (number | string)[]; root: ValidationTraceNode; @@ -561,6 +618,12 @@ function validateObject( } } } + if ( + value.result && typeof value.result === "object" && + !Array.isArray(value.result) && value.result?.tag === "!expr" + ) { + throw new NoExprTag(value, value.source); + } throw new InternalError(`Couldn't locate key ${key}`); }; const inspectedProps: Set = new Set(); diff --git a/src/core/schema/validate-document.ts b/src/core/schema/validate-document.ts index 42e2276ff9f..2a99f6a9cfe 100644 --- a/src/core/schema/validate-document.ts +++ b/src/core/schema/validate-document.ts @@ -21,6 +21,7 @@ import { isObject } from "../lodash.ts"; import { getFrontMatterSchema } from "../lib/yaml-schema/front-matter.ts"; import { JSONValue, LocalizedError } from "../lib/yaml-schema/types.ts"; import { MappedString } from "../lib/mapped-text.ts"; +import { NoExprTag } from "../lib/yaml-validation/validator.ts"; export async function validateDocumentFromSource( src: MappedString, @@ -77,22 +78,31 @@ export async function validateDocumentFromSource( ) { const frontMatterSchema = await getFrontMatterSchema(); - await withValidator(frontMatterSchema, async (frontMatterValidator) => { - const fmValidation = await frontMatterValidator.validateParseWithErrors( - frontMatterText, - annotation, - "Validation of YAML front matter failed.", - errorFn, - reportOnce( - (err: TidyverseError) => - error(tidyverseFormatError(err), { colorize: false }), - reportSet, - ), - ); - if (fmValidation && fmValidation.errors.length) { - result.push(...fmValidation.errors); + try { + await withValidator(frontMatterSchema, async (frontMatterValidator) => { + const fmValidation = await frontMatterValidator + .validateParseWithErrors( + frontMatterText, + annotation, + "Validation of YAML front matter failed.", + errorFn, + reportOnce( + (err: TidyverseError) => + error(tidyverseFormatError(err), { colorize: false }), + reportSet, + ), + ); + if (fmValidation && fmValidation.errors.length) { + result.push(...fmValidation.errors); + } + }); + } catch (e) { + if (e.name === "NoExprTag") { + const err = e as NoExprTag; + error(tidyverseFormatError(err.niceError), { colorize: false }); + throw e; } - }); + } } } else { firstContentCellIndex = 0; diff --git a/tests/docs/yaml/issue-11967.qmd b/tests/docs/yaml/issue-11967.qmd new file mode 100644 index 00000000000..f54971efe2a --- /dev/null +++ b/tests/docs/yaml/issue-11967.qmd @@ -0,0 +1,5 @@ +--- +title: "Some title" +format: html +brand: !expr 'c("demo/_brand.yml")' +--- \ No newline at end of file diff --git a/tests/smoke/smoke-all.test.ts b/tests/smoke/smoke-all.test.ts index 9fe502a272a..ea9be8b2dd1 100644 --- a/tests/smoke/smoke-all.test.ts +++ b/tests/smoke/smoke-all.test.ts @@ -55,7 +55,15 @@ async function guessFormat(fileName: string): Promise { for (const cell of cells) { if (cell.cell_type === "raw") { const src = cell.source.value.replaceAll(/^---$/mg, ""); - const yaml = parse(src); + let yaml; + try { + yaml = parse(src); + } catch (e) { + if (e.message.includes("unknown tag")) { + // assume it's not necessary to guess the format + continue; + } + } if (yaml && typeof yaml === "object") { // deno-lint-ignore no-explicit-any const format = (yaml as Record).format; diff --git a/tests/smoke/yaml-intelligence/issue-11967.test.ts b/tests/smoke/yaml-intelligence/issue-11967.test.ts new file mode 100644 index 00000000000..b6f335f9a52 --- /dev/null +++ b/tests/smoke/yaml-intelligence/issue-11967.test.ts @@ -0,0 +1,27 @@ +/* + * issue-11967.test.ts + * + * Copyright (C) 2025 Posit Software, PBC + * + */ + +import { testQuartoCmd } from "../../test.ts"; +import { fileLoader } from "../../utils.ts"; +import { printsMessage } from "../../verify.ts"; + +const yamlDocs = fileLoader("yaml"); + +const testYamlValidationFails = (file: string) => { + testQuartoCmd( + "render", + [yamlDocs(file, "html").input, "--to", "html", "--quiet"], + [printsMessage({level: "ERROR", regex: /\!expr tags are not allowed in Quarto outside of knitr code cells/})], + ); +}; + +const files = [ + "issue-11967.qmd", +]; + +files.forEach(testYamlValidationFails); + From 65a8e11bc430f8aa3000eebdb22cd6c56fc8054a Mon Sep 17 00:00:00 2001 From: Carlos Scheidegger Date: Mon, 31 Mar 2025 16:55:20 -0400 Subject: [PATCH 2/3] changelog --- news/changelog-1.7.md | 1 + 1 file changed, 1 insertion(+) diff --git a/news/changelog-1.7.md b/news/changelog-1.7.md index d4f34a4e0ec..ca0472a5801 100644 --- a/news/changelog-1.7.md +++ b/news/changelog-1.7.md @@ -137,6 +137,7 @@ All changes included in 1.7: - ([#11606](https://github.com/quarto-dev/quarto-cli/discussions/11606)): Added a new `QUARTO_DOCUMENT_FILE` env var available to computation engine to the name of the file currently being rendered. - ([#11803](https://github.com/quarto-dev/quarto-cli/pull/11803)): Added a new CLI command `quarto call`. First users of this interface are the new `quarto call engine julia ...` subcommands. - ([#11951](https://github.com/quarto-dev/quarto-cli/issues/11951)): Raw LaTeX table without `tbl-` prefix label for using Quarto crossref are now correctly passed through unmodified. +- ([#11967](https://github.com/quarto-dev/quarto-cli/issues/11967)): Produce a better error message when YAML metadata with `!expr` tags are used outside of `knitr` code cells. - ([#12117](https://github.com/quarto-dev/quarto-cli/issues/12117)): Color output to stdout and stderr is now correctly rendered for `html` format in the Jupyter and Julia engines. - ([#12264](https://github.com/quarto-dev/quarto-cli/issues/12264)): Upgrade `dart-sass` to 1.85.1. - ([#11803](https://github.com/quarto-dev/quarto-cli/pull/11803)): Added a new CLI command `quarto call`. First users of this interface are the new `quarto call engine julia ...` subcommands. From d89ecf50b75de0bd13259aa5f6e26bb66739b47d Mon Sep 17 00:00:00 2001 From: Carlos Scheidegger Date: Mon, 31 Mar 2025 16:59:26 -0400 Subject: [PATCH 3/3] don't use chaining operator in code that runs in old chrome --- src/core/lib/yaml-validation/validator.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/lib/yaml-validation/validator.ts b/src/core/lib/yaml-validation/validator.ts index 2d20942303e..1342be3a8a6 100644 --- a/src/core/lib/yaml-validation/validator.ts +++ b/src/core/lib/yaml-validation/validator.ts @@ -620,7 +620,7 @@ function validateObject( } if ( value.result && typeof value.result === "object" && - !Array.isArray(value.result) && value.result?.tag === "!expr" + !Array.isArray(value.result) && value.result.tag === "!expr" ) { throw new NoExprTag(value, value.source); }