From f7cffe27d9fa0e77320d52b7b15de1857f7580e6 Mon Sep 17 00:00:00 2001 From: AskaEth Date: Sun, 16 Aug 2026 17:35:13 +0800 Subject: [PATCH] fix: final review findings - thread safety, xss, cache --- app/src/main/assets/h5/js/pages/applist.js | 24 ++++++++++---- app/src/main/assets/h5/js/pages/webapplist.js | 29 +++++++++++++---- .../top/yeij/hearth/card/CardRepository.kt | 32 +++++++++++++------ .../yeij/hearth/webapp/WebAppRepository.kt | 28 ++++++++++++---- .../java/top/yeij/hearth/webview/JsBridge.kt | 26 +++++++++------ .../yeij/hearth/card/CardRepositoryTest.kt | 15 +++++++++ .../hearth/webapp/WebAppRepositoryTest.kt | 15 +++++++++ .../top/yeij/hearth/webview/JsBridgeTest.kt | 27 +++++++++++++++- 8 files changed, 157 insertions(+), 39 deletions(-) diff --git a/app/src/main/assets/h5/js/pages/applist.js b/app/src/main/assets/h5/js/pages/applist.js index 164dd2c..f44e966 100644 --- a/app/src/main/assets/h5/js/pages/applist.js +++ b/app/src/main/assets/h5/js/pages/applist.js @@ -9,12 +9,24 @@ window.loadAppList = async function () { const apps = JSON.parse(await bridge.call('listApps')); const grid = document.getElementById('app-grid'); const render = (list) => { - grid.innerHTML = list.map(a => ` - `).join(''); - grid.querySelectorAll('.cell').forEach(el => el.onclick = () => bridge.call('launchApp', el.dataset.pkg)); + grid.textContent = ''; + // 用 createElement + textContent/dataset 渲染，避免 innerHTML 拼接用户数据（XSS） + // Render with createElement + textContent/dataset to avoid innerHTML user data (XSS) + list.forEach(a => { + const btn = document.createElement('button'); + btn.className = 'cell'; + btn.dataset.pkg = a.packageName; + const img = document.createElement('img'); + img.className = 'ic'; + img.src = a.iconBase64 || ''; + img.alt = ''; + const lbl = document.createElement('span'); + lbl.className = 'lbl'; + lbl.textContent = a.label; + btn.append(img, lbl); + btn.onclick = () => bridge.call('launchApp', btn.dataset.pkg); + grid.appendChild(btn); + }); }; render(apps); document.getElementById('app-search').oninput = (e) => { diff --git a/app/src/main/assets/h5/js/pages/webapplist.js b/app/src/main/assets/h5/js/pages/webapplist.js index 5e36ad0..a66fac2 100644 --- a/app/src/main/assets/h5/js/pages/webapplist.js +++ b/app/src/main/assets/h5/js/pages/webapplist.js @@ -9,13 +9,28 @@ window.loadWebAppList = async function () { const apps = JSON.parse(await bridge.call('fetchWebApps')); const grid = document.getElementById('web-grid'); const render = (list) => { - grid.innerHTML = list.map(a => ` - `).join(''); - grid.querySelectorAll('.wcell').forEach(el => el.onclick = () => bridge.call('openWebApp', el.dataset.id)); + grid.textContent = ''; + // 用 createElement + textContent/dataset 渲染，避免 innerHTML 拼接远端数据（XSS） + // Render with createElement + textContent/dataset to avoid innerHTML remote data (XSS) + list.forEach(a => { + const btn = document.createElement('button'); + btn.className = 'wcell'; + btn.dataset.id = a.id; + btn.dataset.url = a.url; + const img = document.createElement('img'); + img.className = 'ic'; + img.src = a.icon; + img.alt = ''; + const lbl = document.createElement('span'); + lbl.className = 'lbl'; + lbl.textContent = a.name; + const tag = document.createElement('span'); + tag.className = 'tag'; + tag.textContent = a.offline ? '离线' : '在线'; + btn.append(img, lbl, tag); + btn.onclick = () => bridge.call('openWebApp', btn.dataset.id); + grid.appendChild(btn); + }); }; render(apps); document.getElementById('web-search').oninput = (e) => { diff --git a/app/src/main/java/top/yeij/hearth/card/CardRepository.kt b/app/src/main/java/top/yeij/hearth/card/CardRepository.kt index 1ace48b..a100b1f 100644 --- a/app/src/main/java/top/yeij/hearth/card/CardRepository.kt +++ b/app/src/main/java/top/yeij/hearth/card/CardRepository.kt @@ -22,24 +22,38 @@ class CardRepository( fun fetchCatalog(): List { val body = http.get(catalogUrl) if (body != null) { - cache.save(KEY, body) - Log.d(TAG, "fetchCatalog: fetched ${body.length} bytes from network") - return parse(body) + // 先解析、成功才缓存，避免畸形 body 被永久缓存 + // Parse first, cache only on success (never cache malformed bodies) + val cards = parse(body) + if (cards != null) { + cache.save(KEY, body) + Log.d(TAG, "fetchCatalog: fetched ${body.length} bytes from network") + return cards + } + Log.w(TAG, "fetchCatalog: invalid catalog body, not cached") + return builtin() } val cached = cache.load(KEY) if (cached != null) { Log.d(TAG, "fetchCatalog: network failed, using cache") - return parse(cached) + return parse(cached) ?: builtin() } Log.d(TAG, "fetchCatalog: no network and no cache, return builtin time card") return builtin() } - // 解析目录 JSON：{ "cards": [ {id,name,priority,entry} ] }，字段缺失/非法时跳过该项 - // Parse catalog JSON; skip malformed or missing-field entries - private fun parse(body: String): List { - val root = gson.fromJson(body, Map::class.java) - val list = root["cards"] as? List<*> ?: return builtin() + // 解析目录 JSON：{ "cards": [ {id,name,priority,entry} ] }，字段缺失/非法时跳过该项； + // 畸形 JSON 或缺少 cards 字段返回 null（调用方回退内置 time 卡） + // Parse catalog JSON; skip malformed or missing-field entries. Return null on + // malformed JSON or a missing cards field (caller falls back to builtin time card) + private fun parse(body: String): List? { + val root = try { + gson.fromJson(body, Map::class.java) + } catch (e: Exception) { + Log.w(TAG, "parse: invalid catalog JSON: ${e.message}") + return null + } + val list = root["cards"] as? List<*> ?: return null val parsed = list.mapNotNull { m -> val map = m as? Map<*, *> ?: return@mapNotNull null val id = map["id"] as? String ?: return@mapNotNull null diff --git a/app/src/main/java/top/yeij/hearth/webapp/WebAppRepository.kt b/app/src/main/java/top/yeij/hearth/webapp/WebAppRepository.kt index 832b837..2622e17 100644 --- a/app/src/main/java/top/yeij/hearth/webapp/WebAppRepository.kt +++ b/app/src/main/java/top/yeij/hearth/webapp/WebAppRepository.kt @@ -1,5 +1,6 @@ package top.yeij.hearth.webapp +import android.util.Log import com.google.gson.Gson import top.yeij.hearth.cache.CacheManager @@ -33,16 +34,30 @@ class WebAppRepository( fun fetchManifest(): List { val body = http.get(manifestUrl) if (body != null) { - cache.save(KEY, body) - return parse(body) + // 先解析、成功才缓存，避免畸形 body 或空结果被永久缓存 + // Parse first, cache only on success (never cache malformed/empty bodies) + val apps = parse(body) + if (apps != null) { + cache.save(KEY, body) + return apps + } + Log.w(TAG, "fetchManifest: invalid body, not cached") + return emptyList() } val cached = cache.load(KEY) ?: return emptyList() - return parse(cached) + return parse(cached) ?: emptyList() } - private fun parse(body: String): List { - val root = gson.fromJson(body, Map::class.java) - val apps = root["apps"] as? List<*> ?: return emptyList() + // 解析清单 JSON；畸形 JSON 或缺少 apps 字段返回 null（调用方决定回退） + // Parse the manifest JSON; return null on malformed JSON / missing apps field + private fun parse(body: String): List? { + val root = try { + gson.fromJson(body, Map::class.java) + } catch (e: Exception) { + Log.w(TAG, "parse: invalid manifest JSON: ${e.message}") + return null + } + val apps = root["apps"] as? List<*> ?: return null return apps.mapNotNull { m -> val map = m as? Map<*, *> ?: return@mapNotNull null val id = map["id"] as? String ?: return@mapNotNull null @@ -57,6 +72,7 @@ class WebAppRepository( } companion object { + private const val TAG = "HearthWebApp" private const val KEY = "webapp-manifest" } } diff --git a/app/src/main/java/top/yeij/hearth/webview/JsBridge.kt b/app/src/main/java/top/yeij/hearth/webview/JsBridge.kt index da24e3a..5bd9c9a 100644 --- a/app/src/main/java/top/yeij/hearth/webview/JsBridge.kt +++ b/app/src/main/java/top/yeij/hearth/webview/JsBridge.kt @@ -75,8 +75,10 @@ class JsBridge( Log.d("HearthBridge", "openWebApp: unknown id=$id") return } - container.open(id, app.url, app.name) + // 状态变更与 View 变更统一在主线程串行，避免与 onBackPressed/listTabs 并发读写 + // Mutate tab state and view on the main thread together, serializing access onMain { + container.open(id, app.url, app.name) host.openWebView(id, app.url) host.syncTabs(container.tabs()) } @@ -90,14 +92,14 @@ class JsBridge( fun closeWebApp(id: String) { val container = webAppContainer ?: return val host = webAppHost ?: return - container.close(id) - val nextActiveId = container.activeTabId() onMain { + container.close(id) + val nextActiveId = container.activeTabId() host.closeWebView(id) nextActiveId?.let { host.switchWebView(it) } host.syncTabs(container.tabs()) + Log.d("HearthBridge", "closeWebApp: id=$id remaining=${container.tabs().size}") } - Log.d("HearthBridge", "closeWebApp: id=$id remaining=${container.tabs().size}") } // 切换激活标签：校验 id 存在后更新状态 → 主线程切换 WebView 可见性 → 同步顶栏 @@ -107,10 +109,11 @@ class JsBridge( fun switchTab(id: String) { val container = webAppContainer ?: return val host = webAppHost ?: return - if (!container.switchTo(id)) return onMain { - host.switchWebView(id) - host.syncTabs(container.tabs()) + if (container.switchTo(id)) { + host.switchWebView(id) + host.syncTabs(container.tabs()) + } } Log.d("HearthBridge", "switchTab: id=$id") } @@ -153,12 +156,15 @@ class JsBridge( Log.d("HearthBridge", "webReload called") } - // 清单内存缓存：首次拉取后缓存，后续复用（避免 openWebApp 重复 I/O） - // In-memory manifest cache: fetch once, reuse afterwards (avoid repeated I/O) + // 清单内存缓存：首次拉取成功后缓存，后续复用（避免 openWebApp 重复 I/O） + // 空结果（离线失败）不缓存，下次调用重试拉取，避免空清单被永久缓存 + // In-memory manifest cache: cache only on a successful non-empty fetch (avoid + // repeated I/O); empty results (offline failure) are never cached so the next + // call retries instead of being permanently stuck with an empty manifest private fun manifest(): List { manifestCache?.let { return it } val apps = webAppRepository?.fetchManifest() ?: emptyList() - manifestCache = apps + if (apps.isNotEmpty()) manifestCache = apps return apps } diff --git a/app/src/test/java/top/yeij/hearth/card/CardRepositoryTest.kt b/app/src/test/java/top/yeij/hearth/card/CardRepositoryTest.kt index c9f77cb..d9b7826 100644 --- a/app/src/test/java/top/yeij/hearth/card/CardRepositoryTest.kt +++ b/app/src/test/java/top/yeij/hearth/card/CardRepositoryTest.kt @@ -50,4 +50,19 @@ class CardRepositoryTest { assertEquals("time", cards[0].id) dir.deleteRecursively() } + + @Test + fun fetchCatalog_malformedBody_returnsBuiltin_andDoesNotCache() { + val dir = File(System.getProperty("java.io.tmpdir"), "c4") + val cm = CacheManager(dir) + val http = object : HttpClient { override fun get(url: String) = "{broken json" } + val repo = CardRepository(http, cm, "http://fake") + // 畸形 body 不崩溃、回退内置 time 卡，且不得写入缓存 + // Malformed body: no crash, fall back to builtin time card, and must not be cached + val cards = repo.fetchCatalog() + assertEquals(1, cards.size) + assertEquals("time", cards[0].id) + assertEquals(null, cm.load("card-catalog")) + dir.deleteRecursively() + } } diff --git a/app/src/test/java/top/yeij/hearth/webapp/WebAppRepositoryTest.kt b/app/src/test/java/top/yeij/hearth/webapp/WebAppRepositoryTest.kt index ca70698..c9b71a5 100644 --- a/app/src/test/java/top/yeij/hearth/webapp/WebAppRepositoryTest.kt +++ b/app/src/test/java/top/yeij/hearth/webapp/WebAppRepositoryTest.kt @@ -41,4 +41,19 @@ class WebAppRepositoryTest { assertTrue(repo.fetchManifest().isEmpty()) dir.deleteRecursively() } + + @Test + fun fetchManifest_malformedBody_returnsEmpty_andDoesNotCache() { + val dir = File(System.getProperty("java.io.tmpdir"), "w4") + val cm = CacheManager(dir) + val http = object : HttpClient { + override fun get(url: String) = "{not valid json" + } + val repo = WebAppRepository(http, cm, "http://fake") + // 畸形 body 不崩溃、返回空，且不得写入缓存 + // Malformed body: no crash, empty result, and must not be cached + assertTrue(repo.fetchManifest().isEmpty()) + assertEquals(null, cm.load("webapp-manifest")) + dir.deleteRecursively() + } } diff --git a/app/src/test/java/top/yeij/hearth/webview/JsBridgeTest.kt b/app/src/test/java/top/yeij/hearth/webview/JsBridgeTest.kt index 1fb624b..8021b8d 100644 --- a/app/src/test/java/top/yeij/hearth/webview/JsBridgeTest.kt +++ b/app/src/test/java/top/yeij/hearth/webview/JsBridgeTest.kt @@ -143,10 +143,11 @@ class JsBridgeTest { postToMainThread = { posted.add(it) }, ) bridge.openWebApp("a") - assertEquals(1, container.tabs().size) + assertEquals(0, container.tabs().size) assertEquals(null, host.openedId) assertEquals(1, posted.size) posted.forEach { it() } + assertEquals(1, container.tabs().size) assertEquals("a", host.openedId) assertEquals("http://x/a", host.openedUrl) dir.deleteRecursively() @@ -165,4 +166,28 @@ class JsBridgeTest { assertEquals(null, host.switchedId) assertEquals("a", container.activeTabId()) } + + @Test + fun fetchWebApps_doesNotCacheEmptyManifest() { + val dir = File(System.getProperty("java.io.tmpdir"), "jb-empty-manifest") + var online = false + val http = object : HttpClient { + override fun get(url: String) = + if (online) """{"apps":[{"id":"a","name":"云音乐","icon":"i","url":"http://x/a"}]}""" else null + } + val repo = WebAppRepository(http, CacheManager(dir), "http://fake") + val bridge = JsBridge( + deviceWidthPx = 800, deviceHeightPx = 480, density = 2.0f, darkMode = true, + webAppRepository = repo, + ) + // 首次离线：返回空，且不得缓存空清单 + // First call offline: empty result must not be cached + assertEquals("[]", bridge.fetchWebApps()) + // 联网后再次拉取：不得命中空缓存，应返回应用 + // Second call online: must not hit the empty cache, should return apps + online = true + val json = bridge.fetchWebApps() + assertTrue(json.contains("\"id\":\"a\"")) + dir.deleteRecursively() + } }