refs #51 - percent encode files that contain new lines or carriage returns

Author: johnscancella

src/main/java/gov/loc/repository/bagit/domain/Bag.java

@@ -1,5 +1,6 @@
 package gov.loc.repository.bagit.domain;
 
+import java.nio.charset.Charset;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Path;
 import java.util.ArrayList;
@@ -18,7 +19,7 @@ public class Bag {
   private Version version = new Version(-1, -1);
 
   //from the bagit.txt or UTF-8 for new bags
-  private String fileEncoding = StandardCharsets.UTF_8.name();
+  private Charset fileEncoding = StandardCharsets.UTF_8;
 
   //equivalent to the manifest-<ALG>.txt files
   private Set<Manifest> payLoadManifests = new HashSet<>();
@@ -88,11 +89,11 @@ public void setMetadata(List<Pair<String, String>> metadata) {
     this.metadata = metadata;
   }
 
-  public String getFileEncoding() {
+  public Charset getFileEncoding() {
     return fileEncoding;
   }
 
-  public void setFileEncoding(String fileEncoding) {
+  public void setFileEncoding(Charset fileEncoding) {
     this.fileEncoding = fileEncoding;
   }
 

src/main/java/gov/loc/repository/bagit/reader/BagReader.java

@@ -3,6 +3,8 @@
 import java.io.BufferedReader;
 import java.io.IOException;
 import java.net.URL;
+import java.nio.charset.Charset;
+import java.nio.charset.StandardCharsets;
 import java.nio.file.DirectoryStream;
 import java.nio.file.Files;
 import java.nio.file.Path;
@@ -94,14 +96,14 @@ public Bag readBagitTextFile(Path bagitFile, Bag bag) throws IOException, Unpars
     List<Pair<String, String>> pairs = readKeyValuesFromFile(bagitFile, ":");
 
     String version = "";
-    String encoding = "";
+    Charset encoding = StandardCharsets.UTF_8;
     for(Pair<String, String> pair : pairs){
       if("BagIt-Version".equals(pair.getKey())){
         version = pair.getValue();
         logger.debug("BagIt-Version is [{}]", version);
       }
       if("Tag-File-Character-Encoding".equals(pair.getKey())){
-        encoding = pair.getValue();
+        encoding = Charset.forName(pair.getValue());
         logger.debug("Tag-File-Character-Encoding is [{}]", encoding);
       }
     }
@@ -199,7 +201,7 @@ protected HashMap<Path, String> readChecksumFileMap(Path manifestFile, Path bagR
     String line = br.readLine();
     while(line != null){
       String[] parts = line.split("\\s+", 2);
-      Path file = bagRootDir.resolve(parts[1]);
+      Path file = bagRootDir.resolve(PathUtils.decodeFilname(parts[1]));
       if(!file.normalize().startsWith(bagRootDir)){
         throw new MaliciousManifestException("Path " + file + " is outside the bag root directory of " + bagRootDir + 
             "! This is not allowed according to the bagit specification!");

src/main/java/gov/loc/repository/bagit/util/PathUtils.java

@@ -21,4 +21,22 @@ public static String getFilename(Path path){
 
     return filename;
   }
+  
+  /**
+   * as per https://github.com/jkunze/bagitspec/commit/152d42f6298b31a4916ea3f8f644ca4490494070 decode percent encoded filenames
+   * @param encoded the encoded filename
+   * @return the decoded filename 
+   */
+  public static String decodeFilname(String encoded){
+    return encoded.replaceAll("%0A", "\n").replaceAll("%0D", "\r");
+  }
+  
+  /**
+   * as per https://github.com/jkunze/bagitspec/commit/152d42f6298b31a4916ea3f8f644ca4490494070 encode any new lines or carriage returns
+   * @param path the path to encode
+   * @return the encoded filename
+   */
+  public static String encodeFilename(Path path){
+    return path.toString().replaceAll("\n", "%0A").replaceAll("\r", "%0D");
+  }
 }

src/main/java/gov/loc/repository/bagit/writer/BagWriter.java

@@ -22,6 +22,7 @@
 import gov.loc.repository.bagit.domain.Manifest;
 import gov.loc.repository.bagit.domain.Version;
 import gov.loc.repository.bagit.hash.Hasher;
+import gov.loc.repository.bagit.util.PathUtils;
 import javafx.util.Pair;
 
 /**
@@ -92,7 +93,7 @@ protected static Path writeVersionDependentPayloadFiles(Bag bag, Path outputDir)
    * 
    * @throws IOException if there was a problem writing the file
    */
-  public static void writeBagitFile(Version version, String encoding, Path outputDir) throws IOException{
+  public static void writeBagitFile(Version version, Charset encoding, Path outputDir) throws IOException{
     Path bagitPath = outputDir.resolve("bagit.txt");
     logger.debug("Writing bagit.txt file to [{}]", outputDir);
 
@@ -143,7 +144,7 @@ public static void writePayloadFiles(Set<Manifest> payloadManifests, Path output
    * 
    * @throws IOException if there was a problem writing a file
    */
-  public static void writePayloadManifests(Set<Manifest> manifests, Path outputDir, Path bagitRootDir, String charsetName) throws IOException{
+  public static void writePayloadManifests(Set<Manifest> manifests, Path outputDir, Path bagitRootDir, Charset charsetName) throws IOException{
     logger.info("Writing payload manifest(s)");
     writeManifests(manifests, outputDir, bagitRootDir, "manifest-", charsetName);
   }
@@ -178,12 +179,12 @@ protected static Set<Manifest> updateTagManifests(Bag bag, Path newBagRootDir) t
    * 
    * @throws IOException if there was a problem writing a file
    */
-  public static void writeTagManifests(Set<Manifest> tagManifests, Path outputDir, Path bagitRootDir, String charsetName) throws IOException{
+  public static void writeTagManifests(Set<Manifest> tagManifests, Path outputDir, Path bagitRootDir, Charset charsetName) throws IOException{
     logger.info("Writing tag manifest(s)");
     writeManifests(tagManifests, outputDir, bagitRootDir, "tagmanifest-", charsetName);
   }
 
-  protected static void writeManifests(Set<Manifest> manifests, Path outputDir, Path relativeTo, String filenameBase, String charsetName) throws IOException{
+  protected static void writeManifests(Set<Manifest> manifests, Path outputDir, Path relativeTo, String filenameBase, Charset charsetName) throws IOException{
     for(Manifest manifest : manifests){
       Path manifestPath = outputDir.resolve(filenameBase + manifest.getAlgorithm().getBagitName() + ".txt");
       logger.debug("Writing manifest to [{}]", manifestPath);
@@ -192,9 +193,10 @@ protected static void writeManifests(Set<Manifest> manifests, Path outputDir, Pa
       Files.createFile(manifestPath);
 
       for(Entry<Path, String> entry : manifest.getFileToChecksumMap().entrySet()){
-        String line = entry.getValue() + " " + relativeTo.relativize(entry.getKey()) + System.lineSeparator();
+        String line = entry.getValue() + " " + 
+            PathUtils.encodeFilename(relativeTo.relativize(entry.getKey())) + System.lineSeparator();
         logger.debug("Writing [{}] to [{}]", line, manifestPath);
-        Files.write(manifestPath, line.getBytes(Charset.forName(charsetName)), 
+        Files.write(manifestPath, line.getBytes(charsetName), 
             StandardOpenOption.APPEND, StandardOpenOption.CREATE);
       }
     }
@@ -224,7 +226,7 @@ protected static void writeAdditionalTagPayloadFiles(Set<Manifest> manifests, Pa
    * 
    * @throws IOException if there was a problem writing a file
    */
-  public static void writeBagitInfoFile(List<Pair<String, String>> metadata, Version version, Path outputDir, String charsetName) throws IOException{
+  public static void writeBagitInfoFile(List<Pair<String, String>> metadata, Version version, Path outputDir, Charset charsetName) throws IOException{
     Path bagInfoFilePath = outputDir.resolve("bag-info.txt");
     if(VERSION_0_95.compareTo(version) >= 0){
       bagInfoFilePath = outputDir.resolve("package-info.txt");
@@ -236,7 +238,7 @@ public static void writeBagitInfoFile(List<Pair<String, String>> metadata, Versi
     for(Pair<String, String> entry : metadata){
       String line = entry.getKey() + " : " + entry.getValue() + System.lineSeparator();
       logger.debug("Writing [{}] to [{}]", line, bagInfoFilePath);
-      Files.write(bagInfoFilePath, line.getBytes(Charset.forName(charsetName)), 
+      Files.write(bagInfoFilePath, line.getBytes(charsetName), 
           StandardOpenOption.APPEND, StandardOpenOption.CREATE);
     }
   }
@@ -250,15 +252,14 @@ public static void writeBagitInfoFile(List<Pair<String, String>> metadata, Versi
    * 
    * @throws IOException if there was a problem writing a file
    */
-  public static void writeFetchFile(List<FetchItem> itemsToFetch, Path outputDir, String charsetName) throws IOException{
+  public static void writeFetchFile(List<FetchItem> itemsToFetch, Path outputDir, Charset charsetName) throws IOException{
     logger.debug("Writing fetch.txt to [{}]", outputDir);
     Path fetchFilePath = outputDir.resolve("fetch.txt");
 
     for(FetchItem item : itemsToFetch){
       String line = item.toString();
       logger.debug("Writing [{}] to [{}]", line, fetchFilePath);
-      Files.write(fetchFilePath, line.getBytes(Charset.forName(charsetName)), 
-          StandardOpenOption.APPEND, StandardOpenOption.CREATE);
+      Files.write(fetchFilePath, line.getBytes(charsetName), StandardOpenOption.APPEND, StandardOpenOption.CREATE);
     }
   }
 }

src/test/java/gov/loc/repository/bagit/reader/BagReaderTest.java

@@ -207,7 +207,7 @@ public void testReadBagitFile()throws Exception{
     Path bagitFile = Paths.get(getClass().getClassLoader().getResource("bagitFiles/bagit-0.97.txt").toURI());
     Bag returnedBag = sut.readBagitTextFile(bagitFile, new Bag(new Version(0, 96)));
     assertEquals(new Version(0, 97), returnedBag.getVersion());
-    assertEquals(StandardCharsets.UTF_8.name(), returnedBag.getFileEncoding());
+    assertEquals(StandardCharsets.UTF_8, returnedBag.getFileEncoding());
   }
 
   @Test

src/test/java/gov/loc/repository/bagit/util/PathUtilsTest.java

@@ -0,0 +1,52 @@
+package gov.loc.repository.bagit.util;
+
+import java.nio.file.Path;
+import java.nio.file.Paths;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+public class PathUtilsTest extends Assert {
+
+  @Test
+  public void testDecode(){
+    //just carriage return 
+    String testString = "/foo/bar/ham%0D";
+    String expectedDecoded = "/foo/bar/ham\r";
+    String actualDecoded = PathUtils.decodeFilname(testString);
+    assertEquals(expectedDecoded, actualDecoded);
+    
+    //just new line
+    testString = "/foo/bar/ham%0A";
+    expectedDecoded = "/foo/bar/ham\n";
+    actualDecoded = PathUtils.decodeFilname(testString);
+    assertEquals(expectedDecoded, actualDecoded);
+    
+    //both carriage return and new line
+    testString = "/foo/bar/ham%0D%0A";
+    expectedDecoded = "/foo/bar/ham\r\n";
+    actualDecoded = PathUtils.decodeFilname(testString);
+    assertEquals(expectedDecoded, actualDecoded);
+  }
+  
+  @Test
+  public void testEncode(){
+    //just carriage return
+    Path testPath = Paths.get("/foo/bar/ham\r");
+    String expectedEncoded = "/foo/bar/ham%0D";
+    String actualEncoded = PathUtils.encodeFilename(testPath);
+    assertEquals(expectedEncoded, actualEncoded);
+    
+    //just new line
+    testPath = Paths.get("/foo/bar/ham\n");
+    expectedEncoded = "/foo/bar/ham%0A";
+    actualEncoded = PathUtils.encodeFilename(testPath);
+    assertEquals(expectedEncoded, actualEncoded);
+    
+    //both carriage return and new line
+    testPath = Paths.get("/foo/bar/ham\r\n");
+    expectedEncoded = "/foo/bar/ham%0D%0A";
+    actualEncoded = PathUtils.encodeFilename(testPath);
+    assertEquals(expectedEncoded, actualEncoded);
+  }
+}

src/test/java/gov/loc/repository/bagit/writer/BagWriterTest.java

@@ -148,13 +148,13 @@ public void testWriteBagitFile() throws Exception{
     Path bagit = rootDirPath.resolve("bagit.txt");
 
     assertFalse(Files.exists(bagit));
-    BagWriter.writeBagitFile(new Version(0, 97), StandardCharsets.UTF_8.name(), rootDirPath);
+    BagWriter.writeBagitFile(new Version(0, 97), StandardCharsets.UTF_8, rootDirPath);
     assertTrue(Files.exists(bagit));
 
     //test truncating existing
     long originalModified = Files.getLastModifiedTime(bagit).toMillis();
     long size = Files.size(bagit);
-    BagWriter.writeBagitFile(new Version(0, 97), StandardCharsets.UTF_8.name(), rootDirPath);
+    BagWriter.writeBagitFile(new Version(0, 97), StandardCharsets.UTF_8, rootDirPath);
     assertTrue(Files.exists(bagit));
     assertTrue(Files.getLastModifiedTime(bagit) + " should be >= " + originalModified, 
         Files.getLastModifiedTime(bagit).toMillis() >= originalModified);
@@ -174,10 +174,10 @@ public void testWriteBagitInfoFile() throws IOException{
     assertFalse(bagInfo.exists());
     assertFalse(packageInfo.exists());
 
-    BagWriter.writeBagitInfoFile(metadata, new Version(0,96), Paths.get(rootDir.toURI()), StandardCharsets.UTF_8.name());
+    BagWriter.writeBagitInfoFile(metadata, new Version(0,96), Paths.get(rootDir.toURI()), StandardCharsets.UTF_8);
     assertTrue(bagInfo.exists());
 
-    BagWriter.writeBagitInfoFile(metadata, new Version(0,95), Paths.get(rootDir.toURI()), StandardCharsets.UTF_8.name());
+    BagWriter.writeBagitInfoFile(metadata, new Version(0,95), Paths.get(rootDir.toURI()), StandardCharsets.UTF_8);
     assertTrue(packageInfo.exists());
   }
 
@@ -191,7 +191,7 @@ public void testWriteFetchFile() throws Exception{
 
 
     assertFalse(fetch.exists());
-    BagWriter.writeFetchFile(itemsToFetch, Paths.get(rootDir.toURI()), StandardCharsets.UTF_8.name());
+    BagWriter.writeFetchFile(itemsToFetch, Paths.get(rootDir.toURI()), StandardCharsets.UTF_8);
     assertTrue(fetch.exists());
   }
 
@@ -205,7 +205,7 @@ public void testWriteTagManifests() throws IOException{
     File tagManifest = new File(outputDir, "tagmanifest-md5.txt");
 
     assertFalse(tagManifest.exists());
-    BagWriter.writeTagManifests(tagManifests, Paths.get(outputDir.toURI()), Paths.get("/foo/bar/ham"), StandardCharsets.UTF_8.name());
+    BagWriter.writeTagManifests(tagManifests, Paths.get(outputDir.toURI()), Paths.get("/foo/bar/ham"), StandardCharsets.UTF_8);
     assertTrue(tagManifest.exists());
   }