diff --git a/spec/content_css_sanitize_spec.lua b/spec/content_css_sanitize_spec.lua new file mode 100644 index 0000000..e068804 --- /dev/null +++ b/spec/content_css_sanitize_spec.lua @@ -0,0 +1,139 @@ +package.path = "./?.lua;./?/init.lua;" .. package.path + +local checks = 0 +local function expect(condition, message) + checks = checks + 1 + if not condition then error(message or ("check " .. checks .. " failed")) end +end + +package.preload["logger"] = function() + return { + info = function() end, + warn = function() end, + err = function() end, + dbg = function() end, + } +end +package.preload["weread.lib.crypto"] = function() return {} end +package.preload["weread.lib.reader_state"] = function() return {} end +package.preload["weread.lib.protocol"] = function() return {} end +package.preload["weread.lib.thoughts"] = function() return {} end + +local Content = require("weread/lib/content") + +-- The hostile rule observed in real WeRead e_2 shards: crengine honors the +-- root-element zero sizing and p{font-size:1rem} then collapses the whole book. +local hostile = "html,\nbody {\n margin: 0;\n padding: 0;\n font-size: 0;\n }" +local cleaned, count = Content.sanitize_book_css(hostile) +expect(count == 1, "the single font-size:0 declaration must be reported as one removal") +expect(not cleaned:find("font%-size"), "font-size:0 declaration survived sanitization") +expect(cleaned:find("{", 1, true) and cleaned:find("}", 1, true), + "sanitization must keep the rule braces intact") +expect(cleaned:find("margin: 0;", 1, true) and cleaned:find("padding: 0;", 1, true), + "sibling margin/padding declarations must survive sanitization") + +-- Review: `font-size: 0` can be intentional outside the root elements (e.g. +-- hiding whitespace between inline-block items), so only rules whose selector +-- list names exactly html/body may be touched. +cleaned, count = Content.sanitize_book_css(".a{font-size: 0 !important;color:red}") +expect(count == 0 and cleaned == ".a{font-size: 0 !important;color:red}", + "a non-root selector had its intentional font-size:0 removed") + +cleaned, count = Content.sanitize_book_css(".slide-nav { font-size: 0 }") +expect(count == 0 and cleaned == ".slide-nav { font-size: 0 }", + "an inline-block whitespace trick on a class selector was stripped") + +cleaned, count = Content.sanitize_book_css("body p{font-size:0}") +expect(count == 0 and cleaned == "body p{font-size:0}", + "a descendant-of-body selector was treated as the root element") + +cleaned, count = Content.sanitize_book_css("body,.wrapper{font-size:0}") +expect(count == 0 and cleaned == "body,.wrapper{font-size:0}", + "a selector list mixing body with other targets was treated as root-only") + +cleaned, count = Content.sanitize_book_css("HTML ,\nBODY { font-size: 0 }") +expect(count == 1 and not cleaned:find("font%-size"), + "root selector matching must be case- and whitespace-insensitive") + +-- Deliberate limits: :root and @media-wrapped root rules stay untouched. +cleaned, count = Content.sanitize_book_css(":root{font-size:0}") +expect(count == 0 and cleaned == ":root{font-size:0}", + ":root sizing was stripped although only html/body rules are covered") + +local media_css = "@media print { html, body { font-size: 0 } }" +cleaned, count = Content.sanitize_book_css(media_css) +expect(count == 0 and cleaned == media_css, + "@media-wrapped root sizing was stripped although only top-level rules are covered") + +-- An at-rule before the selector must not prevent matching the rule itself. +cleaned, count = Content.sanitize_book_css('@charset "utf-8";html,body{font-size:0}') +expect(count == 1 and not cleaned:find("font%-size"), + "a preceding at-rule broke root-selector matching") + +cleaned, count = Content.sanitize_book_css("html{font-size: 0px}body{font-size: 0%;}") +expect(count == 2 and not cleaned:find("font%-size"), + "zero font-size with px/% units was not fully removed on root selectors") + +cleaned, count = Content.sanitize_book_css(".fs05 { font-size: 0.5rem; }") +expect(count == 0 and cleaned == ".fs05 { font-size: 0.5rem; }", + "fractional font-size must stay untouched") + +cleaned, count = Content.sanitize_book_css("html, body { font-size: 0.5rem; }") +expect(count == 0 and cleaned == "html, body { font-size: 0.5rem; }", + "fractional root font-size must stay untouched") + +cleaned, count = Content.sanitize_book_css("p { font-size: 1rem; }") +expect(count == 0 and cleaned == "p { font-size: 1rem; }", + "non-zero font-size must stay untouched") + +cleaned, count = Content.sanitize_book_css("{ color: #000; font-size: 0\n}") +expect(count == 0 and cleaned:find("font-size: 0", 1, true), + "a block without a selector must be left untouched") + +local passthrough, passthrough_count = Content.sanitize_book_css(nil) +expect(passthrough == nil and passthrough_count == 0, + "nil input must pass through unchanged with count 0") +passthrough, passthrough_count = Content.sanitize_book_css("") +expect(passthrough == "" and passthrough_count == 0, + "empty input must pass through unchanged with count 0") + +-- Integration-style: sanitizing an already-sanitized shard changes nothing. +-- Only the root rule qualifies; the other blocks keep their declarations. +local combined = table.concat({ + hostile, + ".a{font-size: 0 !important;color:red}", + "p{font-size: 0px}h1{font-size: 0%;}", + "{ color: #000; font-size: 0\n}", +}, "\n") +local once, first_count = Content.sanitize_book_css(combined) +local twice, second_count = Content.sanitize_book_css(once) +expect(first_count == 1, "combined shard should lose only the root font-size declaration") +expect(second_count == 0 and twice == once, "sanitization must be idempotent") + +-- Adjacent zero declarations: each pass consumes one boundary character, so +-- the sanitizer must iterate to a fixpoint instead of keeping the second one. +cleaned, count = Content.sanitize_book_css("html,body{font-size:0;font-size:0}") +expect(count == 2, "adjacent zero declarations must both be removed") +expect(not cleaned:find("font%-size"), "adjacent removal left a hostile declaration behind") +expect(cleaned:find("^html,body%{.*%}$"), "block structure was damaged by adjacent removal") + +cleaned, count = Content.sanitize_book_css("html,body{font-size:0 ;font-size:0 ;color:red}") +expect(count == 2 and cleaned:find("color:red", 1, true), + "spaced adjacent zeros must be removed while siblings survive") + +cleaned, count = Content.sanitize_book_css("html,body{font-size:0vh}") +expect(count == 1 and not cleaned:find("font%-size"), + "zero with any letter unit (0vh) is a zero length and must be removed") + +-- A CSS comment carrying the exact declaration may lose its interior, but the +-- stylesheet structure around it must survive and the result stays idempotent. +local commented = "body{ /* font-size: 0 ; old */ color:blue }" +cleaned, count = Content.sanitize_book_css(commented) +local open_braces = select(2, cleaned:gsub("%{", "")) +local close_braces = select(2, cleaned:gsub("}", "")) +expect(count == 1 and cleaned:find("color:blue", 1, true) and open_braces == close_braces, + "comment rewrite must keep the block structurally valid") +local _, recount = Content.sanitize_book_css(cleaned) +expect(recount == 0, "sanitization after comment rewrite must be idempotent") + +print(("content_css_sanitize_spec: %d checks"):format(checks)) diff --git a/weread/lib/content.lua b/weread/lib/content.lua index 9f27f16..4a1cf6e 100755 --- a/weread/lib/content.lua +++ b/weread/lib/content.lua @@ -1362,12 +1362,108 @@ function Content.fetch_chapter_xhtml(client, settings, book, chapter) ) end +-- True when the text following the literal "0" of a font-size declaration +-- (already captured by the caller's pattern) is only an optional unit plus +-- whitespace and an optional !important flag, i.e. the declared size really is +-- zero. Zero times any unit is still zero length, so an empty tail (bare 0) and +-- every letter-unit form (px/em/rem/vh/...) count; fractional sizes such as +-- 0.5rem never match because "." is not a letter. +local function is_zero_font_size(tail) + local value = tail:lower():match("^%s*(.-)%s*$") + if value:sub(-10) == "!important" then + value = (value:match("^(.-)%s*!important$") or ""):match("^%s*(.-)%s*$") + end + return value == "" or value == "%" or value:match("^%a+$") ~= nil +end + +-- Known limitations: property-name matching is case-sensitive (all observed +-- WeRead shards are lowercase), a CSS comment containing exactly +-- "font-size: 0" may have its interior rewritten without structural harm, and +-- only top-level rules naming exactly html/body are touched (:root, +-- descendant selectors and @media-wrapped rules are left as-is). + +-- True when the selector list names nothing but the root elements, i.e. every +-- comma-separated selector is exactly html or body (case- and +-- whitespace-insensitive). Compound selectors such as "body p" or +-- "body, .wrapper" also style other content, so they never qualify. +local function is_root_selector_list(selectors) + local count = 0 + for selector in (selectors or ""):gmatch("[^,]+") do + count = count + 1 + local name = selector:lower():gsub("^%s+", ""):gsub("%s+$", "") + if name ~= "html" and name ~= "body" then return false end + end + return count > 0 +end + +-- Remove every zero `font-size` declaration from one braceless declaration +-- block. The sentinel "{" guarantees the boundary capture below always has a +-- character to inspect, even when the declaration opens the block. +local function strip_zero_font_sizes(block) + local removed = 0 + local cleaned = ("{" .. block):gsub("([^%w%-])(%s*)font%-size%s*:%s*0([^;}]*)(;?)", function(boundary, leading, tail, _terminator) + if not is_zero_font_size(tail) then + return nil -- keep fractional sizes such as 0.5rem untouched + end + removed = removed + 1 + return boundary .. leading + end) + return cleaned:sub(2), removed +end + +-- Strip hostile `font-size: 0` declarations from server-provided book css, but +-- only inside rules whose selector list is exactly `html` and/or `body`. +-- WeRead shards occasionally ship `html, body { ... font-size: 0; }`; WeRead's +-- own apps ignore root-element sizing but crengine honors it, collapsing the +-- whole book to a near-zero font size on device. Elsewhere `font-size: 0` can +-- be intentional (e.g. hiding whitespace between inline-block items), so every +-- other rule passes through verbatim. +local function sanitize_book_css_pass(css) + local removed = 0 + -- Scan whole `selector { block }` units (balanced braces); untouched units + -- are returned verbatim so no other declaration can be disturbed. + local sanitized = css:gsub("([^{}]*)(%b{})", function(prelude, block) + -- Text before the last ";" belongs to an at-rule or a previous + -- statement, not to this block's selector list. + local selectors = prelude:match("[^;]*$") or "" + if not is_root_selector_list(selectors) then + return prelude .. block + end + local cleaned, dropped = strip_zero_font_sizes(block:sub(2, -2)) + removed = removed + dropped + return prelude .. "{" .. cleaned .. "}" + end) + return sanitized, removed +end + +function Content.sanitize_book_css(css) + if type(css) ~= "string" or css == "" then + return css, 0 + end + -- Each pass consumes one boundary character per match, so adjacent zero + -- declarations ("font-size:0;font-size:0") need repeated passes until the + -- fixpoint; the cap only guards pathological input. + local removed_total = 0 + local sanitized = css + for _i = 1, 16 do + local removed + sanitized, removed = sanitize_book_css_pass(sanitized) + removed_total = removed_total + removed + if removed == 0 then break end + end + return sanitized, removed_total +end + function Content.fetch_chapter_css(client, settings, book, chapter) local ok, css = pcall(function() return Content.decode_content_shard(Content.fetch_chapter_shard(client, settings, book, chapter, "/web/book/chapter/e_2")) end) if ok then - return css + local sanitized, removed = Content.sanitize_book_css(css) + if removed > 0 then + logger.warn("removed ", removed, " hostile font-size:0 declarations from book css") + end + return sanitized end return nil end