[ 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)