fix: escape special characters in query-dsl page refs - #13541
tiensonqin wants to merge 8 commits into
Conversation
ff00b3e to
b7bae86
Compare
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Before / after UX verificationBefore: master After: Desktop quote-in-title flow passed; loaded revision verified. Other parser edge cases were not exercised in this browser recording. |
pre-transform quoted [[page]] titles without escaping, so renaming a tag to include ", \, [[, or ]] made existing queries fail to parse. Co-authored-by: Tienson Qin <tiensonqin@gmail.com>
Avoid embedding ]] inside string literals that confuse the CLJS reader and clj-kondo when asserting page refs with bracket titles. Co-authored-by: Tienson Qin <tiensonqin@gmail.com>
Keep titles with mid-title ]] or nested [[...]] intact, and stop quoting a trailing vector ] as part of the page name. Co-authored-by: Tienson Qin <tiensonqin@gmail.com>
Close the pre-transform-test form so later deftest vars register instead of nesting inside it. Co-authored-by: Tienson Qin <tiensonqin@gmail.com>
Treat only a lone vector ] as a closer, and stop leftover-]] lookahead at the current list so later titles cannot steal an earlier between date. Co-authored-by: Tienson Qin <tiensonqin@gmail.com>
A ) only ends leftover-]] lookahead when the next token is a sibling list, so titles like A]] B) C stay one page-ref. Co-authored-by: Tienson Qin <tiensonqin@gmail.com>
After a form close, [[ starts the next argument, so a later title ending in ]] cannot swallow an earlier between date. Co-authored-by: Tienson Qin <tiensonqin@gmail.com>
812ef62 to
c83036f
Compare
Query DSL strings that reference pages by title (e.g. (tags [[Project"]])) break cljs.reader when a title contains special characters like " or \\. Instead of hardening the title-form reader path, keep page refs in their stored [[uuid]] form end to end: - execution reads :block/raw-title (the stored form) instead of the display-substituted :block/title (query-result, flashcards, query builder) - new query value blocks write [[uuid]] refs: pipeline tag-query sync, create-property-text-block!, and the file-graph exporter - dsl clauses resolve [[uuid]] args via get-page (tags/page/between/property) - display still shows titles via canonical-block's substituted :block/title - pre-transform keeps a simplified scanner for legacy title-form queries Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Devin Review found 2 new potential issues.
1 flag not posted on this PR by your GitHub settings — view it in Devin Review. (Configure)
| (= \) (nth s i)) | ||
| false |
There was a problem hiding this comment.
🟡 Parenthesized page titles break query parsing
For (tags [[A]] B) C]]), unmatched-page-ref-close? stops at the title's ) before seeing its final ]]. The query splits the page title at A, leaving invalid trailing text instead of matching A]] B) C.
Learn more
The scanner decides whether a candidate ]] closes a page reference by searching ahead for another unmatched ]]. Stopping that search at every ) assumes parentheses cannot occur in page titles, even when a later closing pair is part of the same title. Such page names are valid, and queries using their rendered references no longer parse.
Example: A tag titled A]] B) C appears as (tags [[A]] B) C]]). The scanner quotes only [[A]] and leaves B) C]]) as query syntax rather than matching the tag.
Recommended fix: Distinguish a parenthesis inside a reference title from a DSL form closer during lookahead, and restore coverage for ]] followed by ) and more title text.
Was this helpful? React with 👍 or 👎 to provide feedback.
| (if (string/includes? query "[[") | ||
| (let [ref-pages (keep (fn [[_ page-name]] | ||
| (when-let [id (or (get @page-names-to-uuids page-name) | ||
| (get @page-names-to-uuids | ||
| (common-util/page-name-sanity-lc page-name)))] | ||
| {:block/title page-name :block/uuid id})) | ||
| (re-seq page-ref/page-ref-re query))] | ||
| (if (seq ref-pages) | ||
| (db-content/title-ref->id-ref query ref-pages {:replace-tag? false}) |
There was a problem hiding this comment.
🟡 Quoted query text changes during import
When an imported query contains a quoted [[page]] literal, query-refs->id-refs replaces it with [[uuid]]. Text searches and advanced query string predicates then search for different content, changing their results.
Learn more
The importer now converts all regex-matched page references in simple, advanced, and cards query text to IDs. This includes matches inside quoted query strings. Quoted strings in DSL property or content searches, and string constants in advanced Datalog queries, can be literal text rather than page arguments. Replacing those strings changes the stored query's meaning.
Example: An advanced query whose predicate compares a title against "[[Foo]]" is imported while a Foo page exists. The string becomes "[[<Foo's UUID>]]", so the predicate no longer matches the original title.
Recommended fix: Parse the query format and convert only syntactic page-reference operands, preserving quoted string literals. Add tests for quoted refs in advanced queries and literal-content searches.
Was this helpful? React with 👍 or 👎 to provide feedback.
|
End-to-end verified on the Desktop dev build (Electron, db graph) — tag created, block tagged, query node running Query results — tag renamed to The What was verified
One unrelated pre-existing issue noticed while testing: |
This branch has not been deployed
3 participants
Summary
Fixes logseq/db-test#1374
Query DSL strings reference pages by title —
(tags [[Project"]]). After a tag was renamed to a title containing",\,[[, or]],pre-transformre-rendered the stored query with the raw title andcljs.readercrashed on unmatched delimiters.Instead of hardening the reader path against arbitrary title text, this keeps page refs in their stored
[[uuid]]form end to end — uuid characters are reader-safe, so the crash case can't occur.query-result,flashcards, and the query builder now use:block/raw-title(the stored[[uuid]]form) instead of the display-substituted:block/title. Rendering continues to show titles viacanonical-block's substituted:block/title.[[uuid]].ensure-query-property-on-tag-additions(pipeline),create-property-text-block!, and the file-graph exporter'shandle-queriesruntitle-ref->id-refon query values before storing.[[uuid]]args.tags,page,between/timestamps, and(property k v)resolve ref args throughldb/get-page(which already accepts uuid strings);(property ... "2 [[uuid]]")keeps quoted values literal.pre-transformkeeps a simplified scanner (pr-strquoting + scan for the terminating]]) as the fallback for legacy stored queries that still contain title-form refs and for hand-written ones.Tests:
src/test/frontend/db/query_dsl_test.cljs— uuid-form args for[[uuid]],(tags),(page),(between),(property), plus the remaining title-form casesdeps/outliner/test/logseq/outliner/property_test.cljs— asserts query property values are stored with[[uuid]]refsdeps/db/test/logseq/db/frontend/query_dsl_test.cljs— trimmed to corepre-transformcasesLink to Devin session: https://app.devin.ai/sessions/89ffdb2b39ce45469c10960d91f6ecc7
Open in Devin Desktop: https://app.devin.ai/desktop/session/89ffdb2b39ce45469c10960d91f6ecc7?variant=devin
Requested by: @megayu