fix: final review findings - thread safety, xss, cache
This commit is contained in:
@@ -9,12 +9,24 @@ window.loadAppList = async function () {
|
|||||||
const apps = JSON.parse(await bridge.call('listApps'));
|
const apps = JSON.parse(await bridge.call('listApps'));
|
||||||
const grid = document.getElementById('app-grid');
|
const grid = document.getElementById('app-grid');
|
||||||
const render = (list) => {
|
const render = (list) => {
|
||||||
grid.innerHTML = list.map(a => `
|
grid.textContent = '';
|
||||||
<button class="cell" data-pkg="${a.packageName}">
|
// 用 createElement + textContent/dataset 渲染,避免 innerHTML 拼接用户数据(XSS)
|
||||||
<img class="ic" src="${a.iconBase64 || ''}" alt="">
|
// Render with createElement + textContent/dataset to avoid innerHTML user data (XSS)
|
||||||
<span class="lbl">${a.label}</span>
|
list.forEach(a => {
|
||||||
</button>`).join('');
|
const btn = document.createElement('button');
|
||||||
grid.querySelectorAll('.cell').forEach(el => el.onclick = () => bridge.call('launchApp', el.dataset.pkg));
|
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);
|
render(apps);
|
||||||
document.getElementById('app-search').oninput = (e) => {
|
document.getElementById('app-search').oninput = (e) => {
|
||||||
|
|||||||
@@ -9,13 +9,28 @@ window.loadWebAppList = async function () {
|
|||||||
const apps = JSON.parse(await bridge.call('fetchWebApps'));
|
const apps = JSON.parse(await bridge.call('fetchWebApps'));
|
||||||
const grid = document.getElementById('web-grid');
|
const grid = document.getElementById('web-grid');
|
||||||
const render = (list) => {
|
const render = (list) => {
|
||||||
grid.innerHTML = list.map(a => `
|
grid.textContent = '';
|
||||||
<button class="wcell" data-id="${a.id}" data-url="${a.url}">
|
// 用 createElement + textContent/dataset 渲染,避免 innerHTML 拼接远端数据(XSS)
|
||||||
<img class="ic" src="${a.icon}" alt="">
|
// Render with createElement + textContent/dataset to avoid innerHTML remote data (XSS)
|
||||||
<span class="lbl">${a.name}</span>
|
list.forEach(a => {
|
||||||
<span class="tag">${a.offline ? '离线' : '在线'}</span>
|
const btn = document.createElement('button');
|
||||||
</button>`).join('');
|
btn.className = 'wcell';
|
||||||
grid.querySelectorAll('.wcell').forEach(el => el.onclick = () => bridge.call('openWebApp', el.dataset.id));
|
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);
|
render(apps);
|
||||||
document.getElementById('web-search').oninput = (e) => {
|
document.getElementById('web-search').oninput = (e) => {
|
||||||
|
|||||||
@@ -22,24 +22,38 @@ class CardRepository(
|
|||||||
fun fetchCatalog(): List<Card> {
|
fun fetchCatalog(): List<Card> {
|
||||||
val body = http.get(catalogUrl)
|
val body = http.get(catalogUrl)
|
||||||
if (body != null) {
|
if (body != null) {
|
||||||
cache.save(KEY, body)
|
// 先解析、成功才缓存,避免畸形 body 被永久缓存
|
||||||
Log.d(TAG, "fetchCatalog: fetched ${body.length} bytes from network")
|
// Parse first, cache only on success (never cache malformed bodies)
|
||||||
return parse(body)
|
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)
|
val cached = cache.load(KEY)
|
||||||
if (cached != null) {
|
if (cached != null) {
|
||||||
Log.d(TAG, "fetchCatalog: network failed, using cache")
|
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")
|
Log.d(TAG, "fetchCatalog: no network and no cache, return builtin time card")
|
||||||
return builtin()
|
return builtin()
|
||||||
}
|
}
|
||||||
|
|
||||||
// 解析目录 JSON:{ "cards": [ {id,name,priority,entry} ] },字段缺失/非法时跳过该项
|
// 解析目录 JSON:{ "cards": [ {id,name,priority,entry} ] },字段缺失/非法时跳过该项;
|
||||||
// Parse catalog JSON; skip malformed or missing-field entries
|
// 畸形 JSON 或缺少 cards 字段返回 null(调用方回退内置 time 卡)
|
||||||
private fun parse(body: String): List<Card> {
|
// Parse catalog JSON; skip malformed or missing-field entries. Return null on
|
||||||
val root = gson.fromJson(body, Map::class.java)
|
// malformed JSON or a missing cards field (caller falls back to builtin time card)
|
||||||
val list = root["cards"] as? List<*> ?: return builtin()
|
private fun parse(body: String): List<Card>? {
|
||||||
|
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 parsed = list.mapNotNull { m ->
|
||||||
val map = m as? Map<*, *> ?: return@mapNotNull null
|
val map = m as? Map<*, *> ?: return@mapNotNull null
|
||||||
val id = map["id"] as? String ?: return@mapNotNull null
|
val id = map["id"] as? String ?: return@mapNotNull null
|
||||||
|
|||||||
@@ -1,5 +1,6 @@
|
|||||||
package top.yeij.hearth.webapp
|
package top.yeij.hearth.webapp
|
||||||
|
|
||||||
|
import android.util.Log
|
||||||
import com.google.gson.Gson
|
import com.google.gson.Gson
|
||||||
import top.yeij.hearth.cache.CacheManager
|
import top.yeij.hearth.cache.CacheManager
|
||||||
|
|
||||||
@@ -33,16 +34,30 @@ class WebAppRepository(
|
|||||||
fun fetchManifest(): List<WebApp> {
|
fun fetchManifest(): List<WebApp> {
|
||||||
val body = http.get(manifestUrl)
|
val body = http.get(manifestUrl)
|
||||||
if (body != null) {
|
if (body != null) {
|
||||||
cache.save(KEY, body)
|
// 先解析、成功才缓存,避免畸形 body 或空结果被永久缓存
|
||||||
return parse(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()
|
val cached = cache.load(KEY) ?: return emptyList()
|
||||||
return parse(cached)
|
return parse(cached) ?: emptyList()
|
||||||
}
|
}
|
||||||
|
|
||||||
private fun parse(body: String): List<WebApp> {
|
// 解析清单 JSON;畸形 JSON 或缺少 apps 字段返回 null(调用方决定回退)
|
||||||
val root = gson.fromJson(body, Map::class.java)
|
// Parse the manifest JSON; return null on malformed JSON / missing apps field
|
||||||
val apps = root["apps"] as? List<*> ?: return emptyList()
|
private fun parse(body: String): List<WebApp>? {
|
||||||
|
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 ->
|
return apps.mapNotNull { m ->
|
||||||
val map = m as? Map<*, *> ?: return@mapNotNull null
|
val map = m as? Map<*, *> ?: return@mapNotNull null
|
||||||
val id = map["id"] as? String ?: return@mapNotNull null
|
val id = map["id"] as? String ?: return@mapNotNull null
|
||||||
@@ -57,6 +72,7 @@ class WebAppRepository(
|
|||||||
}
|
}
|
||||||
|
|
||||||
companion object {
|
companion object {
|
||||||
|
private const val TAG = "HearthWebApp"
|
||||||
private const val KEY = "webapp-manifest"
|
private const val KEY = "webapp-manifest"
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -75,8 +75,10 @@ class JsBridge(
|
|||||||
Log.d("HearthBridge", "openWebApp: unknown id=$id")
|
Log.d("HearthBridge", "openWebApp: unknown id=$id")
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
container.open(id, app.url, app.name)
|
// 状态变更与 View 变更统一在主线程串行,避免与 onBackPressed/listTabs 并发读写
|
||||||
|
// Mutate tab state and view on the main thread together, serializing access
|
||||||
onMain {
|
onMain {
|
||||||
|
container.open(id, app.url, app.name)
|
||||||
host.openWebView(id, app.url)
|
host.openWebView(id, app.url)
|
||||||
host.syncTabs(container.tabs())
|
host.syncTabs(container.tabs())
|
||||||
}
|
}
|
||||||
@@ -90,14 +92,14 @@ class JsBridge(
|
|||||||
fun closeWebApp(id: String) {
|
fun closeWebApp(id: String) {
|
||||||
val container = webAppContainer ?: return
|
val container = webAppContainer ?: return
|
||||||
val host = webAppHost ?: return
|
val host = webAppHost ?: return
|
||||||
container.close(id)
|
|
||||||
val nextActiveId = container.activeTabId()
|
|
||||||
onMain {
|
onMain {
|
||||||
|
container.close(id)
|
||||||
|
val nextActiveId = container.activeTabId()
|
||||||
host.closeWebView(id)
|
host.closeWebView(id)
|
||||||
nextActiveId?.let { host.switchWebView(it) }
|
nextActiveId?.let { host.switchWebView(it) }
|
||||||
host.syncTabs(container.tabs())
|
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 可见性 → 同步顶栏
|
// 切换激活标签:校验 id 存在后更新状态 → 主线程切换 WebView 可见性 → 同步顶栏
|
||||||
@@ -107,10 +109,11 @@ class JsBridge(
|
|||||||
fun switchTab(id: String) {
|
fun switchTab(id: String) {
|
||||||
val container = webAppContainer ?: return
|
val container = webAppContainer ?: return
|
||||||
val host = webAppHost ?: return
|
val host = webAppHost ?: return
|
||||||
if (!container.switchTo(id)) return
|
|
||||||
onMain {
|
onMain {
|
||||||
host.switchWebView(id)
|
if (container.switchTo(id)) {
|
||||||
host.syncTabs(container.tabs())
|
host.switchWebView(id)
|
||||||
|
host.syncTabs(container.tabs())
|
||||||
|
}
|
||||||
}
|
}
|
||||||
Log.d("HearthBridge", "switchTab: id=$id")
|
Log.d("HearthBridge", "switchTab: id=$id")
|
||||||
}
|
}
|
||||||
@@ -153,12 +156,15 @@ class JsBridge(
|
|||||||
Log.d("HearthBridge", "webReload called")
|
Log.d("HearthBridge", "webReload called")
|
||||||
}
|
}
|
||||||
|
|
||||||
// 清单内存缓存:首次拉取后缓存,后续复用(避免 openWebApp 重复 I/O)
|
// 清单内存缓存:首次拉取成功后缓存,后续复用(避免 openWebApp 重复 I/O)
|
||||||
// In-memory manifest cache: fetch once, reuse afterwards (avoid repeated 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<WebApp> {
|
private fun manifest(): List<WebApp> {
|
||||||
manifestCache?.let { return it }
|
manifestCache?.let { return it }
|
||||||
val apps = webAppRepository?.fetchManifest() ?: emptyList<WebApp>()
|
val apps = webAppRepository?.fetchManifest() ?: emptyList<WebApp>()
|
||||||
manifestCache = apps
|
if (apps.isNotEmpty()) manifestCache = apps
|
||||||
return apps
|
return apps
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -50,4 +50,19 @@ class CardRepositoryTest {
|
|||||||
assertEquals("time", cards[0].id)
|
assertEquals("time", cards[0].id)
|
||||||
dir.deleteRecursively()
|
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()
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -41,4 +41,19 @@ class WebAppRepositoryTest {
|
|||||||
assertTrue(repo.fetchManifest().isEmpty())
|
assertTrue(repo.fetchManifest().isEmpty())
|
||||||
dir.deleteRecursively()
|
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()
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -143,10 +143,11 @@ class JsBridgeTest {
|
|||||||
postToMainThread = { posted.add(it) },
|
postToMainThread = { posted.add(it) },
|
||||||
)
|
)
|
||||||
bridge.openWebApp("a")
|
bridge.openWebApp("a")
|
||||||
assertEquals(1, container.tabs().size)
|
assertEquals(0, container.tabs().size)
|
||||||
assertEquals(null, host.openedId)
|
assertEquals(null, host.openedId)
|
||||||
assertEquals(1, posted.size)
|
assertEquals(1, posted.size)
|
||||||
posted.forEach { it() }
|
posted.forEach { it() }
|
||||||
|
assertEquals(1, container.tabs().size)
|
||||||
assertEquals("a", host.openedId)
|
assertEquals("a", host.openedId)
|
||||||
assertEquals("http://x/a", host.openedUrl)
|
assertEquals("http://x/a", host.openedUrl)
|
||||||
dir.deleteRecursively()
|
dir.deleteRecursively()
|
||||||
@@ -165,4 +166,28 @@ class JsBridgeTest {
|
|||||||
assertEquals(null, host.switchedId)
|
assertEquals(null, host.switchedId)
|
||||||
assertEquals("a", container.activeTabId())
|
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()
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user