Fix #90: use proper URL encoding when downloading jobs

worker/src/main/java/nl/tudelft/ewi/auta/worker/JobSplitter.java

@@ -68,7 +68,7 @@ public class JobSplitter {
      * @return the job object for this message
      */
     public Job split(final Message message) throws IOException {
-        final var arPath = this.downloader.download(this.downloader.getDownloadUrl(message));
+        final var arPath = this.downloader.download(this.downloader.getDownloadUri(message));
 
         var submissionDirectory = Files.createTempDirectory("submission-");
         final var rawFragments = this.unzipper.unzip(arPath, submissionDirectory);

worker/src/main/java/nl/tudelft/ewi/auta/worker/files/JobDownloader.java

@@ -3,7 +3,8 @@ package nl.tudelft.ewi.auta.worker.files;
 import java.io.IOException;
 import java.net.HttpURLConnection;
 import java.net.MalformedURLException;
-import java.net.URL;
+import java.net.URI;
+import java.net.URISyntaxException;
 import java.nio.file.Files;
 import java.nio.file.Path;
 
@@ -40,21 +41,21 @@ public class JobDownloader {
     }
 
     /**
-     * Downloads the file at the given URL.
+     * Downloads the file at the given URI.
      *
-     * @param urlstr the URL to download
+     * @param uri the URI to download
      *
      * @return the path to the downloaded archive
      *
      * @throws IOException if the job could not be downloaded
      */
-    public Path download(final String urlstr) throws IOException {
+    public Path download(final URI uri) throws IOException {
         try {
-            logger.info("Trying to download from {}", urlstr);
-            final var url = (HttpURLConnection) new URL(urlstr).openConnection();
+            logger.info("Trying to download from {}", uri);
+            final var url = (HttpURLConnection) uri.toURL().openConnection();
 
             final var file = Files.createTempFile(
-                    this.settings.getTemp(), "job-", this.getExtension(urlstr)
+                    this.settings.getTemp(), "job-", this.getExtension(uri.getPath())
             );
 
             url.setRequestProperty("Auth-Token", this.settings.getApiAuthToken());
@@ -62,7 +63,7 @@ public class JobDownloader {
 
             if (url.getResponseCode() != 200) {
                 logger.error("Partial, no or error response {} {} from {}",
-                        url.getResponseCode(), url.getResponseMessage(), urlstr
+                        url.getResponseCode(), url.getResponseMessage(), uri
                 );
                 throw new BadJobException("Incomplete or error response " + url.getResponseCode());
             }
@@ -71,7 +72,7 @@ public class JobDownloader {
                  var in = url.getInputStream()) {
                 final var contentLength = url.getContentLengthLong();
 
-                logger.debug("Downloading {} bytes from {}", contentLength, urlstr);
+                logger.debug("Downloading {} bytes from {}", contentLength, uri);
 
                 byte[] buf = new byte[BUFFER_SIZE];
 
@@ -101,11 +102,15 @@ public class JobDownloader {
      *
      * @return the URL
      */
-    public String getDownloadUrl(final Message job) {
-        return String.format("%s://%s:%s/api/v1/files/get%s",
-                this.settings.getApiProtocol(), this.settings.getHost(),
-                this.settings.getApiPort(), job.getData()
+    public URI getDownloadUri(final Message job) {
+        try {
+            return new URI(
+                    this.settings.getApiProtocol(), "", this.settings.getHost(),
+                    this.settings.getApiPort(), "/api/v1/files/get" + job.getData(), "", ""
             );
+        } catch (final URISyntaxException ex) {
+            throw new RuntimeException(ex);
+        }
     }
 
     /**

worker/src/test/java/nl/tudelft/ewi/auta/worker/files/JobDownloaderTest.java

@@ -4,6 +4,8 @@ import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 
 import java.io.IOException;
+import java.net.URI;
+import java.net.URISyntaxException;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.Collections;
@@ -40,8 +42,8 @@ public class JobDownloaderTest {
     }
 
     @Test
-    public void testDownload() throws IOException {
-        final var path = this.downloader.download("https://auta.f00f.nl/f00f.txt");
+    public void testDownload() throws IOException, URISyntaxException {
+        final var path = this.downloader.download(new URI("https://auta.f00f.nl/f00f.txt"));
 
         assertThat(path.toString()).endsWith(".txt");
         assertThat(Files.size(path)).isEqualTo(0xF00F);
@@ -50,42 +52,41 @@ public class JobDownloaderTest {
     @Test
     public void testDownloadNotFound() {
         assertThrows(BadJobException.class, () ->
-                this.downloader.download("https://auta.f00f.nl/f00f.jar")
+                this.downloader.download(new URI("https://auta.f00f.nl/f00f.jar"))
         );
     }
 
-    @Test
-    public void testDownloadSillyUrl() {
-        assertThrows(BadJobException.class, () -> this.downloader.download("stuff://"));
-    }
-
     @Test
     public void buildUrl() {
         final var job = new Message("name", Collections.emptyMap(), "/data.zip");
+        final var uri = this.downloader.getDownloadUri(job);
 
-        assertThat(this.downloader.getDownloadUrl(job))
-                .isEqualTo("jump://auta.f00f:61455/api/v1/files/get/data.zip");
+        assertThat(uri)
+                .hasScheme("jump")
+                .hasHost("auta.f00f")
+                .hasPort(61455)
+                .hasPath("/api/v1/files/get/data.zip");
 
     }
 
     @Test
     public void buildUrlWithoutExtension() {
         assertThrows(BadJobException.class, () ->
-                this.downloader.download("https://auta.f00f.nl/nl/f00f/tarmac/tarmac")
+                this.downloader.download(new URI("https://auta.f00f.nl/nl/f00f/tarmac/tarmac"))
         );
     }
 
     @Test
     public void buildUrlWithoutExtensionTwo() {
         assertThrows(BadJobException.class, () ->
-                this.downloader.download("https://auta.f00f.nl/nl/f00f/tarmac/.tarmac")
+                this.downloader.download(new URI("https://auta.f00f.nl/nl/f00f/tarmac/.tarmac"))
         );
     }
 
     @Test
     public void buildUrlWithoutExtensionThree() {
         assertThrows(BadJobException.class, () ->
-                this.downloader.download("https://localhost/nl/f00f/tarmac/tarmac")
+                this.downloader.download(new URI("https://localhost/nl/f00f/tarmac/tarmac"))
         );
     }
 

worker/src/test/java/nl/tudelft/ewi/auta/worker/files/JobUnzipperTest.java

@@ -4,6 +4,8 @@ import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 
 import java.io.IOException;
+import java.net.URI;
+import java.net.URISyntaxException;
 import java.nio.file.Files;
 import java.nio.file.Path;
 
@@ -38,12 +40,14 @@ public class JobUnzipperTest {
 
     @Test
     @Disabled
-    public void testUnzip() throws IOException {
+    public void testUnzip() throws IOException, URISyntaxException {
         Mockito.when(this.settings.getMaxUnzippedSubmissionSize()).thenReturn(
                 512 * 1024 * 1024L
         );
 
-        final var path = this.downloader.download("https://auta.f00f.nl/nl/f00f/tarmac/tarmac.zip");
+        final var path = this.downloader.download(
+                new URI("https://auta.f00f.nl/nl/f00f/tarmac/tarmac.zip")
+        );
 
         final var files = this.unzipper.unzip(path, this.temp);
 
@@ -52,10 +56,12 @@ public class JobUnzipperTest {
 
     @Test
     @Disabled
-    public void testUnzipTooLarge() throws IOException {
+    public void testUnzipTooLarge() throws IOException, URISyntaxException {
         Mockito.when(this.settings.getMaxUnzippedSubmissionSize()).thenReturn(1024L);
 
-        final var path = this.downloader.download("https://auta.f00f.nl/nl/f00f/tarmac/tarmac.zip");
+        final var path = this.downloader.download(
+                new URI("https://auta.f00f.nl/nl/f00f/tarmac/tarmac.zip")
+        );
 
         assertThrows(MaliciousZipException.class, () -> this.unzipper.unzip(path, this.temp));
     }