[ 
https://issues.apache.org/jira/browse/MSHARED-1453?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17901319#comment-17901319
 ] 

ASF GitHub Bot commented on MSHARED-1453:
-----------------------------------------

gnodet commented on code in PR #77:
URL: https://github.com/apache/maven-archiver/pull/77#discussion_r1859346432


##########
src/main/java/org/apache/maven/shared/archiver/PomPropertiesUtil.java:
##########
@@ -61,34 +59,29 @@ private boolean sameContents(Properties props, Path file) 
throws IOException {
         return fileProps.equals(props);
     }
 
-    private void createPropertiesFile(Properties properties, Path outputFile, 
boolean forceCreation)
+    private void createPropertiesFile(Properties unsortedProperties, Path 
outputFile, boolean forceCreation)
             throws IOException {
         Path outputDir = outputFile.getParent();
         if (outputDir != null && !Files.isDirectory(outputDir)) {
             Files.createDirectories(outputDir);
         }
-        if (!forceCreation && sameContents(properties, outputFile)) {
+        if (!forceCreation && sameContents(unsortedProperties, outputFile)) {
             return;
         }
 
-        try (PrintWriter pw = new PrintWriter(outputFile.toFile(), 
StandardCharsets.ISO_8859_1.name());
-                StringWriter sw = new StringWriter()) {
-
-            properties.store(sw, null);
-
-            List<String> lines = new ArrayList<>();
-            try (BufferedReader r = new BufferedReader(new 
StringReader(sw.toString()))) {
-                String line;
-                while ((line = r.readLine()) != null) {
-                    if (!line.startsWith("#")) {
-                        lines.add(line);
-                    }
-                }
-            }
-
-            Collections.sort(lines);
-            for (String l : lines) {
-                pw.println(l);
+        // For reproducible builds, sort the properties and drop comments.
+        // The java.util.Properties class doesn't guarantee order so we have
+        // to write the file using a Writer.
+        Set<String> propertyNames = unsortedProperties.stringPropertyNames();
+        List<String> sortedPropertyNames = new ArrayList<>(propertyNames);
+        Collections.sort(sortedPropertyNames);
+
+        try (Writer out = Files.newBufferedWriter(outputFile, 
StandardCharsets.ISO_8859_1)) {
+            for (String key : sortedPropertyNames) {
+                out.write(key);
+                out.write(": ");
+                out.write(unsortedProperties.getProperty(key));

Review Comment:
   I don't think it's risky.  That part is very clearly specified. Changing the 
way properties file are stored would be a big breakage.
   See 
https://docs.oracle.com/javase/8/docs/api/java/util/Properties.html#store-java.io.Writer-java.lang.String-
 for an exact explanation of what is written.
   The way key / separator / value are written is clearly specified and has not 
changed from JDK 8 to JDK 24.
   
   > Then every entry in this Properties table is written out, one per line. 
For each entry the key string is written, then an ASCII =, then the associated 
element string. For the key, all space characters are written with a preceding 
\ character. For the element, leading space characters, but not embedded or 
trailing space characters, are written with a preceding \ character. The key 
and element characters #, !, =, and : are written with a preceding backslash to 
ensure that they are properly loaded.
   
   Feel free to re-implement this mechanism, but I'm not really sure it's worth 
it.





> Canonicalize properties files for reproducible builds
> -----------------------------------------------------
>
>                 Key: MSHARED-1453
>                 URL: https://issues.apache.org/jira/browse/MSHARED-1453
>             Project: Maven Shared Components
>          Issue Type: Bug
>          Components: maven-archiver
>            Reporter: Elliotte Rusty Harold
>            Assignee: Elliotte Rusty Harold
>            Priority: Minor
>
> See discussion on https://github.com/apache/maven-archiver/pull/77/files
> Briefly, properties files have non-unique representations and there's no 
> guarantee two JDKs from different companies and Java versions produce the 
> same byte-per-byte serialization. Our current code accounts for property 
> order and comments, but not variations in escaping (hex vs. UTF-8), separator 
> characters, and insignificant whitespace. 



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to