Merge 2e24fea into 900afe9

grails-app/services/org/bbop/apollo/FileService.groovy

@@ -14,6 +14,8 @@ import org.springframework.web.multipart.commons.CommonsMultipartFile
 
 import java.nio.file.FileSystemException
 import java.nio.file.Files
+import java.nio.file.Path
+import java.nio.file.Paths
 import java.nio.file.StandardCopyOption
 
 @Transactional
@@ -75,6 +77,8 @@ class FileService {
                     continue;
                 }
 
+                // TODO: handle symbolic link
+
                 File outputFile = new File(initialLocation, entry.getName());
 
                 if (outputFile.isDirectory()) {
@@ -119,20 +123,22 @@ class FileService {
     /**
      * Decompress a tar.gz archive to a folder specified by directoryName in the given path
      * @param tarFile
-     * @param path
+     * @param pathString
      * @param directoryName
      * @param tempDir
      * @return
      */
-    List<String> decompressTarArchive(File tarFile, String path, String directoryName = null, boolean tempDir = false) {
+    List<String> decompressTarArchive(File tarFile, String pathString, String directoryName = null, boolean tempDir = false) {
         List<String> fileNames = []
         boolean atArchiveRoot = true
         String archiveRootDirectoryName
-        String initialLocation = tempDir ? path + File.separator + "temp" : path
+        String initialLocation = tempDir ? pathString + File.separator + "temp" : pathString
         log.debug "initial location: ${initialLocation}"
         TarArchiveInputStream tais = new TarArchiveInputStream(new GzipCompressorInputStream(new FileInputStream(tarFile)))
         TarArchiveEntry entry = null
 
+        Path destDir = Paths.get(pathString)
+        String prefix = destDir.toString()
 
         while ((entry = (TarArchiveEntry) tais.getNextEntry()) != null) {
             if (atArchiveRoot) {
@@ -141,29 +147,36 @@ class FileService {
             }
 
             try {
-                validateFileName(entry.getName(), archiveRootDirectoryName)
+                String fileName = validateFileName(entry.getName(), archiveRootDirectoryName)
+                Path path = destDir.resolve(entry.getName()).normalize();
+                if(!pathString.toString().startsWith(prefix)){
+                    throw new RuntimeException("Archive includes an invalid entry: " + entry.getName());
+                }
                 if (entry.isDirectory()) {
-                    File dir = new File(initialLocation, entry.getName())
-                    if (!dir.exists()) {
-                        dir.mkdirs()
-                    }
-                    continue;
+                    Files.createDirectories(path)
                 }
-
-                File outputFile = new File(initialLocation, entry.getName())
-
-                if (outputFile.isDirectory()) {
-                    continue;
+                else if (entry.isSymbolicLink()) {
+                    println "is a symlink. ${entry.name}"
+                    String dest = entry.getLinkName();
+                    Path destAbsPath = path.getParent().resolve(dest).normalize();
+                    if (!destAbsPath.normalize().toString().startsWith(prefix)) {
+                        throw new RuntimeException("Archive includes an invalid symlink: " + entry.getName() + " -> " + dest);
+                    }
+                    Files.createSymbolicLink(path, Paths.get(dest));
+                    fileNames.add(destAbsPath.toString())
                 }
+                else{
+                    Files.createDirectories(path.getParent());
+                    File outputFile = new File(initialLocation, entry.getName())
+                    if (outputFile.exists()) {
+                        continue;
+                    }
 
-                if (outputFile.exists()) {
-                    continue;
+                    FileOutputStream fos = new FileOutputStream(outputFile)
+                    IOUtils.copy(tais, fos)
+                    fos.close()
+                    fileNames.add(outputFile.absolutePath)
                 }
-
-                FileOutputStream fos = new FileOutputStream(outputFile)
-                IOUtils.copy(tais, fos)
-                fos.close()
-                fileNames.add(outputFile.absolutePath)
             } catch (IOException e) {
                 log.error("Problem decrompression file ${entry.name} vs ${archiveRootDirectoryName}", e)
             }
@@ -172,7 +185,7 @@ class FileService {
         if (tempDir) {
             // move files from temp directory to folder supplied via directoryName
             String unpackedArchiveLocation = initialLocation + File.separator + archiveRootDirectoryName
-            String finalLocation = path + File.separator + directoryName
+            String finalLocation = pathString + File.separator + directoryName
             File finalLocationFile = new File(finalLocation)
             if (finalLocationFile.mkdir()) log.debug "${finalLocation} directory created"
             try {
@@ -327,12 +340,14 @@ class FileService {
      * @return
      * @throws IOException
      */
-    def validateFileName(String fileName, String intendedOutputDirectory) throws IOException {
+    String validateFileName(String fileName, String intendedOutputDirectory) throws IOException {
+        println "intpu filename ${fileName} ${intendedOutputDirectory}"
         File file = new File(fileName)
         String canonicalPath = file.getCanonicalPath()
         File intendedOutputDirectoryFile = new File(intendedOutputDirectory)
         String canonicalIntendedOutputDirectoryPath = intendedOutputDirectoryFile.getCanonicalPath()
-        if (canonicalPath.startsWith(canonicalIntendedOutputDirectoryPath)) {
+        println "canonical path ${canonicalIntendedOutputDirectoryPath} vs $canonicalPath = > ${canonicalIntendedOutputDirectoryPath == canonicalPath}"
+        if (canonicalPath.startsWith(canonicalIntendedOutputDirectoryPath) || canonicalPath == canonicalIntendedOutputDirectoryPath) {
             return canonicalPath
         } else {
             throw new IOException("File is outside extraction target directory.")

test/unit/org/bbop/apollo/FileServiceSpec.groovy

@@ -0,0 +1,77 @@
+package org.bbop.apollo
+
+import grails.test.mixin.TestFor
+import spock.lang.Specification
+
+import java.nio.file.Files
+import java.nio.file.Paths
+
+/**
+ * See the API for {@link grails.test.mixin.services.ServiceUnitTestMixin} for usage instructions
+ */
+@TestFor(FileService)
+class FileServiceSpec extends Specification {
+
+    private final String FINAL_DIRECTORY = "test/unit/resources/archive_tests/"
+
+    File parentDir = new File(FINAL_DIRECTORY+"data")
+    File fileA = new File(FINAL_DIRECTORY+"data/a.txt")
+    File fileB = new File(FINAL_DIRECTORY+"data/b.txt")
+
+    def setup() {
+    }
+
+    def cleanup(){
+        fileA.delete()
+        fileB.delete()
+        parentDir.delete()
+    }
+
+    void "handle tar.gz decompress"() {
+
+        given: "a tar.gz file"
+        File inputFile = new File(FINAL_DIRECTORY + "/no_symlinks.tgz" )
+        println "input file ${inputFile} ${inputFile.exists()}"
+        println "current working directory  ${new File(".").absolutePath}"
+        assert inputFile.exists()
+        assert !fileA.exists()
+        assert !fileB.exists()
+
+        when: "we expand it"
+        List<String> fileNames = service.decompressTarArchive(inputFile,FINAL_DIRECTORY)
+        println "fileNames ${fileNames.join(",")}"
+
+        then: "we should have the right file"
+        assert fileA.exists()
+        assert fileB.exists()
+        assert fileA.text == 'aaa\n'
+        assert fileB.text == 'bbb\n'
+
+
+    }
+
+    void "handle symlinks"() {
+
+        given: "a tar.gz file"
+        File inputFile = new File(FINAL_DIRECTORY + "/symlinks.tgz" )
+        println "current working directory  ${new File(".").absolutePath}"
+        assert inputFile.exists()
+        assert !fileA.exists()
+        assert !fileB.exists()
+
+        when: "we expand it"
+        List<String> fileNames = service.decompressTarArchive(inputFile,FINAL_DIRECTORY)
+        println "fileNames should have a symlink in it ${fileNames.join(",")}"
+
+        then: "we should have the right file"
+        assert fileB.exists()
+        assert fileB.text == 'bbb - no symlink\n'
+        assert !fileA.exists()
+        assert Files.isSymbolicLink(Paths.get(fileA.absolutePath))
+
+
+    }
+
+
+
+}