PropertiesFileTransformer - make merged properties reproducible

`PropertiesFileTransformer` leverages `java.util.Properties`, which relies on `java.util.Hashtable`. The serialized properties are not guaranteed to be reproducible.

This change changes the transformer to use a sorted map to generate reproducible output. The existing class `CleanProperties` is replaced with `ReproducibleProperties`.
Functionality around charset handling is retained, and extended with a functionality to generate unicode escapes (ASCII output).

Author: snazy

api/shadow.api

@@ -419,6 +419,7 @@ public class com/github/jengelman/gradle/plugins/shadow/transformers/PropertiesF
 	public fun <init> (Lorg/gradle/api/model/ObjectFactory;)V
 	public fun canTransformResource (Lorg/gradle/api/file/FileTreeElement;)Z
 	public fun getCharsetName ()Lorg/gradle/api/provider/Property;
+	public fun getEscapeUnicode ()Lorg/gradle/api/provider/Property;
 	public fun getKeyTransformer ()Lkotlin/jvm/functions/Function1;
 	public fun getMappings ()Lorg/gradle/api/provider/MapProperty;
 	public fun getMergeSeparator ()Lorg/gradle/api/provider/Property;

docs/changes/README.md

@@ -19,6 +19,7 @@
 - Update ASM and jdependency to support Java 26. ([#1799](https://github.com/GradleUp/shadow/pull/1799))
 - Bump min Gradle requirement to 9.0.0. ([#1801](https://github.com/GradleUp/shadow/pull/1801))
 - Deprecate `PreserveFirstFoundResourceTransformer.resources`. ([#1855](https://github.com/GradleUp/shadow/pull/1855))
+- Make the output of `PropertiesFileTransformer` reproducible. ([#1861](https://github.com/GradleUp/shadow/pull/1861))
 
 ### Fixed
 

src/functionalTest/kotlin/com/github/jengelman/gradle/plugins/shadow/transformers/PropertiesFileTransformerTest.kt

@@ -167,8 +167,6 @@ class PropertiesFileTransformerTest : BaseTransformerTest() {
     val content = outputShadowedJar.use { it.getContent("META-INF/test.properties") }
     assertThat(content.trimIndent()).isEqualTo(
       """
-        #
-
         foo=one,two
       """.trimIndent(),
     )

src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/internal/CleanProperties.kt

@@ -0,0 +1,60 @@
+package com.github.jengelman.gradle.plugins.shadow.internal
+
+import java.io.OutputStream
+import java.nio.charset.Charset
+import java.util.HexFormat
+import java.util.SortedMap
+
+/**
+ * Maintains a map of properties sorted by key, which can be written out to a text file with a consistent
+ * ordering to satisfy the requirements of reproducible builds.
+ */
+internal class ReproducibleProperties {
+  internal val props: SortedMap<String, String> = sortedMapOf()
+
+  fun writeProperties(charset: Charset, os: OutputStream, escape: Boolean) {
+    val zipWriter = os.writer(charset)
+    props.forEach { (key, value) ->
+      zipWriter.write(convertString(key, isKey = true, escape))
+      zipWriter.write("=")
+      zipWriter.write(convertString(value, isKey = false, escape))
+      zipWriter.write("\n")
+    }
+    zipWriter.flush()
+  }
+
+  private fun convertString(
+    str: String,
+    isKey: Boolean,
+    escape: Boolean,
+  ): String {
+    val len = str.length
+    val out = StringBuilder()
+    val hex = HexFormat.of().withUpperCase()
+    for (x in 0..<len) {
+      val aChar = str[x]
+      // Handle the common case first, avoid more expensive special cases
+      if ((aChar.code > 61) && (aChar.code < 127)) {
+        out.append(if (aChar == '\\') "\\\\" else aChar)
+        continue
+      }
+      when (aChar) {
+        ' ' -> {
+          if (x == 0 || isKey) out.append('\\')
+          out.append(' ')
+        }
+        '\t' -> out.append("\\t")
+        '\n' -> out.append("\\n")
+        '\r' -> out.append("\\r")
+        '\u000c' -> out.append("\\f")
+        '=', ':', '#', '!' -> out.append('\\').append(aChar)
+        else -> if (escape && ((aChar.code < 0x0020) || (aChar.code > 0x007e))) {
+          out.append("\\u").append(hex.toHexDigits(aChar))
+        } else {
+          out.append(aChar)
+        }
+      }
+    }
+    return out.toString()
+  }
+}

src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/internal/ReproducibleProperties.kt

@@ -1,7 +1,6 @@
 package com.github.jengelman.gradle.plugins.shadow.transformers
 
-import com.github.jengelman.gradle.plugins.shadow.internal.CleanProperties
-import com.github.jengelman.gradle.plugins.shadow.internal.inputStream
+import com.github.jengelman.gradle.plugins.shadow.internal.ReproducibleProperties
 import com.github.jengelman.gradle.plugins.shadow.internal.mapProperty
 import com.github.jengelman.gradle.plugins.shadow.internal.property
 import com.github.jengelman.gradle.plugins.shadow.internal.setProperty
@@ -109,7 +108,7 @@ public open class PropertiesFileTransformer @Inject constructor(
   internal val conflicts: MutableMap<String, MutableMap<String, Int>> = mutableMapOf()
 
   @get:Internal
-  internal val propertiesEntries = mutableMapOf<String, CleanProperties>()
+  internal val propertiesEntries = mutableMapOf<String, ReproducibleProperties>()
 
   @get:Input
   public open val paths: SetProperty<String> = objectFactory.setProperty()
@@ -123,6 +122,22 @@ public open class PropertiesFileTransformer @Inject constructor(
   @get:Input
   public open val mergeSeparator: Property<String> = objectFactory.property(",")
 
+  /**
+   * Properties files are written without escaping Unicode characters using the character set
+   * configured by [charsetName].
+   *
+   * Set this property to `true` to escape all Unicode characters in the properties file, producing
+   * ASCII compatible files.
+   */
+  @get:Input
+  public open val escapeUnicode: Property<Boolean> = objectFactory.property(false)
+
+  /**
+   * The character set to use when reading and writing property files.
+   * Defaults to `ISO-8859-1`.
+   *
+   * See also [escapeUnicode].
+   */
   @get:Input
   public open val charsetName: Property<String> = objectFactory.property(Charsets.ISO_8859_1.name())
 
@@ -146,49 +161,35 @@ public open class PropertiesFileTransformer @Inject constructor(
   }
 
   override fun transform(context: TransformerContext) {
-    val props = propertiesEntries[context.path]
-    val incoming = loadAndTransformKeys(context.inputStream)
-    if (props == null) {
-      propertiesEntries[context.path] = incoming
-    } else {
-      for ((key, value) in incoming) {
-        if (props.containsKey(key)) {
-          when (MergeStrategy.from(mergeStrategyFor(context.path))) {
-            MergeStrategy.Latest -> {
-              props[key] = value
-            }
-            MergeStrategy.Append -> {
-              props[key] = props.getProperty(key as String) + mergeSeparatorFor(context.path) + value
-            }
-            MergeStrategy.First -> Unit
-            MergeStrategy.Fail -> {
-              val conflictsForPath = conflicts.computeIfAbsent(context.path) { mutableMapOf() }
-              conflictsForPath.compute(key as String) { _, count -> (count ?: 1) + 1 }
-            }
+    val props = propertiesEntries.computeIfAbsent(context.path) { ReproducibleProperties() }.props
+    val mergeStrategy = MergeStrategy.from(mergeStrategyFor(context.path))
+    val mergeSeparator = if (mergeStrategy == MergeStrategy.Append) mergeSeparatorFor(context.path) else ""
+    loadAndTransformKeys(context.inputStream) { key, value ->
+      if (props.containsKey(key)) {
+        when (mergeStrategy) {
+          MergeStrategy.Latest -> {
+            props[key] = value
+          }
+          MergeStrategy.Append -> {
+            props[key] = props[key] + mergeSeparator + value
+          }
+          MergeStrategy.First -> Unit
+          MergeStrategy.Fail -> {
+            val conflictsForPath = conflicts.computeIfAbsent(context.path) { mutableMapOf() }
+            conflictsForPath.compute(key) { _, count -> (count ?: 1) + 1 }
           }
-        } else {
-          props[key] = value
         }
+      } else {
+        props[key] = value
       }
     }
   }
 
-  private fun loadAndTransformKeys(inputStream: InputStream): CleanProperties {
-    val props = CleanProperties()
+  private fun loadAndTransformKeys(inputStream: InputStream, keyValue: (key: String, value: String) -> Unit) {
+    val props = Properties()
     // InputStream closed by caller, so we don't do it here.
     props.load(inputStream.bufferedReader(charset))
-    return transformKeys(props)
-  }
-
-  private fun transformKeys(properties: Properties): CleanProperties {
-    if (keyTransformer == IDENTITY) {
-      return properties as CleanProperties
-    }
-    val result = CleanProperties()
-    properties.forEach { (key, value) ->
-      result[keyTransformer(key as String)] = value
-    }
-    return result
+    props.forEach { keyValue(keyTransformer(it.key as String), it.value as String) }
   }
 
   private fun mergeStrategyFor(path: String): String {
@@ -234,13 +235,9 @@ public open class PropertiesFileTransformer @Inject constructor(
     }
 
     // Cannot close the writer as the OutputStream needs to remain open.
-    val zipWriter = os.writer(charset)
     propertiesEntries.forEach { (path, props) ->
       os.putNextEntry(zipEntry(path, preserveFileTimestamps))
-      props.inputStream(charset).bufferedReader(charset).use {
-        it.copyTo(zipWriter)
-      }
-      zipWriter.flush()
+      props.writeProperties(charset, os, escapeUnicode.get())
       os.closeEntry()
     }
   }

src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/transformers/PropertiesFileTransformer.kt

@@ -74,7 +74,7 @@ class PropertiesFileTransformerTest : BaseTransformerTest<PropertiesFileTransfor
       transformer.transform(context(path, input2))
     }
 
-    assertThat(transformer.propertiesEntries[path].orEmpty()).isEqualTo(expectedOutput)
+    assertThat(transformer.propertiesEntries[path]?.props.orEmpty()).isEqualTo(expectedOutput)
     assertThat(transformer.conflicts).isEqualTo(expectedConflicts)
   }
 
@@ -95,7 +95,7 @@ class PropertiesFileTransformerTest : BaseTransformerTest<PropertiesFileTransfor
       transformer.transform(context(path, input2))
     }
 
-    assertThat(transformer.propertiesEntries[path].orEmpty()).isEqualTo(expectedOutput)
+    assertThat(transformer.propertiesEntries[path]?.props.orEmpty()).isEqualTo(expectedOutput)
   }
 
   @ParameterizedTest
@@ -115,7 +115,7 @@ class PropertiesFileTransformerTest : BaseTransformerTest<PropertiesFileTransfor
       transformer.transform(context(path, input2))
     }
 
-    assertThat(transformer.propertiesEntries[path].orEmpty()).isEqualTo(expectedOutput)
+    assertThat(transformer.propertiesEntries[path]?.props.orEmpty()).isEqualTo(expectedOutput)
   }
 
   @ParameterizedTest
@@ -135,7 +135,7 @@ class PropertiesFileTransformerTest : BaseTransformerTest<PropertiesFileTransfor
       transformer.transform(context(path, input2))
     }
 
-    assertThat(transformer.propertiesEntries[path].orEmpty()).isEqualTo(expectedOutput)
+    assertThat(transformer.propertiesEntries[path]?.props.orEmpty()).isEqualTo(expectedOutput)
   }
 
   @ParameterizedTest
@@ -152,7 +152,7 @@ class PropertiesFileTransformerTest : BaseTransformerTest<PropertiesFileTransfor
       transformer.transform(context(path, input, Charset.forName(charset)))
     }
 
-    assertThat(transformer.propertiesEntries[path].orEmpty()).isEqualTo(expectedOutput)
+    assertThat(transformer.propertiesEntries[path]?.props.orEmpty()).isEqualTo(expectedOutput)
   }
 
   private companion object {