chore(release): prepare 4.0.0 with bug fixes and broader test coverage

AGENTS.md

@@ -8,7 +8,7 @@ Rules for AI coding agents (and a quick reference for humans) working in this re
 - `.github/workflows/`: CI. Actions are pinned by commit SHA with a version comment.
 
 ## Build and test
-- Backend: `cd backend && ./gradlew test` (needs a JDK matching `sourceCompatibility` in `backend/build.gradle.kts` and a running Docker daemon for Testcontainers).
+- Backend: `cd backend && ./gradlew test` (needs a JDK matching `sourceCompatibility` in `backend/build.gradle.kts` and a running Docker daemon for Testcontainers). Add `jacocoTestReport` for coverage (`build/reports/jacoco/test/html/index.html`, vendored `org.torproject` code is excluded).
 - Local backend image: `./gradlew jibDockerBuild -Djib.to.image=tormap/backend:local -Djib.to.tags=local`.
 - Frontend: `cd frontend && yarn install --immutable && yarn build && yarn lint`.
 - Frontend tests: `cd frontend && yarn test:run` (Vitest). `*.unit.test.ts` run in plain Node, all other `*.test.ts(x)` run in headless Chromium (browser mode), so run `yarn playwright install chromium` once. Use `yarn test` for watch mode and `yarn test:coverage` for coverage. The sandbox blocks Chromium, so browser tests must run outside it.

backend/README.md

@@ -50,7 +50,7 @@ less reliable.
    `SPRING_PROFILES_ACTIVE` is set.
 4. Backend should be available at http://localhost:8080
 
-Run `./gradlew test`, to only build & tests.
+Run `./gradlew test`, to only build & tests. Run `./gradlew test jacocoTestReport` for a coverage report in `build/reports/jacoco/test/html/index.html`.
 
 In a freshly cloned repository the backend will start to download an archive `>33 GB` in size
 from [TorProject](https://metrics.torproject.org/collector/archive/). The total time for the whole archive will depend

backend/build.gradle.kts

@@ -2,7 +2,7 @@ import org.jetbrains.kotlin.gradle.dsl.JvmTarget
 import org.jetbrains.kotlin.gradle.tasks.KotlinCompile
 
 group = "org.tormap"
-version = "3.0.1"
+version = "4.0.0"
 java.sourceCompatibility = JavaVersion.VERSION_25
 
 plugins {
@@ -17,6 +17,9 @@ plugins {
 
     // Build and push docker images
     id("com.google.cloud.tools.jib") version "3.5.4"
+
+    // Test coverage, run with ./gradlew test jacocoTestReport
+    jacoco
 }
 
 repositories {
@@ -152,6 +155,22 @@ tasks.withType<Test> {
     useJUnitPlatform()
 }
 
+jacoco {
+    // Java 25 class files need JaCoCo 0.8.14 or later
+    toolVersion = "0.8.15"
+}
+
+// Writes build/reports/jacoco/test/html/index.html and an XML report for tools
+tasks.jacocoTestReport {
+    dependsOn(tasks.test)
+    reports {
+        xml.required = true
+        html.required = true
+    }
+    // The vendored metrics-lib (org.torproject) is mostly unused by TorMap, so it would dilute the numbers
+    classDirectories.setFrom(classDirectories.files.map { fileTree(it) { exclude("org/torproject/**") } })
+}
+
 // The local development server uses the dev profile unless SPRING_PROFILES_ACTIVE is set
 tasks.bootRun {
     if (providers.environmentVariable("SPRING_PROFILES_ACTIVE").orNull == null) {

backend/src/main/kotlin/org/tormap/config/RequestBodyLimitConfig.kt

@@ -14,6 +14,7 @@ import org.springframework.web.filter.OncePerRequestFilter
 import org.springframework.web.server.ContentTooLargeException
 import java.io.BufferedReader
 import java.io.InputStreamReader
+import java.io.UnsupportedEncodingException
 import java.nio.charset.Charset
 
 /**
@@ -57,7 +58,12 @@ class RequestBodyLimitFilter(private val maxBodyBytes: Long) : OncePerRequestFil
         override fun getInputStream(): ServletInputStream = inputStream
 
         override fun getReader(): BufferedReader {
-            val charset = characterEncoding?.let { Charset.forName(it) } ?: Charsets.ISO_8859_1
+            val charset = try {
+                characterEncoding?.let { Charset.forName(it) } ?: Charsets.ISO_8859_1
+            } catch (_: IllegalArgumentException) {
+                // The encoding comes from the request, the servlet API reports an unknown one like this
+                throw UnsupportedEncodingException(characterEncoding)
+            }
             return BufferedReader(InputStreamReader(inputStream, charset))
         }
     }

backend/src/main/kotlin/org/tormap/config/SecurityConfig.kt

@@ -52,12 +52,23 @@ class SecurityConfig(
     fun passwordEncoder(): PasswordEncoder = BCryptPasswordEncoder()
 
     @Bean
-    fun userDetailsService(passwordEncoder: PasswordEncoder): UserDetailsService {
-        if (!System.getenv(ENV_REMOVED_ADMIN_PASSWORD_BCRYPT).isNullOrBlank()) {
+    fun userDetailsService(passwordEncoder: PasswordEncoder): UserDetailsService =
+        buildUserDetailsService(passwordEncoder, System.getenv(ENV_ADMIN_PASSWORD), System.getenv(ENV_REMOVED_ADMIN_PASSWORD_BCRYPT))
+
+    /**
+     * Creates the admin user from the values of [ENV_ADMIN_PASSWORD] and [ENV_REMOVED_ADMIN_PASSWORD_BCRYPT].
+     * Without a password there is no user, so the actuator denies every request.
+     */
+    internal fun buildUserDetailsService(
+        passwordEncoder: PasswordEncoder,
+        adminPassword: String?,
+        removedAdminPasswordBcrypt: String?,
+    ): UserDetailsService {
+        if (!removedAdminPasswordBcrypt.isNullOrBlank()) {
             logger.warn("$ENV_REMOVED_ADMIN_PASSWORD_BCRYPT is no longer supported and is ignored. Use $ENV_ADMIN_PASSWORD instead.")
         }
 
-        val password = System.getenv(ENV_ADMIN_PASSWORD)?.trim().orEmpty()
+        val password = adminPassword?.trim().orEmpty()
 
         if (password.isBlank()) {
             logger.warn("------------------------------------------------------")

backend/src/main/kotlin/org/tormap/database/entity/RelayLocation.kt

@@ -37,11 +37,10 @@ class RelayLocation(
     @Column(length = 40, columnDefinition = "bpchar(40)")
     var fingerprint: String = networkStatusEntry.fingerprint
 
-    var flags: String? = try {
-        networkStatusEntry.flags.map { TorRelayFlag.valueOf(it).ordinal }.joinToString(", ")
-    } catch (exception: Exception) {
-        null
-    }
+    // Flags this version does not know (e.g. MiddleOnly) are skipped, so the known flags of the relay are kept
+    var flags: String? = networkStatusEntry.flags
+        .mapNotNull { flag -> TorRelayFlag.entries.find { it.name == flag }?.ordinal }
+        .joinToString(", ")
 
     /**
      * The `w Bandwidth=` value of the consensus entry, which clients use for path selection.

backend/src/main/kotlin/org/tormap/service/DescriptorCoordinationService.kt

@@ -124,10 +124,14 @@ class DescriptorCoordinationService(
 
     private fun computeRelayDetailsAndCaches(descriptorType: DescriptorType, processedMonths: Set<String>) {
         if (descriptorType.isRelayServerType()) {
-            relayDetailsUpdateService.computeFamilies(processedMonths)
             relayDetailsUpdateService.lookupMissingAutonomousSystems(processedMonths)
+            // The cached relay locations contain the family IDs, so they are only updated once the families are computed
+            relayDetailsUpdateService.computeFamilies(processedMonths).whenComplete { _, _ ->
+                updateCaches(descriptorType, processedMonths)
+            }
+        } else {
+            updateCaches(descriptorType, processedMonths)
         }
-        updateCaches(descriptorType, processedMonths)
     }
 
     private fun updateCaches(descriptorType: DescriptorType, processedMonths: Set<String>) {

backend/src/main/kotlin/org/tormap/service/RelayDetailsUpdateService.kt

@@ -10,6 +10,7 @@ import org.tormap.util.addFamilyMember
 import org.tormap.util.commaSeparatedToList
 import org.tormap.util.getFamilyMember
 import org.tormap.util.logger
+import java.util.concurrent.CompletableFuture
 import javax.sql.DataSource
 
 /**
@@ -76,23 +77,29 @@ class RelayDetailsUpdateService(
         val monthFamilyMemberCount =
             relayDetailsRepositoryImpl.findDistinctMonthFamilyMemberCount().filter { it.count == 0L }
         val monthsToProcess = monthFamilyMemberCount.map { it.month }.toSet()
-        computeFamilies(monthsToProcess)
-        cacheService.evictRelayLocationsPerDay(monthsToProcess)
+        // The families are computed asynchronously, evicting earlier would let requests cache the old families again
+        computeFamilies(monthsToProcess).whenComplete { _, _ ->
+            cacheService.evictRelayLocationsPerDay(monthsToProcess)
+        }
     }
 
 
     /**
-     * Updates [RelayDetails.familyId] for all entities of the requested [months].
+     * Updates [RelayDetails.familyId] for all entities of the requested [months] asynchronously.
+     * @return a future that completes once the families of all [months] are computed (also if some failed)
      */
-    fun computeFamilies(months: Set<String>) {
+    fun computeFamilies(months: Set<String>): CompletableFuture<Void> {
         logger.info("... Updating relay families for months: {}", months.joinToString(", "))
-        months.forEach { month ->
+        val futures = months.map { month ->
             coalesceService.submitAsync("computeFamilies-$month") {
                 transactionTemplate.executeWithoutResult {
                     computeFamiliesForMonth(month)
                 }
             }
+                // Failures are logged by the CoalesceService, a failed month must not hide the others
+                .exceptionally { null }
         }
+        return CompletableFuture.allOf(*futures.toTypedArray())
     }
 
     /**

backend/src/main/kotlin/org/tormap/service/ReverseDnsLookupService.kt

@@ -85,24 +85,28 @@ class DNSJavaReverseDnsResolver : ReverseDnsResolver {
 
     private fun lookup(name: Name, type: Int): List<Record> =
         Lookup(name, type).apply { setResolver(dnsResolver) }.run()?.toList() ?: emptyList()
-
-    private fun InetAddress.isULA(): Boolean =
-        this is Inet6Address &&
-            (address[0].toInt() and 0xFE) == 0xFC  // fc00::/7
-
-    private fun InetAddress.isPubliclyRoutable(): Boolean =
-        !isLoopbackAddress &&
-            !isLinkLocalAddress &&
-            !isSiteLocalAddress &&   // reliable for IPv4
-            !isULA() &&              // fills the IPv6 gap isSiteLocalAddress misses
-            !isMulticastAddress &&
-            !isAnyLocalAddress &&
-            address.first() != 0.toByte() &&
-            !(address.size == 4 &&
-                address[0] == 100.toByte() &&
-                (address[1].toInt() and 0xFF) in 64..127)
 }
 
+private fun InetAddress.isULA(): Boolean =
+    this is Inet6Address &&
+        (address[0].toInt() and 0xFE) == 0xFC  // fc00::/7
+
+/**
+ * Whether the address can be reached on the public internet. Forward lookups only verify a PTR name with such
+ * addresses, so that a name pointing to an internal address is never reported as verified.
+ */
+internal fun InetAddress.isPubliclyRoutable(): Boolean =
+    !isLoopbackAddress &&
+        !isLinkLocalAddress &&
+        !isSiteLocalAddress &&   // reliable for IPv4
+        !isULA() &&              // fills the IPv6 gap isSiteLocalAddress misses
+        !isMulticastAddress &&
+        !isAnyLocalAddress &&
+        address.first() != 0.toByte() &&
+        !(address.size == 4 &&
+            address[0] == 100.toByte() &&
+            (address[1].toInt() and 0xFF) in 64..127)
+
 @Service
 class ReverseDnsLookupService(
     private val reverseDnsResolver: ReverseDnsResolver,

backend/src/main/kotlin/org/tormap/util/PathSelection.kt

@@ -13,23 +13,21 @@ object PathSelection {
     /**
      * Returns the share of a relay's consensus weight that clients use for choosing it as entry guard, so the
      * relay's probability of being picked as guard is `guardWeight / sum of the guard weights of all relays`.
+     * It follows the guard position weights of Tor clients (`compute_weighted_bandwidths` in Tor's node_select.c):
      *
      * - A relay that is not `Running` and `Valid` can not be picked, which results in 0.
-     * - Guard flagged relays use `Wgg`, relays that are also exits use `Wgd`, all other relays `Wgm`.
+     * - Clients only pick relays with the `Guard` flag as entry guards, all other relays result in 0.
+     * - Guard flagged relays that are also exits use `Wgd`, all other guards `Wgg`. A `BadExit` does not count as exit.
      *
      * @param consensusWeight the `w Bandwidth=` value of the relay's consensus entry
      * @param flags the names of the flags the relay has in the consensus
      * @param bandwidthWeights the `bandwidth-weights` of the consensus, null if it has none
      * @return null, if the consensus does not contain the required weight
      */
     fun guardWeight(consensusWeight: Long, flags: Set<String>, bandwidthWeights: Map<String, Int>?): Long? {
-        if (!flags.contains("Running") || !flags.contains("Valid")) return 0
-        val weightKey = when {
-            flags.contains("Guard") && flags.contains("Exit") -> "Wgd"
-            flags.contains("Guard") -> "Wgg"
-            else -> "Wgm"
-        }
-        val weight = bandwidthWeights?.get(weightKey) ?: return null
+        if (!flags.contains("Running") || !flags.contains("Valid") || !flags.contains("Guard")) return 0
+        val isExit = flags.contains("Exit") && !flags.contains("BadExit")
+        val weight = bandwidthWeights?.get(if (isExit) "Wgd" else "Wgg") ?: return null
         return Math.round(consensusWeight.toDouble() * weight / BANDWIDTH_WEIGHT_SCALE)
     }
 }

backend/src/test/kotlin/org/tormap/config/AdminUserTest.kt

@@ -0,0 +1,51 @@
+package org.tormap.config
+
+import io.kotest.assertions.throwables.shouldThrow
+import io.kotest.core.spec.style.StringSpec
+import io.kotest.matchers.shouldBe
+import org.springframework.security.core.userdetails.UsernameNotFoundException
+import org.springframework.security.crypto.bcrypt.BCryptPasswordEncoder
+import org.springframework.web.cors.CorsConfiguration
+import org.springframework.mock.web.MockHttpServletRequest
+
+class AdminUserTest : StringSpec({
+    val passwordEncoder = BCryptPasswordEncoder(4)
+    fun securityConfig(allowedOrigins: String = "https://tormap.org") =
+        SecurityConfig("admin", "/actuator", allowedOrigins, false)
+
+    "an admin password creates the admin user with a hashed password" {
+        val users = securityConfig().buildUserDetailsService(passwordEncoder, "  a-long-generated-password  ", null)
+        val admin = users.loadUserByUsername("admin")
+        admin.authorities.map { it.authority } shouldBe listOf("ROLE_ADMIN")
+        // Surrounding whitespace, e.g. from an env file, is not part of the password
+        passwordEncoder.matches("a-long-generated-password", admin.password) shouldBe true
+        admin.password!!.startsWith("{") shouldBe false
+    }
+
+    "a short password still creates the admin user" {
+        val users = securityConfig().buildUserDetailsService(passwordEncoder, "short", null)
+        passwordEncoder.matches("short", users.loadUserByUsername("admin").password) shouldBe true
+    }
+
+    "without a password there is no user" {
+        listOf(null, "", "   ").forEach { password ->
+            val users = securityConfig().buildUserDetailsService(passwordEncoder, password, null)
+            shouldThrow<UsernameNotFoundException> { users.loadUserByUsername("admin") }
+        }
+    }
+
+    "the removed bcrypt variable does not create a user" {
+        val hash = passwordEncoder.encode("a-long-generated-password")
+        val users = securityConfig().buildUserDetailsService(passwordEncoder, null, hash)
+        shouldThrow<UsernameNotFoundException> { users.loadUserByUsername("admin") }
+    }
+
+    "allowed CORS origins are read from a comma separated list" {
+        val source = securityConfig(" https://a.example , ,https://*.b.example").corsConfigurationSource()
+        val cors: CorsConfiguration = source.getCorsConfiguration(MockHttpServletRequest("GET", "/relay/location/days"))!!
+        cors.allowedOriginPatterns shouldBe listOf("https://a.example", "https://*.b.example")
+        cors.checkOrigin("https://x.b.example") shouldBe "https://x.b.example"
+        cors.checkOrigin("https://evil.example") shouldBe null
+        cors.allowCredentials shouldBe false
+    }
+})

backend/src/test/kotlin/org/tormap/config/RequestBodyLimitFilterTest.kt

@@ -0,0 +1,83 @@
+package org.tormap.config
+
+import io.kotest.assertions.throwables.shouldThrow
+import io.kotest.core.spec.style.StringSpec
+import io.kotest.matchers.shouldBe
+import jakarta.servlet.FilterChain
+import jakarta.servlet.ReadListener
+import jakarta.servlet.ServletRequest
+import jakarta.servlet.http.HttpServletRequest
+import org.springframework.mock.web.MockHttpServletRequest
+import org.springframework.mock.web.MockHttpServletResponse
+import org.springframework.web.server.ContentTooLargeException
+import java.io.UnsupportedEncodingException
+
+class RequestBodyLimitFilterTest : StringSpec({
+    val filter = RequestBodyLimitFilter(maxBodyBytes = 10)
+
+    fun request(body: String, encoding: String? = null) = MockHttpServletRequest("POST", "/relay/details/relay/identifiers").apply {
+        setContent(body.toByteArray())
+        if (encoding != null) characterEncoding = encoding
+    }
+
+    fun filterWith(request: HttpServletRequest, read: (ServletRequest) -> Unit): MockHttpServletResponse {
+        val response = MockHttpServletResponse()
+        filter.doFilter(request, response, FilterChain { wrapped, _ -> read(wrapped) })
+        return response
+    }
+
+    "a declared Content-Length above the limit is rejected before the body is read" {
+        var called = false
+        val response = filterWith(request("x".repeat(11))) { called = true }
+        response.status shouldBe 413
+        called shouldBe false
+    }
+
+    "a body within the limit can be read as stream and as text" {
+        filterWith(request("0123456789")) { it.inputStream.readAllBytes().decodeToString() shouldBe "0123456789" }
+        filterWith(request("äöü", "UTF-8")) { it.reader.readText() shouldBe "äöü" }
+        // Without an encoding the servlet default ISO-8859-1 applies
+        filterWith(request("abc")) { it.reader.readText() shouldBe "abc" }
+    }
+
+    "reading beyond the limit fails, byte by byte as well as in blocks" {
+        // Without a known length, as with chunked transfer encoding
+        fun unknownLength(body: String) = object : MockHttpServletRequest("POST", "/") {
+            override fun getContentLengthLong() = -1L
+        }.apply { setContent(body.toByteArray()) }
+
+        filterWith(unknownLength("x".repeat(20))) { request ->
+            shouldThrow<ContentTooLargeException> { request.inputStream.readAllBytes() }
+        }
+        filterWith(unknownLength("x".repeat(11))) { request ->
+            val stream = request.inputStream
+            repeat(10) { stream.read() shouldBe 'x'.code }
+            shouldThrow<ContentTooLargeException> { stream.read() }
+        }
+        filterWith(unknownLength("")) { it.inputStream.read() shouldBe -1 }
+    }
+
+    "an unknown character encoding is reported like by the servlet API" {
+        filterWith(request("abc", "no-such-charset")) {
+            shouldThrow<UnsupportedEncodingException> { it.reader }
+        }
+    }
+
+    "the stream state and read listener are those of the request" {
+        filterWith(request("abc")) {
+            val stream = it.inputStream
+            stream.isFinished shouldBe false
+            stream.isReady shouldBe true
+            stream.readAllBytes()
+            stream.isFinished shouldBe true
+            // MockHttpServletRequest does not support async IO, which shows the call is delegated
+            shouldThrow<UnsupportedOperationException> {
+                stream.setReadListener(object : ReadListener {
+                    override fun onDataAvailable() = Unit
+                    override fun onAllDataRead() = Unit
+                    override fun onError(t: Throwable) = Unit
+                })
+            }
+        }
+    }
+})