Keep DNS retry safe for WebSockets and preserve saved user edits
All checks were successful
review/security-claude Blocking finding fixed on 052d0b88; delta reviewed

This commit is contained in:
justinzeus 2026-11-07 01:23:33 +01:00
parent 327f631f0d
commit 052d0b8898
10 changed files with 144 additions and 17 deletions

View file

@ -51,6 +51,10 @@ interface JellyfinServerDao {
return user.copy(rowId = result.toInt())
}
/** Network name refreshes must not overwrite settings or protection changed since launch. */
@Query("UPDATE users SET name = :name WHERE serverId = :serverId AND id = :userId")
fun updateUserName(serverId: UUID, userId: UUID, name: String?): Int
@Query("SELECT * FROM users WHERE serverId = :serverId AND id = :userId")
fun getUser(
serverId: UUID,

View file

@ -138,7 +138,8 @@ class ServerRepository
val (server, userDto) = readSession(expected.server)
if (_current.value !== expected || userDto.id != expected.user.id) return@withContext
serverDao.addOrUpdateServer(server)
val user = serverDao.addOrUpdateUser(expected.user.copy(name = userDto.name))
serverDao.updateUserName(expected.server.id, expected.user.id, userDto.name)
val user = serverDao.getUser(expected.server.id, expected.user.id) ?: return@withContext
if (_current.compareAndSet(expected, CurrentUser(server, user))) updateUserDto(userDto)
}

View file

@ -97,11 +97,11 @@ object AppModule {
fun okHttpClient(dns: PersistentDns) =
DnsConnectionRetry.configure(OkHttpClient.Builder(), dns).build()
/** Resolve filesDir lazily: Android may create it and lookup always runs off main. */
/** Resolve noBackupFilesDir lazily: Android may create it and lookup always runs off main. */
@Provides
@Singleton
fun dns(@ApplicationContext context: Context): PersistentDns =
PersistentDns(file = { File(context.filesDir, "wolfskin-dns.json") })
PersistentDns(file = { File(context.noBackupFilesDir, "wolfskin-dns.json") })
@AuthOkHttpClient
@Provides

View file

@ -1,5 +1,8 @@
package com.github.damontecres.wholphin.services
import android.app.Application
import androidx.room.Room
import androidx.test.core.app.ApplicationProvider
import android.content.Context
import com.github.damontecres.wholphin.data.*
import com.github.damontecres.wholphin.data.model.*
@ -20,20 +23,30 @@ import org.jellyfin.sdk.api.operations.UserApi
import org.jellyfin.sdk.model.api.PublicSystemInfo
import org.jellyfin.sdk.model.api.UserDto
import org.junit.Assert.*
import org.junit.After
import org.junit.Before
import org.junit.Test
import org.junit.runner.RunWith
import org.robolectric.RobolectricTestRunner
import org.robolectric.annotation.Config
/** Saved identities can paint offline without unlocking protected logins or accepting a late identity. */
@OptIn(ExperimentalCoroutinesApi::class)
@RunWith(RobolectricTestRunner::class)
@Config(sdk = [34], application = Application::class)
class ServerSavedSessionTest {
private val dispatcher = StandardTestDispatcher()
private val server = JellyfinServer(UUID.randomUUID(), "Saved server", "https://jellyfin.test", "12.1.0")
private val user = JellyfinUser(id = UUID.randomUUID(), serverId = server.id, name = "Saved name", accessToken = "saved-token")
private var user = JellyfinUser(id = UUID.randomUUID(), serverId = server.id, name = "Saved name", accessToken = "saved-token")
private val users = mockk<UserApi>()
private val system = mockk<SystemApi>()
private val api = mockk<ApiClient>(relaxed = true)
private val recent = mockk<MostRecentServerProvider>()
private val dao = mockk<JellyfinServerDao>(relaxed = true)
private val database by lazy {
Room.inMemoryDatabaseBuilder(ApplicationProvider.getApplicationContext<Application>(), AppDatabase::class.java)
.allowMainThreadQueries().build()
}
private val dao by lazy { database.serverDao() }
private val dto = UserDto(id = user.id, name = "Refreshed name",
hasPassword = false, hasConfiguredPassword = false, hasConfiguredEasyPassword = false)
private val info = PublicSystemInfo(serverName = "Refreshed server", version = "12.1.0")
@ -41,14 +54,16 @@ class ServerSavedSessionTest {
@Before fun savedLogin() {
every { recent.get() } returns MostRecentServer.from(CurrentUser(server, user))
every { dao.getServer(server.id) } returns JellyfinServerUsers(server, listOf(user))
every { dao.addOrUpdateUser(any()) } answers { firstArg() }
dao.addOrUpdateServer(server)
user = dao.addOrUpdateUser(user)
every { api.userApi } returns users
every { api.systemApi } returns system
coEvery { users.getCurrentUser() } returns successResponse(dto)
coEvery { system.getPublicSystemInfo() } returns successResponse(info)
}
@After fun closeDatabase() { database.close() }
@Test fun savedSessionIsUsableWhileUserRevalidationWaitsAndSystemInfoRunsInParallel() = runTest(dispatcher) {
val pendingUser = CompletableDeferred<Unit>()
val systemStarted = CompletableDeferred<Unit>()
@ -69,7 +84,7 @@ class ServerSavedSessionTest {
@Test fun savedProtectedPeopleRemainLocked() = runTest(dispatcher) {
for (protected in listOf(user.copy(pin = "1234"), user.copy(requireLogin = true))) {
every { dao.getServer(server.id) } returns JellyfinServerUsers(server, listOf(protected))
dao.updateUser(protected)
assertTrue(repository.restoreSavedSession() is RestoredSession.ServerOnly)
assertNull(repository.current.value)
}
@ -82,9 +97,8 @@ class ServerSavedSessionTest {
coEvery { users.getCurrentUser() } coAnswers { entered.complete(Unit); pending.await(); successResponse(dto) }
val refresh = async { repository.revalidateSession(first) }
entered.await()
val selected = user.copy(id = UUID.randomUUID(), name = "Other person")
val selected = dao.addOrUpdateUser(user.copy(rowId = 0, id = UUID.randomUUID(), name = "Other person"))
every { recent.get() } returns MostRecentServer.from(CurrentUser(server, selected))
every { dao.getServer(server.id) } returns JellyfinServerUsers(server, listOf(selected))
repository.restoreSavedSession()
pending.complete(Unit)
refresh.await()
@ -92,6 +106,26 @@ class ServerSavedSessionTest {
assertNull(repository.currentUserDto)
}
@Test fun launchNameRefreshPreservesSettingsAndPinChangedWhileTheRequestWaits() = runTest(dispatcher) {
val first = (repository.restoreSavedSession() as RestoredSession.Success).currentUser
val entered = CompletableDeferred<Unit>()
val pending = CompletableDeferred<Unit>()
coEvery { users.getCurrentUser() } coAnswers { entered.complete(Unit); pending.await(); successResponse(dto) }
val refresh = async { repository.revalidateSession(first) }
entered.await()
val edited = user.copy(
pin = "4567",
appPreferences = user.appPreferences.copy(preferredAudioLanguage = "nld"),
config = user.config.copy(homeSettingsSource = HomePageSettingsSource.LOCAL),
)
dao.updateUser(edited)
pending.complete(Unit)
refresh.await()
val expected = edited.copy(name = dto.name)
assertEquals(expected, dao.getUser(server.id, user.id))
assertEquals(expected, repository.current.value!!.user)
}
@Test fun anotherUsersResponseCannotRenameTheSavedPerson() = runTest(dispatcher) {
val first = (repository.restoreSavedSession() as RestoredSession.Success).currentUser
coEvery { users.getCurrentUser() } returns successResponse(dto.copy(id = UUID.randomUUID()))

View file

@ -6,7 +6,6 @@ import com.github.damontecres.wholphin.data.MostRecentServer
import com.github.damontecres.wholphin.data.MostRecentServerProvider
import com.github.damontecres.wholphin.services.hilt.IoCoroutineScope
import dagger.hilt.android.qualifiers.ApplicationContext
import java.net.UnknownHostException
import javax.inject.Inject
import javax.inject.Provider
import javax.inject.Singleton
@ -40,7 +39,7 @@ class NetworkStartup @Inject constructor(
private fun warm(url: String?) {
val host = url?.toHttpUrlOrNull()?.host ?: return
try { dns.lookup(host) } catch (_: UnknownHostException) {
try { dns.lookup(host) } catch (_: Exception) {
// An offline launch must still render the saved people.
}
}

View file

@ -34,7 +34,8 @@ class DnsConnectionRetry(private val dns: PersistentDns) : Interceptor, EventLis
}
override fun intercept(chain: Interceptor.Chain): Response {
val attempt = attempts.getValue(chain.call())
// OkHttp deliberately disables event listeners on its derived WebSocket client.
val attempt = attempts[chain.call()] ?: return chain.proceed(chain.request())
return try {
chain.proceed(chain.request())
} catch (failure: IOException) {

View file

@ -71,7 +71,7 @@ class PersistentDns(
executor.execute {
try {
synchronized(host.resolutionLock) { resolve(hostname, host) }
} catch (_: UnknownHostException) {
} catch (_: Exception) {
// Failed refreshes do not extend the lifetime of the last successful answer.
host.refreshAfter = nowMillis() + FAILURE_BACKOFF_MILLIS
} finally {

View file

@ -0,0 +1,68 @@
// SPDX-License-Identifier: GPL-2.0-only
package org.justinvisser.wolfskin.conductarr
import java.net.InetAddress
import java.util.concurrent.CompletableFuture
import java.util.concurrent.Executors
import java.util.concurrent.TimeUnit
import java.util.concurrent.atomic.AtomicReference
import mockwebserver3.MockResponse
import mockwebserver3.MockWebServer
import okhttp3.Dispatcher
import okhttp3.Dns
import okhttp3.OkHttpClient
import okhttp3.Request
import okhttp3.Response
import okhttp3.WebSocket
import okhttp3.WebSocketListener
import org.junit.Assert.*
import org.junit.Rule
import org.junit.Test
import org.junit.rules.TemporaryFolder
/** OkHttp's listener-free WebSocket client must retain a working upgrade and message path. */
class DnsConnectionRetryTest {
@get:Rule val directory = TemporaryFolder()
@Test fun derivedClientOpensWebSocketWithoutAnUncaughtDispatcherException() {
val opened = CompletableFuture<Int>()
val message = CompletableFuture<String>()
val uncaught = AtomicReference<Throwable?>()
val executor = Executors.newCachedThreadPool { task ->
Thread(task, "dns-websocket-test").apply {
uncaughtExceptionHandler = Thread.UncaughtExceptionHandler { _, failure ->
uncaught.set(failure)
opened.completeExceptionally(failure)
message.completeExceptionally(failure)
}
}
}
MockWebServer().use { server ->
server.enqueue(MockResponse.Builder().webSocketUpgrade(object : WebSocketListener() {
override fun onOpen(webSocket: WebSocket, response: Response) { webSocket.send("ready") }
}).build())
server.start()
val dns = PersistentDns({ directory.root.resolve("dns.json") }, Dns { listOf(InetAddress.getByName("127.0.0.1")) })
val client = DnsConnectionRetry.configure(OkHttpClient.Builder(), dns)
.dispatcher(Dispatcher(executor)).build().newBuilder().build()
val socket = client.newWebSocket(Request.Builder().url(server.url("/events")).build(), object : WebSocketListener() {
override fun onOpen(webSocket: WebSocket, response: Response) { opened.complete(response.code) }
override fun onMessage(webSocket: WebSocket, text: String) { message.complete(text) }
override fun onFailure(webSocket: WebSocket, t: Throwable, response: Response?) {
opened.completeExceptionally(t)
message.completeExceptionally(t)
}
})
try {
assertEquals(101, opened.get(5, TimeUnit.SECONDS).toInt())
assertEquals("ready", message.get(5, TimeUnit.SECONDS))
assertNull(uncaught.get())
println("WEBSOCKET SMOKE: derived cached-DNS client opened HTTP 101, received ready, no uncaught dispatcher exception")
} finally {
socket.cancel()
client.connectionPool.evictAll()
executor.shutdownNow()
}
}
}
}

View file

@ -94,6 +94,22 @@ class PersistentDnsTest {
assertEquals(1, queued.size)
}
@Test fun unexpectedRefreshExceptionKeepsTheLastAnswerAndAllowsAnotherRefresh() {
var failing = false
val dns = cache(Dns { if (failing) throw IllegalStateException("resolver unavailable"); answer })
dns.lookup("jellyfin.test")
now += 120_000
failing = true
assertEquals(listOf(old), dns.lookup("jellyfin.test"))
queued.removeFirst().run()
now += 2_001
failing = false
answer = listOf(fresh)
assertEquals(listOf(old), dns.lookup("jellyfin.test"))
queued.removeFirst().run()
assertEquals(listOf(fresh), dns.lookup("jellyfin.test"))
}
@Test fun failedCachedAddressForcesFreshResolutionAndRetriesBeforeSendingPost() {
MockWebServer().use { server ->
server.start()

View file

@ -34,14 +34,16 @@ network validation. The existing Wolfskin picker paints the saved names and
identity-derived colors, then refreshes the household in the background. PIN and
require-login profiles still go through the existing protected sign-in flow.
`Users/Me` and `System/Info/Public` now start concurrently; a late launch response
cannot replace a different selected person.
cannot replace a different selected person. Name revalidation updates only the
name column and reloads the saved user, preserving newer settings and PINs.
Jellyfin, Conductarr, artwork, playback and crash-report clients share one
`PersistentDns` instance through the application graph. It keeps system-ordered
IPv6/IPv4 answers in memory and `filesDir/wolfskin-dns.json` (at most 32 hosts).
IPv6/IPv4 answers in memory and `noBackupFilesDir/wolfskin-dns.json` (at most 32 hosts).
Saved answers return immediately for up to 24 hours and refresh off main after
60 seconds; failed background lookups back off for 2 seconds without extending
the last successful timestamp. Negative answers are not persisted or cached.
the last successful timestamp. Unexpected resolver exceptions are contained at
the background refresh and warm-up boundaries. Negative answers are not persisted or cached.
Missing, oversized or corrupt hint files fall back to system resolution.
Application creation warms the saved Jellyfin host and configured Conductarr
host on parallel IO jobs; it does not change Android Private DNS.
@ -51,6 +53,8 @@ gets one fresh system lookup and one retry. Responses, read failures and
uncertain mutation outcomes are not replayed. Conductarr retains its existing
no-automatic-replay policy after a request starts. A DNS refresh changes the next
new connection; already-working pooled connections are not forcibly closed.
OkHttp's derived WebSocket clients disable event listeners; those calls bypass
the HTTP retry bookkeeping while retaining the shared DNS resolver.
Device acceptance (not established by JVM tests): with saved people present,
cold-launch six times more than 60 seconds apart and require the picker within