From 72f705f4cd04c9399ef3eec82ced61b299d92f98 Mon Sep 17 00:00:00 2001 From: TimMasalme Date: Sun, 26 Jul 2026 19:57:54 +0200 Subject: [PATCH 01/12] Fix internal map paths in coop mission .scmap files on deploy getFileContent() only rewrote /maps// in text files, so the paths embedded in the .scmap binary kept pointing at the unversioned folder and the affected missions lost their custom textures after a release. ScmapPathFixer reads and rewrites the .scmap byte by byte, ported from speed2's sc_map_parser.gd, and inserts the release version into the map folder segment. Only paths that point at the mission's own folder are touched. Several missions deliberately reference a base game map (/maps/X1CA_001/X1CA_001.scmap) or a texture of another map, and versioning those would break them. A map file is only parsed at all when a raw byte scan finds "/maps/", which also keeps the placeholder .scmap files of the missions using a base game map out of the parser. Affects 4 of the 42 deployed missions: Operation_Blockade, Tha_Atha_Aez, Golden_Crystals and Overlord_Surth_Velsok. Their checksums change, so they get one version bump on the first run after this lands. Co-Authored-By: Claude Opus 5 --- .../scripts/CoopMapDeployer.kt | 15 + .../scripts/ScmapPathFixer.kt | 325 ++++++++++++++++++ 2 files changed, 340 insertions(+) create mode 100644 apps/faf-legacy-deployment/scripts/ScmapPathFixer.kt diff --git a/apps/faf-legacy-deployment/scripts/CoopMapDeployer.kt b/apps/faf-legacy-deployment/scripts/CoopMapDeployer.kt index 748c7b59..d36dfc7c 100644 --- a/apps/faf-legacy-deployment/scripts/CoopMapDeployer.kt +++ b/apps/faf-legacy-deployment/scripts/CoopMapDeployer.kt @@ -7,7 +7,9 @@ import com.faforever.FafDatabase import com.faforever.GitRepo import com.faforever.Log import com.faforever.extractChecksumsFromZip +import com.faforever.fixScmapPaths import com.faforever.generateChecksums +import com.faforever.referencesOwnMapFolder import org.apache.commons.compress.archivers.zip.ZipArchiveEntry import org.apache.commons.compress.archivers.zip.ZipArchiveOutputStream import org.slf4j.LoggerFactory @@ -180,6 +182,17 @@ private fun generateChecksumsForMap( * Get file content with path rewriting for text files. */ private fun getFileContent(file: Path, map: CoopMap, version: Int): ByteArray { + if (file.isScmapFile()) { + val bytes = file.readBytes() + // Only missions whose map references assets in their own folder need the version + // inserted. Everything else - including the placeholder .scmap files of the missions + // that use a base game map - is passed through and never parsed. + return if (bytes.referencesOwnMapFolder(map.folderName)) { + fixScmapPaths(bytes, map.folderName, version) + } else { + bytes + } + } return if (file.isTextFile()) { var text = file.readText() .replace( @@ -238,6 +251,8 @@ private fun createZip( private fun Path.isTextFile() = listOf(".md", ".lua", ".json", ".txt").any { toString().endsWith(it) } +private fun Path.isScmapFile() = toString().lowercase().endsWith(".scmap") + fun main(args: Array) { Log.init() diff --git a/apps/faf-legacy-deployment/scripts/ScmapPathFixer.kt b/apps/faf-legacy-deployment/scripts/ScmapPathFixer.kt new file mode 100644 index 00000000..c0ee3a8b --- /dev/null +++ b/apps/faf-legacy-deployment/scripts/ScmapPathFixer.kt @@ -0,0 +1,325 @@ +@file:Suppress("PackageDirectoryMismatch") + +package com.faforever + +import org.slf4j.LoggerFactory +import java.io.ByteArrayOutputStream + +private val log = LoggerFactory.getLogger("scmap-path-fixer") + +/** Every .scmap file starts with these 16 bytes. */ +private val SCMAP_HEADER = byteArrayOf( + 0x4D, 0x61, 0x70, 0x1A, // "Map\x1A" + 0x02, 0x00, 0x00, 0x00, // 2 + 0xED.toByte(), 0xFE.toByte(), 0xEF.toByte(), 0xBE.toByte(), // 0xBEEFFEED, little endian + 0x02, 0x00, 0x00, 0x00, // 2 +) + +fun ByteArray.isScmap(): Boolean = + size >= SCMAP_HEADER.size && SCMAP_HEADER.indices.all { this[it] == SCMAP_HEADER[it] } + +/** + * Case insensitive raw byte search for "/maps/", so we only parse map files + * that actually reference their own folder. Missions whose .scmap contains no such path + * (and the placeholder files that are not maps at all) are passed through untouched. + */ +fun ByteArray.referencesOwnMapFolder(folderName: String): Boolean { + val needle = "/maps/${folderName.lowercase()}".toByteArray(Charsets.ISO_8859_1) + if (needle.size > size) return false + outer@ for (i in 0..size - needle.size) { + for (j in needle.indices) { + var b = this[i + j].toInt() and 0xFF + if (b in 0x41..0x5A) b += 0x20 // ASCII toLowerCase + if (b != (needle[j].toInt() and 0xFF)) continue@outer + } + return true + } + return false +} + +/** + * Rewrites the map folder segment of every path embedded in a .scmap so that it carries + * the release version: + * + * /maps/faf_coop_operation_blockade/env/layers/sand.dds + * -> /maps/faf_coop_operation_blockade.v0004/env/layers/sand.dds + * + * Only paths that point at [folderName] itself are touched. Paths pointing somewhere else + * are left alone and logged - several missions deliberately reference a base game map + * (`/maps/X1CA_001/X1CA_001.scmap`) or another map's texture, and versioning those would + * break them. + * + * A .scmap is tightly packed with no field names, so every field has to be read and + * written back in the exact same order. Most strings are null terminated; decal texture + * paths are the exception and carry a leading int with their length. Arrays of structs + * (decals, waves, props) start with an int count. Which sections exist depends on the map + * file version - the coop missions use 53, 56 and 60. + * + * Ported from speed2's `sc_map_parser.gd` (`fix_paths`). + * + * @param version release version, or a negative value to copy the file without changes + * (used as a self check: the parser then has to reproduce the input byte + * for byte) + * @throws IllegalArgumentException if the file is not a .scmap + * @throws IllegalStateException if the result fails verification + */ +fun fixScmapPaths(bytes: ByteArray, folderName: String, version: Int): ByteArray { + require(bytes.isScmap()) { "$folderName: not a .scmap file, header mismatch" } + + val suffix = if (version >= 0) ".v%04d".format(version) else "" + val rewriter = ScmapRewriter(bytes, folderName, suffix) + val result = rewriter.rewrite() + + if (suffix.isEmpty()) return result + + rewriter.skipped.distinct().forEach { + log.warn("$folderName: path leads outside the mission folder, left unchanged: $it") + } + log.info( + "$folderName: rewrote {} path(s) in {}, {} byte(s) added", + rewriter.rewritten, folderName, result.size - bytes.size + ) + + check(result.size - bytes.size == rewriter.addedBytes) { + "$folderName: byte delta ${result.size - bytes.size} does not match the " + + "${rewriter.addedBytes} byte(s) added to paths" + } + // the rewritten file has to parse again and come out byte identical + val verified = ScmapRewriter(result, folderName, "").rewrite() + check(verified.contentEquals(result)) { + "$folderName: rewritten .scmap does not round trip, refusing to ship it" + } + return result +} + +private class ScmapRewriter( + private val src: ByteArray, + folderName: String, + private val suffix: String, +) { + private val folder = folderName.lowercase() + private val out = ByteArrayOutputStream(src.size + 1024) + private var pos = 0 + + var rewritten = 0 + private set + var addedBytes = 0 + private set + val skipped = mutableListOf() + + fun rewrite(): ByteArray { + copy(16) // header + copy(14) // float size x, y + 6 padding bytes + transferSizedChunk() // preview image + + val version = readInt() + val width = readInt() + val height = readInt() + writeInt(version) + writeInt(width) + writeInt(height) + copy(4) // height scale + + copy((width + 1) * (height + 1) * 2) // heightmap + if (version >= 56) copy(1) // padding after the heightmap + + transferString() // shader name, not a path + transferString(true) // editor background + transferString(true) // skycube + + if (version >= 56) { + val cubemaps = readInt() + writeInt(cubemaps) + repeat(cubemaps) { + transferString() // name + transferString(true) // path + } + } else { + transferString(true) // old format has a single cubemap + } + + copy(23 * 4) // lighting + copy(1 + 23 * 4) // water flag + water settings + transferString(true) // water cubemap + transferString(true) // water ramp + copy(4 * 4) // wave normal repeats + repeat(4) { // wave textures + copy(8) // movement + transferString(true) + } + + val waves = readInt() + writeInt(waves) + repeat(waves) { + transferString() // texture name, not a path + transferString() // ramp name, not a path + copy(17 * 4) + } + + if (version < 56) { + transferString() // always "No Tileset" + val tilesets = readInt() // always 6 + writeInt(tilesets) + repeat(tilesets) { + transferString(true) // albedo + transferString(true) // normal + copy(2 * 4) // scales + } + } else { + copy(6 * 4) // minimap + if (version > 56) copy(4) // unknown + repeat(19) { // 10 albedo + 9 normal + transferString(true) + copy(4) // scale + } + } + + copy(8) // 2 unknown values + + val decals = readInt() + writeInt(decals) + repeat(decals) { + copy(8) // id + type + val textures = readInt() + writeInt(textures) + repeat(textures) { transferSizedString() } + copy(12 * 4) // 11 floats + 1 int + } + + val decalGroups = readInt() + writeInt(decalGroups) + repeat(decalGroups) { + copy(4) // id + transferString() // name, not a path + val ids = readInt() + writeInt(ids) + if (ids > 0) copy(ids * 4) + } + + copy(8) // int width + height + + val normalMaps = readInt() + writeInt(normalMaps) + repeat(normalMaps) { transferSizedChunk() } + + if (version < 56) copy(4) // unknown in the old format + + transferSizedChunk() // texture mask low + if (version >= 56) transferSizedChunk() // texture mask high + + val waterMaps = readInt() + writeInt(waterMaps) + repeat(waterMaps) { transferSizedChunk() } + + val halfSize = (width / 2) * (height / 2) + repeat(3) { copy(halfSize) } // water foam, flatness, depth bias + copy(width * height) // terrain types + + if (version <= 52) copy(2) // unknown in the oldest format + + if (version >= 60) { + copy(16 * 4) // skybox settings + transferString(true) // albedo + transferString(true) // glow + val planets = readInt() + writeInt(planets) + repeat(planets) { copy(10 * 4) } + copy(3 + 4 * 4) // sky mid color + cirrus + transferString(true) // cirrus texture + val cirrusLayers = readInt() + writeInt(cirrusLayers) + repeat(cirrusLayers) { copy(5 * 4) } + copy(4) // one more setting + } + + val props = readInt() + writeInt(props) + repeat(props) { + transferString(true) // blueprint path + copy(15 * 4) + } + + if (pos < src.size) { + log.warn("$folder: {} trailing byte(s) after the props, copied unchanged", src.size - pos) + copy(src.size - pos) + } + return out.toByteArray() + } + + /** + * If [path] points into this mission's own folder inside /maps, the version is added to + * the folder name. Paths are lower cased, which is what the maps deployed so far look + * like. Everything else is returned untouched. + */ + private fun addVersion(path: String): String { + if (suffix.isEmpty()) return path + val lower = path.lowercase() + val parts = lower.split('/').filter { it.isNotEmpty() } + // expected: ["maps", "map_name", "env", ...] + if (parts.size < 3 || parts[0] != "maps") return lower + + val segment = parts[1].replace(VERSIONED, "") + if (segment != folder) { + skipped += path + return lower + } + + rewritten++ + addedBytes += suffix.length - (parts[1].length - segment.length) + return "/" + parts.mapIndexed { i, part -> if (i == 1) segment + suffix else part } + .joinToString("/") + } + + /** Null terminated string. [isPath] marks the ones that may need the version. */ + private fun transferString(isPath: Boolean = false) { + val start = pos + while (pos < src.size && src[pos] != 0.toByte()) pos++ + val raw = String(src, start, pos - start, Charsets.ISO_8859_1) + pos++ // null terminator + val value = if (isPath) addVersion(raw) else raw + out.write(value.toByteArray(Charsets.ISO_8859_1)) + out.write(0) + } + + /** Decal texture paths are not null terminated but prefixed with their length. */ + private fun transferSizedString() { + val length = readInt() + val raw = String(src, pos, length, Charsets.ISO_8859_1) + pos += length + val encoded = addVersion(raw).toByteArray(Charsets.ISO_8859_1) + writeInt(encoded.size) + out.write(encoded) + } + + /** Chunk of bytes whose length is read from the file, used for embedded textures. */ + private fun transferSizedChunk() { + val length = readInt() + writeInt(length) + if (length > 0) copy(length) + } + + private fun readInt(): Int { + val v = (src[pos].toInt() and 0xFF) or + ((src[pos + 1].toInt() and 0xFF) shl 8) or + ((src[pos + 2].toInt() and 0xFF) shl 16) or + ((src[pos + 3].toInt() and 0xFF) shl 24) + pos += 4 + return v + } + + private fun writeInt(value: Int) { + out.write(value and 0xFF) + out.write((value ushr 8) and 0xFF) + out.write((value ushr 16) and 0xFF) + out.write((value ushr 24) and 0xFF) + } + + private fun copy(length: Int) { + out.write(src, pos, length) + pos += length + } + + private companion object { + val VERSIONED = Regex("""\.v\d{4}$""") + } +} From 92e5625f13aaca98260e746dd75e7913ab1aef60 Mon Sep 17 00:00:00 2001 From: TimMasalme Date: Mon, 27 Jul 2026 02:10:58 +0200 Subject: [PATCH 02/12] Verify a coop map release before it is written Two things could still go wrong silently. A path could be rewritten into something structurally valid but wrong - an off by one on the version, the wrong folder - and the .scmap round trip would happily accept it. And a failure while building the zip left a partial file behind in the served maps directory. verifyRelease() resolves every path the fixer rewrote against the files that actually end up in the zip. If one does not resolve, the mission is not deployed: no zip, no database update, the previous version stays in place. Checked against the current missions, this catches all 40 rewritten paths as broken when the rewriting is skipped, which is exactly the bug this branch fixes. References in text files are only reported, not enforced. Three of them have been broken for years - typos in comment headers, one stale hardcoded version - and failing on those would block releases for cosmetic reasons. The zip is now written to .part and moved into place afterwards, so a failure cannot leave a half written archive where the game serves maps from. Co-Authored-By: Claude Opus 5 --- .../scripts/CoopMapDeployer.kt | 69 ++++++++++++++++++- .../scripts/ScmapPathFixer.kt | 39 +++++++---- 2 files changed, 94 insertions(+), 14 deletions(-) diff --git a/apps/faf-legacy-deployment/scripts/CoopMapDeployer.kt b/apps/faf-legacy-deployment/scripts/CoopMapDeployer.kt index d36dfc7c..e0cd1f7e 100644 --- a/apps/faf-legacy-deployment/scripts/CoopMapDeployer.kt +++ b/apps/faf-legacy-deployment/scripts/CoopMapDeployer.kt @@ -9,6 +9,7 @@ import com.faforever.Log import com.faforever.extractChecksumsFromZip import com.faforever.fixScmapPaths import com.faforever.generateChecksums +import com.faforever.isScmap import com.faforever.referencesOwnMapFolder import org.apache.commons.compress.archivers.zip.ZipArchiveEntry import org.apache.commons.compress.archivers.zip.ZipArchiveOutputStream @@ -16,6 +17,7 @@ import org.slf4j.LoggerFactory import java.nio.file.Files import java.nio.file.Path import java.nio.file.Paths +import java.nio.file.StandardCopyOption import kotlin.io.path.copyTo import kotlin.io.path.createDirectories import kotlin.io.path.isDirectory @@ -153,9 +155,17 @@ private fun processCoopMap( val newVersion = currentVersion + 1 log.info("$map updated → v$newVersion") + verifyRelease(map, newVersion, files, tmp) + if (!simulate) { val finalZip = Path.of(mapsDir, map.zipName(newVersion)) - createZip(map, newVersion, files, tmp, finalZip) + val partialZip = Path.of(mapsDir, "${map.zipName(newVersion)}.part") + try { + createZip(map, newVersion, files, tmp, partialZip) + Files.move(partialZip, finalZip, StandardCopyOption.REPLACE_EXISTING) + } finally { + Files.deleteIfExists(partialZip) + } db.update(map, newVersion) } } finally { @@ -188,7 +198,7 @@ private fun getFileContent(file: Path, map: CoopMap, version: Int): ByteArray { // inserted. Everything else - including the placeholder .scmap files of the missions // that use a base game map - is passed through and never parsed. return if (bytes.referencesOwnMapFolder(map.folderName)) { - fixScmapPaths(bytes, map.folderName, version) + fixScmapPaths(bytes, map.folderName, version).bytes } else { bytes } @@ -209,6 +219,61 @@ private fun getFileContent(file: Path, map: CoopMap, version: Int): ByteArray { } } +/** + * Checks that every path the release points at actually exists in the release. + * + * The paths rewritten inside a .scmap are checked hard: if one of them does not resolve to + * a file that ends up in the zip, the mission is not deployed at all. That is the case the + * whole path rewriting exists for, and getting it wrong ships a map with missing textures + * that nothing else would notice. + * + * References in text files are only reported. Some of them have been broken for years - + * typos in comment headers - and failing on those would block releases for cosmetic reasons. + * + * @throws IllegalStateException if a rewritten map path does not resolve + */ +private fun verifyRelease(map: CoopMap, version: Int, files: List, base: Path) { + val prefix = "/maps/${map.folderName(version)}/".lowercase() + val shipped = files + .map { prefix + base.relativize(it).toString().replace("\\", "/").lowercase() } + .toSet() + + // over capture is possible when a path is read out of binary data, so a reference counts + // as resolved when it starts with a shipped file + fun resolves(reference: String) = + shipped.any { reference.lowercase() == it || reference.lowercase().startsWith(it) } + + val broken = mutableListOf() + + files.forEach { file -> + val relative = base.relativize(file).toString().replace("\\", "/") + + if (file.isScmapFile()) { + val bytes = file.readBytes() + if (bytes.isScmap() && bytes.referencesOwnMapFolder(map.folderName)) { + fixScmapPaths(bytes, map.folderName, version).rewritten + .filterNot(::resolves) + .forEach { broken += "$relative points at $it, which is not in the release" } + } + } else if (file.isTextFile()) { + val text = String(getFileContent(file, map, version), Charsets.ISO_8859_1) + MAP_REFERENCE.findAll(text) + .map { it.value } + .filter { it.lowercase().startsWith("/maps/${map.folderName.lowercase()}") } + .filterNot(::resolves) + .distinct() + .forEach { log.warn("$map: $relative points at $it, which is not in the release") } + } + } + + check(broken.isEmpty()) { + broken.forEach { log.error("$map: $it") } + "$map: ${broken.size} rewritten map path(s) do not resolve, not deploying this mission" + } +} + +private val MAP_REFERENCE = Regex("""/maps/[^"'\s,)]+""", RegexOption.IGNORE_CASE) + private fun createZip( map: CoopMap, version: Int, diff --git a/apps/faf-legacy-deployment/scripts/ScmapPathFixer.kt b/apps/faf-legacy-deployment/scripts/ScmapPathFixer.kt index c0ee3a8b..9d9bdbbf 100644 --- a/apps/faf-legacy-deployment/scripts/ScmapPathFixer.kt +++ b/apps/faf-legacy-deployment/scripts/ScmapPathFixer.kt @@ -37,6 +37,19 @@ fun ByteArray.referencesOwnMapFolder(folderName: String): Boolean { return false } +/** + * Result of [fixScmapPaths]. + * + * @property bytes the rewritten map file + * @property rewritten every path that received the version, as written into [bytes] + * @property skipped paths inside /maps that lead to another folder and were left alone + */ +class ScmapPathFix( + val bytes: ByteArray, + val rewritten: List, + val skipped: List, +) + /** * Rewrites the map folder segment of every path embedded in a .scmap so that it carries * the release version: @@ -45,7 +58,8 @@ fun ByteArray.referencesOwnMapFolder(folderName: String): Boolean { * -> /maps/faf_coop_operation_blockade.v0004/env/layers/sand.dds * * Only paths that point at [folderName] itself are touched. Paths pointing somewhere else - * are left alone and logged - several missions deliberately reference a base game map + * are left alone and reported in [ScmapPathFix.skipped] - several missions deliberately + * reference a base game map * (`/maps/X1CA_001/X1CA_001.scmap`) or another map's texture, and versioning those would * break them. * @@ -63,21 +77,22 @@ fun ByteArray.referencesOwnMapFolder(folderName: String): Boolean { * @throws IllegalArgumentException if the file is not a .scmap * @throws IllegalStateException if the result fails verification */ -fun fixScmapPaths(bytes: ByteArray, folderName: String, version: Int): ByteArray { +fun fixScmapPaths(bytes: ByteArray, folderName: String, version: Int): ScmapPathFix { require(bytes.isScmap()) { "$folderName: not a .scmap file, header mismatch" } val suffix = if (version >= 0) ".v%04d".format(version) else "" val rewriter = ScmapRewriter(bytes, folderName, suffix) val result = rewriter.rewrite() + val fix = ScmapPathFix(result, rewriter.rewritten.toList(), rewriter.skipped.distinct()) - if (suffix.isEmpty()) return result + if (suffix.isEmpty()) return fix - rewriter.skipped.distinct().forEach { + fix.skipped.forEach { log.warn("$folderName: path leads outside the mission folder, left unchanged: $it") } log.info( - "$folderName: rewrote {} path(s) in {}, {} byte(s) added", - rewriter.rewritten, folderName, result.size - bytes.size + "$folderName: rewrote {} path(s) in the map file, {} byte(s) added", + fix.rewritten.size, result.size - bytes.size ) check(result.size - bytes.size == rewriter.addedBytes) { @@ -89,7 +104,7 @@ fun fixScmapPaths(bytes: ByteArray, folderName: String, version: Int): ByteArray check(verified.contentEquals(result)) { "$folderName: rewritten .scmap does not round trip, refusing to ship it" } - return result + return fix } private class ScmapRewriter( @@ -101,11 +116,10 @@ private class ScmapRewriter( private val out = ByteArrayOutputStream(src.size + 1024) private var pos = 0 - var rewritten = 0 - private set + val rewritten = mutableListOf() + val skipped = mutableListOf() var addedBytes = 0 private set - val skipped = mutableListOf() fun rewrite(): ByteArray { copy(16) // header @@ -264,10 +278,11 @@ private class ScmapRewriter( return lower } - rewritten++ addedBytes += suffix.length - (parts[1].length - segment.length) - return "/" + parts.mapIndexed { i, part -> if (i == 1) segment + suffix else part } + val fixed = "/" + parts.mapIndexed { i, part -> if (i == 1) segment + suffix else part } .joinToString("/") + rewritten += fixed + return fixed } /** Null terminated string. [isPath] marks the ones that may need the version. */ From 7df0bd025fa38e30c5294e6c47a9900656fa0b08 Mon Sep 17 00:00:00 2001 From: TimMasalme Date: Mon, 27 Jul 2026 02:11:13 +0200 Subject: [PATCH 03/12] Check the coop map path fixer in CI Compiles the deployment scripts and runs the .scmap path fixer over every mission in faf-coop-maps, in the same image the CronJobs use. Only runs when the scripts change. The load bearing assertion is that rewriting with a negative version reproduces the input byte for byte. It runs over all missions, not only the four that need the fix, so it also covers the old file formats - the coop missions use three of them (53, 56 and 60) and scoping a section to the wrong one breaks exactly those and nothing else. Co-Authored-By: Claude Opus 5 --- .github/workflows/coop-deployment-scripts.yml | 38 ++++++ .../scripts/ScmapPathFixerCheck.kt | 108 ++++++++++++++++++ .../scripts/build.gradle.kts | 8 ++ 3 files changed, 154 insertions(+) create mode 100644 .github/workflows/coop-deployment-scripts.yml create mode 100644 apps/faf-legacy-deployment/scripts/ScmapPathFixerCheck.kt diff --git a/.github/workflows/coop-deployment-scripts.yml b/.github/workflows/coop-deployment-scripts.yml new file mode 100644 index 00000000..1ac78582 --- /dev/null +++ b/.github/workflows/coop-deployment-scripts.yml @@ -0,0 +1,38 @@ +name: Coop deployment scripts + +on: + push: + paths: + - 'apps/faf-legacy-deployment/scripts/**' + - '.github/workflows/coop-deployment-scripts.yml' + pull_request: + paths: + - 'apps/faf-legacy-deployment/scripts/**' + - '.github/workflows/coop-deployment-scripts.yml' + +jobs: + verify: + + runs-on: ubuntu-latest + container: + # same image the deployment CronJobs use + image: gradle:9.4-jdk21 + + steps: + - uses: actions/checkout@v6 + + - name: Check out the coop missions + uses: actions/checkout@v6 + with: + repository: FAForever/faf-coop-maps + path: faf-coop-maps + + - name: Compile + working-directory: apps/faf-legacy-deployment/scripts + run: gradle --no-daemon compileKotlin + + - name: Round trip the path fixer over every mission map + working-directory: apps/faf-legacy-deployment/scripts + env: + MAPS_REPO: ${{ github.workspace }}/faf-coop-maps + run: gradle --no-daemon verifyScmapFixer diff --git a/apps/faf-legacy-deployment/scripts/ScmapPathFixerCheck.kt b/apps/faf-legacy-deployment/scripts/ScmapPathFixerCheck.kt new file mode 100644 index 00000000..7c216ee8 --- /dev/null +++ b/apps/faf-legacy-deployment/scripts/ScmapPathFixerCheck.kt @@ -0,0 +1,108 @@ +@file:Suppress("PackageDirectoryMismatch") + +package com.faforever + +import java.io.File +import kotlin.system.exitProcess + +/** + * Runs [fixScmapPaths] over every map file in a checkout of the coop missions and fails if + * anything is off. Meant for CI, so a change to the path rewriting cannot quietly break a + * map file. + * + * The important one is the identity check: rewriting with a negative version has to + * reproduce the input byte for byte. It covers every mission, not only the handful that + * need the fix, and therefore also the old file formats. Scoping a section to the wrong + * file version breaks exactly those and nothing else. + * + * Point MAPS_REPO at a checkout of https://github.com/FAForever/faf-coop-maps. + */ +fun main() { + val repo = File(System.getenv("MAPS_REPO") ?: "/tmp/faf-coop-maps") + require(repo.isDirectory) { "MAPS_REPO does not point at a directory: $repo" } + + val maps = repo.walkTopDown() + .filter { it.isFile && it.name.lowercase().endsWith(".scmap") } + .sortedBy { it.path } + .toList() + require(maps.isNotEmpty()) { "no .scmap files found below $repo" } + + val failures = mutableListOf() + var placeholders = 0 + var identical = 0 + var rewrittenMaps = 0 + + println("%-44s %-8s %-10s %s".format("mission", "format", "size", "result")) + + maps.forEach { file -> + val folder = file.parentFile.name + val bytes = file.readBytes() + + if (!bytes.isScmap()) { + placeholders++ + println("%-44s %-8s %-10s %s".format(folder, "-", bytes.size, "not a map file, skipped")) + return@forEach + } + + val notes = mutableListOf() + + try { + val copy = fixScmapPaths(bytes, folder, -1).bytes + if (copy.contentEquals(bytes)) { + identical++ + notes += "identical" + } else { + failures += "$folder: copy is ${copy.size} bytes, input is ${bytes.size}" + notes += "NOT IDENTICAL" + } + } catch (e: Exception) { + failures += "$folder: ${e.message}" + notes += "FAILED: ${e.message}" + } + + if (bytes.referencesOwnMapFolder(folder)) { + try { + // delta accounting and the round trip are asserted inside fixScmapPaths + val fix = fixScmapPaths(bytes, folder, 4) + rewrittenMaps++ + notes += "${fix.rewritten.size} path(s) rewritten" + + fix.rewritten.filterNot { it.contains(".v0004/") } + .forEach { failures += "$folder: not versioned: $it" } + fix.rewritten.filter { it != it.lowercase() } + .forEach { failures += "$folder: not lower cased: $it" } + fix.rewritten.filter { DOUBLE_VERSION.containsMatchIn(it) } + .forEach { failures += "$folder: version added twice: $it" } + } catch (e: Exception) { + failures += "$folder: ${e.message}" + notes += "FAILED: ${e.message}" + } + } + + println("%-44s %-8s %-10s %s".format(folder, "v" + formatVersion(bytes), bytes.size, notes.joinToString(", "))) + } + + println() + println("$identical of ${maps.size - placeholders} map file(s) reproduced byte for byte") + println("$rewrittenMaps map file(s) reference their own folder and were rewritten") + println("$placeholders placeholder file(s) skipped") + + if (failures.isNotEmpty()) { + println() + failures.forEach { println("FAIL $it") } + println("${failures.size} failure(s)") + exitProcess(1) + } + println("all good") +} + +private val DOUBLE_VERSION = Regex("""\.v\d{4}\.v\d{4}""") + +/** Map file format version, stored behind the preview image. */ +private fun formatVersion(bytes: ByteArray): Int { + fun int(at: Int) = (bytes[at].toInt() and 0xFF) or + ((bytes[at + 1].toInt() and 0xFF) shl 8) or + ((bytes[at + 2].toInt() and 0xFF) shl 16) or + ((bytes[at + 3].toInt() and 0xFF) shl 24) + return int(34 + int(30)) +} diff --git a/apps/faf-legacy-deployment/scripts/build.gradle.kts b/apps/faf-legacy-deployment/scripts/build.gradle.kts index 1dd3ffc9..e7f6ba82 100644 --- a/apps/faf-legacy-deployment/scripts/build.gradle.kts +++ b/apps/faf-legacy-deployment/scripts/build.gradle.kts @@ -36,4 +36,12 @@ tasks.register("deployCoopMaps") { classpath = sourceSets.main.get().runtimeClasspath mainClass.set("com.faforever.coopmapdeployer.CoopMapDeployerKt") +} + +tasks.register("verifyScmapFixer") { + group = "verification" + description = "Run the .scmap path fixer over every map in a faf-coop-maps checkout (MAPS_REPO)" + + classpath = sourceSets.main.get().runtimeClasspath + mainClass.set("com.faforever.ScmapPathFixerCheckKt") } \ No newline at end of file From 2926d6c1b559ae4bb81d2eba0133e16b3edab27b Mon Sep 17 00:00:00 2001 From: SeraphimNoob01 Date: Sun, 16 Aug 2026 06:09:32 +0100 Subject: [PATCH 04/12] fix(coop-maps): harden the .scmap path rewriting Only rewrite what has to be rewritten. addVersion() used to lower case every string handed to it, including ones that are not /maps/ paths at all - and it is also handed whatever a mis-framed transferString() believes to be a string. Lower casing binary data flips bytes without changing the length, so neither the delta accounting nor the round trip could have caught it. Paths that do not point into the mission folder are now returned byte for byte as they came in; they are shipped unparsed today and work. Rebuilding the path from its segments also normalised its shape: a missing leading slash, a double slash or a trailing slash changed the length by more than the suffix, and the delta check refused the mission. The folder segment is now spliced into the original string instead, and addedBytes is the real per-path delta, which leaves the delta check testing what it is meant to test - that the structural pass neither lost nor invented bytes. Add the check that was missing: no unversioned /maps// may survive in the output. The delta accounting and the round trip are both blind to a rewrite that never happened, which is the exact bug this class exists to fix. Fold the isScmap() + referencesOwnMapFolder() pair into needsPathFix(), so getFileContent() and verifyRelease() cannot disagree on which files get parsed - getFileContent() was missing the isScmap() guard and would have thrown on a placeholder that mentions its own folder. A refused release was a WARN in a job that exits 0. Log it as an error and exit non zero once every other mission has been processed. CI: cross check referencesOwnMapFolder against an independent search, so a regression to "false" cannot skip every versioned check and still pass; fail when nothing was rewritten at all; report the format versions and fail on one the rewriter has never been run against. Drop the claim that the identity check catches wrong section scoping - without a suffix the rewriter is a byte copier by construction, so it only proves that no read ran off the end. Also: contents: read and persist-credentials: false for the workflow, and an atomic move for the finished zip. Co-Authored-By: Claude Opus 5 --- .github/workflows/coop-deployment-scripts.yml | 8 +++ .../scripts/CoopMapDeployer.kt | 22 ++++-- .../scripts/ScmapPathFixer.kt | 69 ++++++++++++------- .../scripts/ScmapPathFixerCheck.kt | 62 +++++++++++++---- .../scripts/build.gradle.kts | 2 +- 5 files changed, 121 insertions(+), 42 deletions(-) diff --git a/.github/workflows/coop-deployment-scripts.yml b/.github/workflows/coop-deployment-scripts.yml index 1ac78582..a72d6ccc 100644 --- a/.github/workflows/coop-deployment-scripts.yml +++ b/.github/workflows/coop-deployment-scripts.yml @@ -10,6 +10,10 @@ on: - 'apps/faf-legacy-deployment/scripts/**' - '.github/workflows/coop-deployment-scripts.yml' +# the job builds and runs repository controlled Kotlin, so it gets nothing but read access +permissions: + contents: read + jobs: verify: @@ -20,12 +24,16 @@ jobs: steps: - uses: actions/checkout@v6 + with: + # nothing here pushes, so the job token has no business staying in .git/config + persist-credentials: false - name: Check out the coop missions uses: actions/checkout@v6 with: repository: FAForever/faf-coop-maps path: faf-coop-maps + persist-credentials: false - name: Compile working-directory: apps/faf-legacy-deployment/scripts diff --git a/apps/faf-legacy-deployment/scripts/CoopMapDeployer.kt b/apps/faf-legacy-deployment/scripts/CoopMapDeployer.kt index e0cd1f7e..638a2847 100644 --- a/apps/faf-legacy-deployment/scripts/CoopMapDeployer.kt +++ b/apps/faf-legacy-deployment/scripts/CoopMapDeployer.kt @@ -9,8 +9,7 @@ import com.faforever.Log import com.faforever.extractChecksumsFromZip import com.faforever.fixScmapPaths import com.faforever.generateChecksums -import com.faforever.isScmap -import com.faforever.referencesOwnMapFolder +import com.faforever.needsPathFix import org.apache.commons.compress.archivers.zip.ZipArchiveEntry import org.apache.commons.compress.archivers.zip.ZipArchiveOutputStream import org.slf4j.LoggerFactory @@ -25,6 +24,7 @@ import kotlin.io.path.isRegularFile import kotlin.io.path.readBytes import kotlin.io.path.readText import kotlin.io.path.walk +import kotlin.system.exitProcess private val log = LoggerFactory.getLogger("coop-maps-updater") @@ -162,7 +162,7 @@ private fun processCoopMap( val partialZip = Path.of(mapsDir, "${map.zipName(newVersion)}.part") try { createZip(map, newVersion, files, tmp, partialZip) - Files.move(partialZip, finalZip, StandardCopyOption.REPLACE_EXISTING) + Files.move(partialZip, finalZip, StandardCopyOption.ATOMIC_MOVE) } finally { Files.deleteIfExists(partialZip) } @@ -197,7 +197,7 @@ private fun getFileContent(file: Path, map: CoopMap, version: Int): ByteArray { // Only missions whose map references assets in their own folder need the version // inserted. Everything else - including the placeholder .scmap files of the missions // that use a base game map - is passed through and never parsed. - return if (bytes.referencesOwnMapFolder(map.folderName)) { + return if (bytes.needsPathFix(map.folderName)) { fixScmapPaths(bytes, map.folderName, version).bytes } else { bytes @@ -250,7 +250,7 @@ private fun verifyRelease(map: CoopMap, version: Int, files: List, base: P if (file.isScmapFile()) { val bytes = file.readBytes() - if (bytes.isScmap() && bytes.referencesOwnMapFolder(map.folderName)) { + if (bytes.needsPathFix(map.folderName)) { fixScmapPaths(bytes, map.folderName, version).rewritten .filterNot(::resolves) .forEach { broken += "$relative points at $it, which is not in the release" } @@ -338,13 +338,23 @@ fun main(args: Array) { gitRef = GIT_REF, ).checkout() + val failed = mutableListOf() + CoopMapDatabase(dryRun = DRYRUN).use { db -> coopMaps.forEach { try { processCoopMap(db, it, DRYRUN, WORKDIR, MAP_DIR) } catch (e: Exception) { - log.warn("Failed processing $it", e) + failed += it + log.error("Failed processing $it", e) } } } + + // one mission failing must not stop the others, but it may not pass for a successful run + // either - a refused release is only visible in the logs otherwise + if (failed.isNotEmpty()) { + log.error("{} mission(s) were not deployed: {}", failed.size, failed.joinToString { it.folderName }) + exitProcess(1) + } } diff --git a/apps/faf-legacy-deployment/scripts/ScmapPathFixer.kt b/apps/faf-legacy-deployment/scripts/ScmapPathFixer.kt index 9d9bdbbf..3df32a23 100644 --- a/apps/faf-legacy-deployment/scripts/ScmapPathFixer.kt +++ b/apps/faf-legacy-deployment/scripts/ScmapPathFixer.kt @@ -18,25 +18,31 @@ private val SCMAP_HEADER = byteArrayOf( fun ByteArray.isScmap(): Boolean = size >= SCMAP_HEADER.size && SCMAP_HEADER.indices.all { this[it] == SCMAP_HEADER[it] } -/** - * Case insensitive raw byte search for "/maps/", so we only parse map files - * that actually reference their own folder. Missions whose .scmap contains no such path - * (and the placeholder files that are not maps at all) are passed through untouched. - */ -fun ByteArray.referencesOwnMapFolder(folderName: String): Boolean { - val needle = "/maps/${folderName.lowercase()}".toByteArray(Charsets.ISO_8859_1) - if (needle.size > size) return false - outer@ for (i in 0..size - needle.size) { - for (j in needle.indices) { +/** Case insensitive raw byte search for an ASCII needle. */ +fun ByteArray.containsAscii(needle: String): Boolean { + val bytes = needle.lowercase().toByteArray(Charsets.ISO_8859_1) + if (bytes.size > size) return false + outer@ for (i in 0..size - bytes.size) { + for (j in bytes.indices) { var b = this[i + j].toInt() and 0xFF if (b in 0x41..0x5A) b += 0x20 // ASCII toLowerCase - if (b != (needle[j].toInt() and 0xFF)) continue@outer + if (b != (bytes[j].toInt() and 0xFF)) continue@outer } return true } return false } +/** + * Whether the map references its own folder, so we only parse map files that actually need + * the fix. Missions whose .scmap contains no such path (and the placeholder files that are + * not maps at all) are passed through untouched. + */ +fun ByteArray.referencesOwnMapFolder(folderName: String) = containsAscii("/maps/$folderName") + +/** A .scmap only needs rewriting when it is one and it points into its own folder. */ +fun ByteArray.needsPathFix(folderName: String) = isScmap() && referencesOwnMapFolder(folderName) + /** * Result of [fixScmapPaths]. * @@ -95,10 +101,20 @@ fun fixScmapPaths(bytes: ByteArray, folderName: String, version: Int): ScmapPath fix.rewritten.size, result.size - bytes.size ) + // the file may only have grown by exactly what went into the paths - anything else means + // the structural pass lost or invented bytes somewhere between them check(result.size - bytes.size == rewriter.addedBytes) { "$folderName: byte delta ${result.size - bytes.size} does not match the " + "${rewriter.addedBytes} byte(s) added to paths" } + // no unversioned reference to the mission folder may survive. This is the one check that + // catches a path the parser never recognised as a path in the first place - the delta + // accounting and the round trip below are both blind to a rewrite that simply did not + // happen, which is exactly the bug this whole class exists to fix. + check(!result.containsAscii("/maps/$folderName/")) { + "$folderName: the rewritten .scmap still contains unversioned /maps/$folderName/ " + + "path(s), refusing to ship it" + } // the rewritten file has to parse again and come out byte identical val verified = ScmapRewriter(result, folderName, "").rewrite() check(verified.contentEquals(result)) { @@ -262,25 +278,29 @@ private class ScmapRewriter( /** * If [path] points into this mission's own folder inside /maps, the version is added to - * the folder name. Paths are lower cased, which is what the maps deployed so far look - * like. Everything else is returned untouched. + * the folder name and the path is lower cased - that is what `fix_paths` does and what + * the maps deployed so far look like. + * + * Everything else is returned byte for byte as it came in, including paths under /maps + * that lead to another folder. Those are shipped unparsed today and work, so there is + * nothing to gain by touching them - and something to lose: this method is also handed + * whatever a mis-framed [transferString] believes to be a string, and lower casing that + * would silently flip bytes inside binary data without changing the length, which no + * check in [fixScmapPaths] could catch. */ private fun addVersion(path: String): String { if (suffix.isEmpty()) return path - val lower = path.lowercase() - val parts = lower.split('/').filter { it.isNotEmpty() } - // expected: ["maps", "map_name", "env", ...] - if (parts.size < 3 || parts[0] != "maps") return lower + val match = MAP_PATH.matchEntire(path) ?: return path + val (prefix, segment, rest) = match.destructured - val segment = parts[1].replace(VERSIONED, "") - if (segment != folder) { + val bare = segment.lowercase().replace(VERSIONED, "") + if (bare != folder) { skipped += path - return lower + return path } - addedBytes += suffix.length - (parts[1].length - segment.length) - val fixed = "/" + parts.mapIndexed { i, part -> if (i == 1) segment + suffix else part } - .joinToString("/") + val fixed = (prefix + bare + suffix + rest).lowercase() + addedBytes += fixed.length - path.length rewritten += fixed return fixed } @@ -336,5 +356,8 @@ private class ScmapRewriter( private companion object { val VERSIONED = Regex("""\.v\d{4}$""") + + /** `/maps//`, with the three parts captured separately. */ + val MAP_PATH = Regex("""^(/?maps/)([^/]+)(/.*)$""", RegexOption.IGNORE_CASE) } } diff --git a/apps/faf-legacy-deployment/scripts/ScmapPathFixerCheck.kt b/apps/faf-legacy-deployment/scripts/ScmapPathFixerCheck.kt index 7c216ee8..a772d2c7 100644 --- a/apps/faf-legacy-deployment/scripts/ScmapPathFixerCheck.kt +++ b/apps/faf-legacy-deployment/scripts/ScmapPathFixerCheck.kt @@ -10,10 +10,12 @@ import kotlin.system.exitProcess * anything is off. Meant for CI, so a change to the path rewriting cannot quietly break a * map file. * - * The important one is the identity check: rewriting with a negative version has to - * reproduce the input byte for byte. It covers every mission, not only the handful that - * need the fix, and therefore also the old file formats. Scoping a section to the wrong - * file version breaks exactly those and nothing else. + * Rewriting with a negative version has to reproduce the input byte for byte. That covers + * every mission, not only the handful that need the fix, but it is a weaker statement than + * it looks: with no suffix the rewriter is a byte copier by construction, so the identity + * check really only says that no read ran off the end of the file. The checks that carry + * weight are the ones [fixScmapPaths] makes on the rewritten output, and the detection + * cross check below. * * Point MAPS_REPO at a checkout of https://github.com/FAForever/faf-coop-maps. */ @@ -28,9 +30,10 @@ fun main() { require(maps.isNotEmpty()) { "no .scmap files found below $repo" } val failures = mutableListOf() + val rewrittenFolders = mutableListOf() + val versions = mutableMapOf() var placeholders = 0 var identical = 0 - var rewrittenMaps = 0 println("%-44s %-8s %-10s %s".format("mission", "format", "size", "result")) @@ -45,6 +48,17 @@ fun main() { } val notes = mutableListOf() + val version = formatVersion(bytes) + versions[version] = (versions[version] ?: 0) + 1 + + // The rewriter only knows the sections of the versions that are actually in this + // repo. Nothing new will appear here - maps are written as v60, occasionally still + // as v56 - so an unexpected version means a section boundary the rewriter has never + // been run against, and it has to be looked at rather than parsed on a guess. + if (version !in KNOWN_VERSIONS) { + failures += "$folder: unexpected .scmap version $version, the rewriter only " + + "covers ${KNOWN_VERSIONS.joinToString()}" + } try { val copy = fixScmapPaths(bytes, folder, -1).bytes @@ -60,11 +74,20 @@ fun main() { notes += "FAILED: ${e.message}" } - if (bytes.referencesOwnMapFolder(folder)) { + // referencesOwnMapFolder is both the gate and the thing under test here: if it ever + // regressed to false every check below would be skipped and CI would still pass. So + // it is compared against a second, independent implementation. + val detected = bytes.referencesOwnMapFolder(folder) + val expected = String(bytes, Charsets.ISO_8859_1).contains("/maps/$folder", ignoreCase = true) + if (detected != expected) { + failures += "$folder: referencesOwnMapFolder says $detected, a plain text search says $expected" + } + + if (detected) { try { - // delta accounting and the round trip are asserted inside fixScmapPaths + // delta accounting, leftover scan and round trip are asserted in fixScmapPaths val fix = fixScmapPaths(bytes, folder, 4) - rewrittenMaps++ + rewrittenFolders += folder notes += "${fix.rewritten.size} path(s) rewritten" fix.rewritten.filterNot { it.contains(".v0004/") } @@ -79,13 +102,21 @@ fun main() { } } - println("%-44s %-8s %-10s %s".format(folder, "v" + formatVersion(bytes), bytes.size, notes.joinToString(", "))) + println("%-44s %-8s %-10s %s".format(folder, "v$version", bytes.size, notes.joinToString(", "))) + } + + // a run in which nothing at all was rewritten proves nothing about the rewriting + if (rewrittenFolders.isEmpty()) { + failures += "no map file references its own folder, so nothing was rewritten - " + + "either the missions changed or the detection is broken" } println() println("$identical of ${maps.size - placeholders} map file(s) reproduced byte for byte") - println("$rewrittenMaps map file(s) reference their own folder and were rewritten") + println("${rewrittenFolders.size} map file(s) reference their own folder and were rewritten:") + rewrittenFolders.forEach { println(" $it") } println("$placeholders placeholder file(s) skipped") + println("format versions: " + versions.toSortedMap().entries.joinToString { "v${it.key} x${it.value}" }) if (failures.isNotEmpty()) { println() @@ -98,11 +129,18 @@ fun main() { private val DOUBLE_VERSION = Regex("""\.v\d{4}\.v\d{4}""") -/** Map file format version, stored behind the preview image. */ +/** The only .scmap format versions the coop missions contain. */ +private val KNOWN_VERSIONS = setOf(53, 56, 60) + +/** + * Map file format version, stored behind the preview image. A valid header is only 16 bytes, + * so a file can pass [isScmap] and still be too short to hold one - that is reported as an + * unknown version rather than taking the whole run down. + */ private fun formatVersion(bytes: ByteArray): Int { fun int(at: Int) = (bytes[at].toInt() and 0xFF) or ((bytes[at + 1].toInt() and 0xFF) shl 8) or ((bytes[at + 2].toInt() and 0xFF) shl 16) or ((bytes[at + 3].toInt() and 0xFF) shl 24) - return int(34 + int(30)) + return runCatching { int(34 + int(30)) }.getOrDefault(-1) } diff --git a/apps/faf-legacy-deployment/scripts/build.gradle.kts b/apps/faf-legacy-deployment/scripts/build.gradle.kts index e7f6ba82..7808b217 100644 --- a/apps/faf-legacy-deployment/scripts/build.gradle.kts +++ b/apps/faf-legacy-deployment/scripts/build.gradle.kts @@ -44,4 +44,4 @@ tasks.register("verifyScmapFixer") { classpath = sourceSets.main.get().runtimeClasspath mainClass.set("com.faforever.ScmapPathFixerCheckKt") -} \ No newline at end of file +} From 36c9f51d5d3596010b0b6c5545b68d727aa4ccbb Mon Sep 17 00:00:00 2001 From: Brutus5000 Date: Wed, 19 Aug 2026 12:25:36 +0200 Subject: [PATCH 05/12] Increase readability of fun getFileContent --- .../scripts/CoopMapDeployer.kt | 49 ++++++++++--------- 1 file changed, 25 insertions(+), 24 deletions(-) diff --git a/apps/faf-legacy-deployment/scripts/CoopMapDeployer.kt b/apps/faf-legacy-deployment/scripts/CoopMapDeployer.kt index 638a2847..777f0b8b 100644 --- a/apps/faf-legacy-deployment/scripts/CoopMapDeployer.kt +++ b/apps/faf-legacy-deployment/scripts/CoopMapDeployer.kt @@ -25,6 +25,7 @@ import kotlin.io.path.readBytes import kotlin.io.path.readText import kotlin.io.path.walk import kotlin.system.exitProcess +import kotlin.toString private val log = LoggerFactory.getLogger("coop-maps-updater") @@ -191,33 +192,33 @@ private fun generateChecksumsForMap( /** * Get file content with path rewriting for text files. */ -private fun getFileContent(file: Path, map: CoopMap, version: Int): ByteArray { - if (file.isScmapFile()) { - val bytes = file.readBytes() - // Only missions whose map references assets in their own folder need the version - // inserted. Everything else - including the placeholder .scmap files of the missions - // that use a base game map - is passed through and never parsed. - return if (bytes.needsPathFix(map.folderName)) { - fixScmapPaths(bytes, map.folderName, version).bytes - } else { - bytes +private fun getFileContent(file: Path, map: CoopMap, version: Int): ByteArray = + when { + file.isScmapFile() -> { + val bytes = file.readBytes() + // Only missions whose map references assets in their own folder need the version + // inserted. Everything else - including the placeholder .scmap files of the missions + // that use a base game map - is passed through and never parsed. + if (bytes.needsPathFix(map.folderName)) { + fixScmapPaths(bytes, map.folderName, version).bytes + } else { + bytes + } } - } - return if (file.isTextFile()) { - var text = file.readText() - .replace( - "/maps/${map.folderName}/", - "/maps/${map.folderName(version)}/", - ignoreCase = true, - ) - if (file.toString().endsWith("_scenario.lua")) { - text = text.replace(Regex("""(map_version\s*=\s*)\d+"""), "$1$version") + file.isTextFile() -> { + var text = file.readText() + .replace( + "/maps/${map.folderName}/", + "/maps/${map.folderName(version)}/", + ignoreCase = true, + ) + if (file.toString().endsWith("_scenario.lua")) { + text = text.replace(Regex("""(map_version\s*=\s*)\d+"""), "$1$version") + } + text.toByteArray() } - text.toByteArray() - } else { - file.readBytes() + else -> file.readBytes() } -} /** * Checks that every path the release points at actually exists in the release. From 3fe4359fd66cd391c5395acdabba5a49ca3047dc Mon Sep 17 00:00:00 2001 From: Brutus5000 Date: Wed, 19 Aug 2026 12:33:27 +0200 Subject: [PATCH 06/12] Check file endings case insensitive --- apps/faf-legacy-deployment/scripts/CoopMapDeployer.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/faf-legacy-deployment/scripts/CoopMapDeployer.kt b/apps/faf-legacy-deployment/scripts/CoopMapDeployer.kt index 777f0b8b..4693460e 100644 --- a/apps/faf-legacy-deployment/scripts/CoopMapDeployer.kt +++ b/apps/faf-legacy-deployment/scripts/CoopMapDeployer.kt @@ -315,9 +315,9 @@ private fun createZip( } } -private fun Path.isTextFile() = listOf(".md", ".lua", ".json", ".txt").any { toString().endsWith(it) } +private fun Path.isTextFile() = listOf(".md", ".lua", ".json", ".txt").any { toString().endsWith(it, ignoreCase = true) } -private fun Path.isScmapFile() = toString().lowercase().endsWith(".scmap") +private fun Path.isScmapFile() = toString().endsWith(".scmap", ignoreCase = true) fun main(args: Array) { Log.init() From 0f7b645146ea6c49fada0618b052722e202e9aa0 Mon Sep 17 00:00:00 2001 From: Brutus5000 Date: Wed, 19 Aug 2026 13:04:19 +0200 Subject: [PATCH 07/12] Minor renaming --- .../scripts/ScmapPathFixer.kt | 29 ++++++++++--------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/apps/faf-legacy-deployment/scripts/ScmapPathFixer.kt b/apps/faf-legacy-deployment/scripts/ScmapPathFixer.kt index 3df32a23..e1cf9021 100644 --- a/apps/faf-legacy-deployment/scripts/ScmapPathFixer.kt +++ b/apps/faf-legacy-deployment/scripts/ScmapPathFixer.kt @@ -50,7 +50,7 @@ fun ByteArray.needsPathFix(folderName: String) = isScmap() && referencesOwnMapFo * @property rewritten every path that received the version, as written into [bytes] * @property skipped paths inside /maps that lead to another folder and were left alone */ -class ScmapPathFix( +data class ScmapPathFixResult( val bytes: ByteArray, val rewritten: List, val skipped: List, @@ -64,7 +64,7 @@ class ScmapPathFix( * -> /maps/faf_coop_operation_blockade.v0004/env/layers/sand.dds * * Only paths that point at [folderName] itself are touched. Paths pointing somewhere else - * are left alone and reported in [ScmapPathFix.skipped] - several missions deliberately + * are left alone and reported in [ScmapPathFixResult.skipped] - several missions deliberately * reference a base game map * (`/maps/X1CA_001/X1CA_001.scmap`) or another map's texture, and versioning those would * break them. @@ -83,44 +83,45 @@ class ScmapPathFix( * @throws IllegalArgumentException if the file is not a .scmap * @throws IllegalStateException if the result fails verification */ -fun fixScmapPaths(bytes: ByteArray, folderName: String, version: Int): ScmapPathFix { + +fun fixScmapPaths(bytes: ByteArray, folderName: String, version: Int): ScmapPathFixResult { require(bytes.isScmap()) { "$folderName: not a .scmap file, header mismatch" } val suffix = if (version >= 0) ".v%04d".format(version) else "" val rewriter = ScmapRewriter(bytes, folderName, suffix) - val result = rewriter.rewrite() - val fix = ScmapPathFix(result, rewriter.rewritten.toList(), rewriter.skipped.distinct()) + val rewrittenBytes = rewriter.rewrite() + val result = ScmapPathFixResult(rewrittenBytes, rewriter.rewritten.toList(), rewriter.skipped.distinct()) - if (suffix.isEmpty()) return fix + if (suffix.isEmpty()) return result - fix.skipped.forEach { + result.skipped.forEach { log.warn("$folderName: path leads outside the mission folder, left unchanged: $it") } log.info( "$folderName: rewrote {} path(s) in the map file, {} byte(s) added", - fix.rewritten.size, result.size - bytes.size + result.rewritten.size, rewrittenBytes.size - bytes.size ) // the file may only have grown by exactly what went into the paths - anything else means // the structural pass lost or invented bytes somewhere between them - check(result.size - bytes.size == rewriter.addedBytes) { - "$folderName: byte delta ${result.size - bytes.size} does not match the " + + check(rewrittenBytes.size - bytes.size == rewriter.addedBytes) { + "$folderName: byte delta ${rewrittenBytes.size - bytes.size} does not match the " + "${rewriter.addedBytes} byte(s) added to paths" } // no unversioned reference to the mission folder may survive. This is the one check that // catches a path the parser never recognised as a path in the first place - the delta // accounting and the round trip below are both blind to a rewrite that simply did not // happen, which is exactly the bug this whole class exists to fix. - check(!result.containsAscii("/maps/$folderName/")) { + check(!rewrittenBytes.containsAscii("/maps/$folderName/")) { "$folderName: the rewritten .scmap still contains unversioned /maps/$folderName/ " + "path(s), refusing to ship it" } // the rewritten file has to parse again and come out byte identical - val verified = ScmapRewriter(result, folderName, "").rewrite() - check(verified.contentEquals(result)) { + val verified = ScmapRewriter(rewrittenBytes, folderName, "").rewrite() + check(verified.contentEquals(rewrittenBytes)) { "$folderName: rewritten .scmap does not round trip, refusing to ship it" } - return fix + return result } private class ScmapRewriter( From ea7aa84e9985837f743d9e237a5978ce33beb881 Mon Sep 17 00:00:00 2001 From: SeraphimNoob01 Date: Thu, 20 Aug 2026 04:38:36 +0100 Subject: [PATCH 08/12] Lower case only the map folder, not the whole path fix_paths lower cases the entire path, and this port inherited that. Measured against the v9.0.2 corpus, that is wrong for four of the rewritten paths: Golden_Crystals and Overlord_Surth_Velsok reference their decals in exactly the casing the files carry on disk, and lower casing the tail introduces a mismatch that is not in the source data. Only the /maps/.vNNNN head is lower cased now. Exact matches against the files in the release go from 6 to 10; the remaining 28 paths already point at lower case names in the map file itself and are untouched, as before. verifyRelease could not have caught this - it compares lower cased on both sides, which it has to, because those 28 pre-existing mismatches are live and working. --- .../faf-legacy-deployment/scripts/ScmapPathFixer.kt | 13 ++++++++++--- .../scripts/ScmapPathFixerCheck.kt | 7 +++++-- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/apps/faf-legacy-deployment/scripts/ScmapPathFixer.kt b/apps/faf-legacy-deployment/scripts/ScmapPathFixer.kt index e1cf9021..c24b3703 100644 --- a/apps/faf-legacy-deployment/scripts/ScmapPathFixer.kt +++ b/apps/faf-legacy-deployment/scripts/ScmapPathFixer.kt @@ -279,8 +279,15 @@ private class ScmapRewriter( /** * If [path] points into this mission's own folder inside /maps, the version is added to - * the folder name and the path is lower cased - that is what `fix_paths` does and what - * the maps deployed so far look like. + * the folder name. Only the `/maps/.vNNNN` head is lower cased; everything + * after it keeps the casing it had. + * + * `fix_paths` lower cases the whole path, and this deliberately does not. Two of the + * four affected missions reference their decals in exactly the casing the files carry + * on disk - `env/decals/FAF_Coop_Operation_Golden_Crystals_nm.dds` and the three like + * it - and lower casing those introduces a mismatch that is not in the source data. + * The other 28 paths already point at lower case names while the files are mixed case, + * so a lower cased tail buys nothing there either. * * Everything else is returned byte for byte as it came in, including paths under /maps * that lead to another folder. Those are shipped unparsed today and work, so there is @@ -300,7 +307,7 @@ private class ScmapRewriter( return path } - val fixed = (prefix + bare + suffix + rest).lowercase() + val fixed = (prefix + bare + suffix).lowercase() + rest addedBytes += fixed.length - path.length rewritten += fixed return fixed diff --git a/apps/faf-legacy-deployment/scripts/ScmapPathFixerCheck.kt b/apps/faf-legacy-deployment/scripts/ScmapPathFixerCheck.kt index a772d2c7..67f97dc7 100644 --- a/apps/faf-legacy-deployment/scripts/ScmapPathFixerCheck.kt +++ b/apps/faf-legacy-deployment/scripts/ScmapPathFixerCheck.kt @@ -92,8 +92,11 @@ fun main() { fix.rewritten.filterNot { it.contains(".v0004/") } .forEach { failures += "$folder: not versioned: $it" } - fix.rewritten.filter { it != it.lowercase() } - .forEach { failures += "$folder: not lower cased: $it" } + // only the /maps/.vNNNN head is lower cased, the rest of the path + // keeps the casing the map file had - see addVersion + fix.rewritten.map { it.substringBefore(".v0004/") } + .filter { it != it.lowercase() } + .forEach { failures += "$folder: map folder not lower cased: $it" } fix.rewritten.filter { DOUBLE_VERSION.containsMatchIn(it) } .forEach { failures += "$folder: version added twice: $it" } } catch (e: Exception) { From 13ddc10a61c10d62cf8cf47360251674a614aa92 Mon Sep 17 00:00:00 2001 From: SeraphimNoob01 Date: Thu, 20 Aug 2026 04:43:49 +0100 Subject: [PATCH 09/12] Say why verifyRelease compares case insensitively It looks like a place where a lax comparison hides a bug, and the reason it is not has to be written down: 28 of the paths embedded in the four rewritten map files point at lower case names while the files on disk are mixed case, in the missions as committed. Comparing case sensitively would refuse releases that load today. --- apps/faf-legacy-deployment/scripts/CoopMapDeployer.kt | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/apps/faf-legacy-deployment/scripts/CoopMapDeployer.kt b/apps/faf-legacy-deployment/scripts/CoopMapDeployer.kt index 4693460e..f50f1f8a 100644 --- a/apps/faf-legacy-deployment/scripts/CoopMapDeployer.kt +++ b/apps/faf-legacy-deployment/scripts/CoopMapDeployer.kt @@ -234,6 +234,11 @@ private fun getFileContent(file: Path, map: CoopMap, version: Int): ByteArray = * @throws IllegalStateException if a rewritten map path does not resolve */ private fun verifyRelease(map: CoopMap, version: Int, files: List, base: Path) { + // Everything below is compared case insensitively, and it has to be: 28 of the paths + // embedded in the map files point at lower case names while the files themselves are + // mixed case. That is in the missions as committed, it predates this deployment, and + // those maps load - a case sensitive comparison would refuse releases that demonstrably + // work. What the rewriting must not do is add mismatches on top of that; see addVersion. val prefix = "/maps/${map.folderName(version)}/".lowercase() val shipped = files .map { prefix + base.relativize(it).toString().replace("\\", "/").lowercase() } From d3c0e4d346a52680c56cb66c5719510dec6b645c Mon Sep 17 00:00:00 2001 From: SeraphimNoob01 Date: Thu, 20 Aug 2026 04:44:46 +0100 Subject: [PATCH 10/12] Read and write text files as ISO-8859-1 verifyRelease already decoded them as Latin-1 to scan for references, while getFileContent read and wrote them as UTF-8. The two have to agree, and Latin-1 is the side that is right: it maps every byte to one char and back, and the replacements only touch ASCII, so the file comes out byte identical whatever it is really encoded in. UTF-8 turns anything it cannot decode into U+FFFD and writes that into the release. No file in the missions is affected today - all 479 text files are valid UTF-8, and both paths produce byte identical output for every one of them, so no checksum and no version changes. This only keeps the first file with a Latin-1 accent from being corrupted without anyone noticing. --- apps/faf-legacy-deployment/scripts/CoopMapDeployer.kt | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/apps/faf-legacy-deployment/scripts/CoopMapDeployer.kt b/apps/faf-legacy-deployment/scripts/CoopMapDeployer.kt index f50f1f8a..c73ad7ac 100644 --- a/apps/faf-legacy-deployment/scripts/CoopMapDeployer.kt +++ b/apps/faf-legacy-deployment/scripts/CoopMapDeployer.kt @@ -191,6 +191,13 @@ private fun generateChecksumsForMap( /** * Get file content with path rewriting for text files. + * + * Text files are read and written as ISO-8859-1. Latin-1 maps every byte to exactly one + * char and back, and the replacements below only ever touch ASCII, so a file comes out + * byte identical no matter what it is really encoded in. Reading as UTF-8 would turn + * anything that is not valid UTF-8 into U+FFFD and write that into the release instead. + * Every text file in the missions is valid UTF-8 today, so this changes no checksum - it + * only keeps the first file with a Latin-1 accent in it from being corrupted silently. */ private fun getFileContent(file: Path, map: CoopMap, version: Int): ByteArray = when { @@ -206,7 +213,7 @@ private fun getFileContent(file: Path, map: CoopMap, version: Int): ByteArray = } } file.isTextFile() -> { - var text = file.readText() + var text = file.readText(Charsets.ISO_8859_1) .replace( "/maps/${map.folderName}/", "/maps/${map.folderName(version)}/", @@ -215,7 +222,7 @@ private fun getFileContent(file: Path, map: CoopMap, version: Int): ByteArray = if (file.toString().endsWith("_scenario.lua")) { text = text.replace(Regex("""(map_version\s*=\s*)\d+"""), "$1$version") } - text.toByteArray() + text.toByteArray(Charsets.ISO_8859_1) } else -> file.readBytes() } From 6f925e0f36f7931efac1a0cb444f36374513b16c Mon Sep 17 00:00:00 2001 From: SeraphimNoob01 Date: Thu, 20 Aug 2026 04:48:16 +0100 Subject: [PATCH 11/12] Require the leading slash on a map path MAP_PATH accepted maps/x/y as well as /maps/x/y, while referencesOwnMapFolder - the gate that decides whether a file is parsed at all - only ever searched for the form with the slash. A path in the slashless form would therefore have been rewritten by the rewriter but never detected by the gate, so the file would have been passed through unchanged. The slashless form is not in the data: all 40 paths embedded in the missions carry the leading slash. It was only ever accepted because fix_paths splits on "/" discarding empty segments, which makes the two indistinguishable to it. Requiring the slash makes gate and rewriter mean the same thing. The trailing slash stays asymmetric on purpose and now says so in the code: the gate must match an already versioned self reference so the old suffix gets stripped, the leftover scan must not match it because it asserts that nothing unversioned survived. No change in behaviour over the missions - same paths, same byte deltas. --- .../scripts/ScmapPathFixer.kt | 20 +++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/apps/faf-legacy-deployment/scripts/ScmapPathFixer.kt b/apps/faf-legacy-deployment/scripts/ScmapPathFixer.kt index c24b3703..e31f0d4e 100644 --- a/apps/faf-legacy-deployment/scripts/ScmapPathFixer.kt +++ b/apps/faf-legacy-deployment/scripts/ScmapPathFixer.kt @@ -37,6 +37,13 @@ fun ByteArray.containsAscii(needle: String): Boolean { * Whether the map references its own folder, so we only parse map files that actually need * the fix. Missions whose .scmap contains no such path (and the placeholder files that are * not maps at all) are passed through untouched. + * + * Deliberately without a trailing slash, unlike the leftover scan in [fixScmapPaths]. This + * one has to match `/maps/.v0003/...` as well - a self reference that already + * carries a version - so that such a file reaches the parser, where the old suffix is + * stripped instead of a second one being stacked on top. The leftover scan wants the exact + * opposite: it asserts that no *unversioned* reference survived, so it needs the trailing + * slash to not match the versioned form it just wrote. */ fun ByteArray.referencesOwnMapFolder(folderName: String) = containsAscii("/maps/$folderName") @@ -365,7 +372,16 @@ private class ScmapRewriter( private companion object { val VERSIONED = Regex("""\.v\d{4}$""") - /** `/maps//`, with the three parts captured separately. */ - val MAP_PATH = Regex("""^(/?maps/)([^/]+)(/.*)$""", RegexOption.IGNORE_CASE) + /** + * `/maps//`, with the three parts captured separately. + * + * The leading slash is required, so this matches exactly what + * [referencesOwnMapFolder] searches for. All 40 paths embedded in the missions carry + * it; the slashless form was only ever accepted because `fix_paths` splits on `/` + * discarding empty segments, which makes `maps/x/y` and `/maps/x/y` indistinguishable + * to it. Accepting a form the gate does not detect means the gate can wave a file + * through that the rewriter would have changed. + */ + val MAP_PATH = Regex("""^(/maps/)([^/]+)(/.*)$""", RegexOption.IGNORE_CASE) } } From aa36382cc48508ed1a8c82c5d75f4f6c723a8977 Mon Sep 17 00:00:00 2001 From: SeraphimNoob01 Date: Thu, 20 Aug 2026 04:57:20 +0100 Subject: [PATCH 12/12] Check the _scenario.lua suffix case insensitively too The two helpers right below it were made case insensitive in "Check file endings case insensitive"; this one was left behind, so a mission with a differently cased scenario file would have shipped without its map_version rewritten while its paths were rewritten. All 44 scenario files are lower case today, so nothing changes for the missions as they are. --- apps/faf-legacy-deployment/scripts/CoopMapDeployer.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/faf-legacy-deployment/scripts/CoopMapDeployer.kt b/apps/faf-legacy-deployment/scripts/CoopMapDeployer.kt index c73ad7ac..5e9d1808 100644 --- a/apps/faf-legacy-deployment/scripts/CoopMapDeployer.kt +++ b/apps/faf-legacy-deployment/scripts/CoopMapDeployer.kt @@ -219,7 +219,7 @@ private fun getFileContent(file: Path, map: CoopMap, version: Int): ByteArray = "/maps/${map.folderName(version)}/", ignoreCase = true, ) - if (file.toString().endsWith("_scenario.lua")) { + if (file.toString().endsWith("_scenario.lua", ignoreCase = true)) { text = text.replace(Regex("""(map_version\s*=\s*)\d+"""), "$1$version") } text.toByteArray(Charsets.ISO_8859_1)