Skip to content

Commit 736b949

Browse files
fix(typst): clean up latent bugs in CSS translator
While writing tests for #14460, found and fixed five latent bugs in src/resources/filters/modules/typst_css.lua: - parse_color: brand-color reference path (var(--brand-NAME)) crashed when NAME contained digits. The capture class [%a--]* excluded %d, so e.g. var(--brand-red-50) failed to match and fell into a broken error branch that concatenated an undefined `v` and `return null`. Widened the class to [%w-] and fixed the error branch to use the actual color string and return nil. - output_color: implicit global `zero` (missing `local`) leaked from the no-color-with-opacity path. Made it local. - translate_font_weight: passed `null` (an undefined global -> nil) to output_warning, silently bypassing any user-supplied warnings collector. Changed to forward `warnings`. - consume_width / consume_style: `fbeg, fend = ...` without `local` leaked them as globals on every call. Added `local`. Each fix is covered by a regression test in tests/unit-lua/typst-css.test.lua.
1 parent dad7821 commit 736b949

3 files changed

Lines changed: 91 additions & 13 deletions

File tree

news/changelog-1.10.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ All changes included in 1.10:
1616
### `typst`
1717

1818
- ([#14261](https://github.com/quarto-dev/quarto-cli/issues/14261)): Fix theorem/example block titles containing inline code producing invalid Typst markup when syntax highlighting is applied.
19-
- ([#14460](https://github.com/quarto-dev/quarto-cli/issues/14460)): Fix CSS `border` and `border-color` declarations losing tokens that precede an `rgb()`/`rgba()` color (e.g. `border: 0px solid rgb(255, 0, 0)` rendering as a 2.25pt border instead of being suppressed).
19+
- ([#14460](https://github.com/quarto-dev/quarto-cli/issues/14460)): Fix CSS `border` and `border-color` declarations losing tokens that precede an `rgb()`/`rgba()` color (e.g. `border: 0px solid rgb(255, 0, 0)` rendering as a 2.25pt border instead of being suppressed). Also fixes `var(--brand-NAME)` references crashing the Typst CSS translator when `NAME` contained digits (e.g. `--brand-red-50`).
2020

2121
### `revealjs`
2222

src/resources/filters/modules/typst_css.lua

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -305,12 +305,14 @@ local function parse_color(color, warnings)
305305
elseif color:find '^rgb%(' or color:find '^rgba%(' then
306306
return parse_rgb(color)
307307
elseif color:find '^var%(%-%-brand%-' then
308-
local colorName = color:match '^var%(%-%-brand%-([%a--]*)%)'
308+
-- [%w-] (alphanumerics + hyphen) so names like `red-50` parse. Bare
309+
-- [%a--] excluded digits, breaking any brand name with a number.
310+
local colorName = color:match '^var%(%-%-brand%-([%w-]*)%)'
309311
if not colorName then
310-
output_warning(warnings, 'invalid brand color reference ' .. v)
311-
return null
312+
output_warning(warnings, 'invalid brand color reference ' .. color)
313+
return nil
312314
end
313-
return colorName and {
315+
return {
314316
type = 'brand',
315317
value = colorName
316318
}
@@ -332,7 +334,7 @@ local function output_color(color, opacity, warnings)
332334
quarto.log.debug('output_color input', color, opacity)
333335
if opacity then
334336
if not color then
335-
zero = {
337+
local zero = {
336338
unit = 'int',
337339
value = 0
338340
}
@@ -627,10 +629,10 @@ local function translate_font_weight(w, warnings)
627629
if tcontains(same_weights, w) then
628630
return w
629631
end
630-
if tcontains(dashed_weights, w) then
632+
if tcontains(dashed_weights, w) then
631633
return w:gsub('-', '')
632634
else
633-
output_warning(null, 'invalid font weight ' .. tostring(w))
635+
output_warning(warnings, 'invalid font weight ' .. tostring(w))
634636
return nil
635637
end
636638
end
@@ -716,14 +718,14 @@ local function translate_border(v, warnings)
716718
end
717719

718720
local function consume_width(s, start, warnings)
719-
fbeg, fend = s:find('%S+', start)
720-
local term = s:sub(fbeg, fend)
721-
local thickness = translate_border_width(term, warnings)
722-
return thickness, fend + 1
721+
local fbeg, fend = s:find('%S+', start)
722+
local term = s:sub(fbeg, fend)
723+
local thickness = translate_border_width(term, warnings)
724+
return thickness, fend + 1
723725
end
724726

725727
local function consume_style(s, start, warnings)
726-
fbeg, fend = s:find('%S+', start)
728+
local fbeg, fend = s:find('%S+', start)
727729
local term = s:sub(fbeg, fend)
728730
local dash = translate_border_style(term, warnings)
729731
return dash, fend + 1

tests/unit-lua/typst-css.test.lua

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,4 +150,80 @@ function TestTranslateLength:testPassthroughPt()
150150
lu.assertEquals(typst_css.translate_length('12pt', new_warnings()), '12pt')
151151
end
152152

153+
-- Bug A: output_color creates `zero` as an implicit global on the no-color
154+
-- path because the local keyword is missing.
155+
TestOutputColorGlobalLeak = {}
156+
157+
function TestOutputColorGlobalLeak:setUp()
158+
-- Make sure no prior test poisoned this global.
159+
rawset(_G, 'zero', nil)
160+
end
161+
162+
function TestOutputColorGlobalLeak:testNoColorWithOpacityDoesNotLeakGlobal()
163+
typst_css.output_color(nil, { unit = 'percent', value = 50 }, new_warnings())
164+
lu.assertNil(rawget(_G, 'zero'),
165+
'output_color leaked an implicit global "zero"')
166+
end
167+
168+
-- Bug B+C: parse_color of a brand reference whose name contains digits
169+
-- (e.g. `--brand-red-50`) currently crashes because:
170+
-- (B) the [%a--]* class only allows alphas + hyphens, so the capture fails
171+
-- and the function falls into the error branch
172+
-- (C) the error branch concatenates a never-defined `v` and returns
173+
-- `null` (also undefined)
174+
TestParseColorBrand = {}
175+
176+
function TestParseColorBrand:testBrandNameWithDigitsParses()
177+
local c = typst_css.parse_color('var(--brand-red-50)', new_warnings())
178+
lu.assertNotIsNil(c)
179+
lu.assertEquals(c.type, 'brand')
180+
lu.assertEquals(c.value, 'red-50')
181+
end
182+
183+
function TestParseColorBrand:testMalformedBrandRefReturnsNilNoCrash()
184+
-- Anything matching the var(--brand- prefix but failing the name capture
185+
-- must not throw a Lua error; it should warn and return nil.
186+
local warnings = new_warnings()
187+
local ok, c = pcall(typst_css.parse_color, 'var(--brand-!!)', warnings)
188+
lu.assertTrue(ok,
189+
'parse_color crashed on a malformed brand ref: ' .. tostring(c))
190+
lu.assertNil(c)
191+
end
192+
193+
-- Bug D: translate_font_weight passes `null` (an undefined global -> nil)
194+
-- to output_warning instead of `warnings`, so the user-supplied warnings
195+
-- collector is silently bypassed.
196+
TestTranslateFontWeight = {}
197+
198+
function TestTranslateFontWeight:testInvalidWeightWarnsToCollector()
199+
local warnings = new_warnings()
200+
local result = typst_css.translate_font_weight('not-a-weight', warnings)
201+
lu.assertNil(result)
202+
lu.assertEquals(#warnings, 1,
203+
'expected the invalid-weight warning to land in the collector, ' ..
204+
'but the collector saw ' .. #warnings .. ' warning(s)')
205+
end
206+
207+
-- Bug E: consume_width and consume_style write `fbeg, fend = ...` without
208+
-- `local`, leaking them as globals. (consume_color and translate_border
209+
-- correctly declare them local.)
210+
TestConsumeNoGlobalLeak = {}
211+
212+
function TestConsumeNoGlobalLeak:setUp()
213+
rawset(_G, 'fbeg', nil)
214+
rawset(_G, 'fend', nil)
215+
end
216+
217+
function TestConsumeNoGlobalLeak:testConsumeWidthDoesNotLeak()
218+
typst_css.consume_width('2px', 1, new_warnings())
219+
lu.assertNil(rawget(_G, 'fbeg'), 'consume_width leaked global "fbeg"')
220+
lu.assertNil(rawget(_G, 'fend'), 'consume_width leaked global "fend"')
221+
end
222+
223+
function TestConsumeNoGlobalLeak:testConsumeStyleDoesNotLeak()
224+
typst_css.consume_style('solid', 1, new_warnings())
225+
lu.assertNil(rawget(_G, 'fbeg'), 'consume_style leaked global "fbeg"')
226+
lu.assertNil(rawget(_G, 'fend'), 'consume_style leaked global "fend"')
227+
end
228+
153229
os.exit(lu.LuaUnit.run())

0 commit comments

Comments
 (0)