Repository navigation
fix: escape preview URLs and resolve Office schema conflicts - #795
Conversation
…vent SyntaxError on apostrophe
…ent SyntaxError on apostrophe
…ent SyntaxError on apostrophe
…revent SyntaxError on apostrophe
…ent SyntaxError on apostrophe
…vent SyntaxError on apostrophe
…ent SyntaxError on apostrophe
…ent SyntaxError on apostrophe
…o prevent SyntaxError on apostrophe
… prevent SyntaxError on apostrophe
…ent SyntaxError on apostrophe
…vent SyntaxError on apostrophe
…event SyntaxError on apostrophe
…ent SyntaxError on apostrophe
…ped in preview template
klboke
left a comment
There was a problem hiding this comment.
The ?js_string change fixes the PDF reproduction: rendering pdf.ftl with a URL containing Int'l emits Int\'l in the JavaScript string. There is still a gap in two of the formats this PR claims to fix.
In the default office.type.web=web path, OfficeFilePreviewImpl passes both XLSX (pdfUrl, line 78) and CSV (csvUrl, line 82) through KkFileUtils.htmlEscape(url) before rendering officeweb.ftl and csv.ftl. For a source URL containing Int'l, that produces Int'l. Applying ?js_string to this already HTML-escaped value leaves ' unchanged; character references are not decoded inside a <script> element. The JavaScript therefore requests the wrong URL even though it no longer has a syntax error. I reproduced the model-to-template output as var url = 'https://files.example/Int'l/report%20one.xlsx';.
Please handle the URL according to its output context so the JavaScript string retains the original apostrophe, and add a rendered-output regression test for PDF, CSV, and XLSX. The new test only looks for ?js_string in template source, so it cannot catch this failure.
klboke
left a comment
There was a problem hiding this comment.
Re-reviewed the latest head 1acd4e4. The earlier CSV/XLSX blocker is resolved: OfficeFilePreviewImpl retains the source URL, and the selected FreeMarker templates apply JavaScript-string escaping. The new regression test runs the actual PDF/CSV/XLSX preview methods and renders each template, verifying the apostrophe and query string survive correctly.
The E2E runs also exposed a pre-existing POI schema conflict (the same NoSuchFieldError occurred in master Nightly run 37669089254 before these changes). The separate dependency commit removes legacy ooxml-schemas:1.1 from the XDocReport paths. The new tests detect duplicate definitions and exercise real DOCX/PPTX extraction and the preview password check; Maven now resolves only the matching POI schemas.
Validation: all 16 targeted tests pass with the three preview fixes combined locally. The latest PR E2E run passes all 27 tests, including the previously failing DOCX/PPTX paths and both image-title modes. No remaining code blockers.
Fixes #790.
Preview URLs containing an apostrophe could terminate an inline JavaScript string and prevent the viewer from loading. Apply FreeMarker's
?js_stringwhen emitting the URL in all 14 affected preview templates. Retain the original XLSX/CSV URL in the model so HTML entities such as'do not become part of the fetch URL.A separate follow-up commit resolves the existing Office dependency conflict exposed by E2E: XDocReport brought in
ooxml-schemas:1.1alongside POI 5.2.5'spoi-ooxml-lite. Both defined the same schema classes, and loading the old definitions caused DOCX/PPTX previews to throwNoSuchFieldError: ... DocumentFactory Factory. The same failure occurred in the pre-change master Nightly run. Exclude the legacy schema dependency from the XDocReport paths and keep POI's matching schemas.Validation:
PdfViewerCompatibilityTestsverifies JavaScript URL escaping across all 14 templates.PreviewUrlEscapingTestsinvokes the actual PDF/CSV/XLSX preview services and renders their selected templates, covering apostrophes and query-string ampersands. The suite reproduced CSV/XLSX failures before the model fix.OfficeSchemaCompatibilityTestsdetects duplicate schema definitions and creates/reads real DOCX and PPTX files through POI and the preview password check. The duplicate-schema assertion failed before the dependency exclusions; all three tests pass after them.ooxml-schemasartifact.