From 5384a3e0842ef718c6fa3aa8e232a6011a540f51 Mon Sep 17 00:00:00 2001 From: David Gamez <1192523+davidgamez@users.noreply.github.com> Date: Tue, 16 Jan 2024 14:51:14 -0500 Subject: [PATCH] fix: authorize upload error on create job endpoint (#1642) --- .../gtfsvalidator/input/GtfsInput.java | 45 +------------ .../gtfsvalidator/util/HttpGetUtil.java | 65 +++++++++++++++++++ .../controller/ValidationController.java | 3 +- .../web/service/util/StorageHelper.java | 25 ++++--- .../controller/CreateJobEndpointTest.java | 20 ++++-- 5 files changed, 99 insertions(+), 59 deletions(-) create mode 100644 core/src/main/java/org/mobilitydata/gtfsvalidator/util/HttpGetUtil.java diff --git a/core/src/main/java/org/mobilitydata/gtfsvalidator/input/GtfsInput.java b/core/src/main/java/org/mobilitydata/gtfsvalidator/input/GtfsInput.java index 5e65c0874c..05738d1f9c 100644 --- a/core/src/main/java/org/mobilitydata/gtfsvalidator/input/GtfsInput.java +++ b/core/src/main/java/org/mobilitydata/gtfsvalidator/input/GtfsInput.java @@ -27,12 +27,9 @@ import java.util.zip.ZipInputStream; import org.apache.commons.compress.archivers.zip.ZipFile; import org.apache.commons.compress.utils.SeekableInMemoryByteChannel; -import org.apache.http.client.methods.CloseableHttpResponse; -import org.apache.http.client.methods.HttpGet; -import org.apache.http.impl.client.CloseableHttpClient; -import org.apache.http.impl.client.HttpClients; import org.mobilitydata.gtfsvalidator.notice.InvalidInputFilesInSubfolderNotice; import org.mobilitydata.gtfsvalidator.notice.NoticeContainer; +import org.mobilitydata.gtfsvalidator.util.HttpGetUtil; /** * GtfsInput provides a common interface for reading GTFS data, either from a ZIP archive or from a @@ -42,8 +39,6 @@ public abstract class GtfsInput implements Closeable { public static final String invalidInputMessage = "At least 1 GTFS file is in a subfolder. All GTFS files must reside at the root level directly."; - public static final String USER_AGENT_PREFIX = "MobilityData GTFS-Validator"; - /** * Creates a specific GtfsInput to read data from the given path. * @@ -147,7 +142,7 @@ public static GtfsInput createFromUrl( Files.createDirectories(targetDirectory); } try (OutputStream outputStream = Files.newOutputStream(targetPath)) { - loadFromUrl(sourceUrl, outputStream, validatorVersion); + HttpGetUtil.loadFromUrl(sourceUrl, outputStream, validatorVersion); } return createFromPath(targetPath, noticeContainer); } @@ -166,7 +161,7 @@ public static GtfsInput createFromUrlInMemory( URL sourceUrl, NoticeContainer noticeContainer, String validatorVersion) throws IOException, URISyntaxException { try (ByteArrayOutputStream outputStream = new ByteArrayOutputStream()) { - loadFromUrl(sourceUrl, outputStream, validatorVersion); + HttpGetUtil.loadFromUrl(sourceUrl, outputStream, validatorVersion); File zipFile = new File(sourceUrl.toString()); String fileName = zipFile.getName().replace(".zip", ""); if (containsGtfsFileInSubfolder( @@ -179,40 +174,6 @@ public static GtfsInput createFromUrlInMemory( } } - /** - * Downloads data from network. - * - * @param sourceUrl the fully qualified URL - * @param outputStream the output stream - * @param validatorVersion - * @throws IOException if no file could not be found at the specified location - * @throws URISyntaxException if URL is malformed - */ - private static void loadFromUrl(URL sourceUrl, OutputStream outputStream, String validatorVersion) - throws IOException, URISyntaxException { - try (CloseableHttpClient httpClient = HttpClients.createDefault()) { - HttpGet httpGet = new HttpGet(sourceUrl.toURI()); - httpGet.setHeader("User-Agent", getUserAgent(validatorVersion)); - try (CloseableHttpResponse httpResponse = httpClient.execute(httpGet)) { - httpResponse.getEntity().writeTo(outputStream); - } - } - } - - /** - * @param validatorVersion version of the validator - * @return the user agent string in the format: "MobilityData GTFS-Validator/{validatorVersion} - * (Java {java version})" - */ - private static String getUserAgent(String validatorVersion) { - return USER_AGENT_PREFIX - + "/" - + (validatorVersion != null ? validatorVersion : "") - + " (Java " - + System.getProperty("java.version") - + ")"; - } - /** * Lists all files inside the GTFS dataset, even if they are not CSV and do not have .txt * extension. diff --git a/core/src/main/java/org/mobilitydata/gtfsvalidator/util/HttpGetUtil.java b/core/src/main/java/org/mobilitydata/gtfsvalidator/util/HttpGetUtil.java new file mode 100644 index 0000000000..6290be4a5f --- /dev/null +++ b/core/src/main/java/org/mobilitydata/gtfsvalidator/util/HttpGetUtil.java @@ -0,0 +1,65 @@ +/* + * Copyright 2023 MobilityData + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.mobilitydata.gtfsvalidator.util; + +import java.io.IOException; +import java.io.OutputStream; +import java.net.URISyntaxException; +import java.net.URL; +import org.apache.http.client.methods.CloseableHttpResponse; +import org.apache.http.client.methods.HttpGet; +import org.apache.http.impl.client.CloseableHttpClient; +import org.apache.http.impl.client.HttpClients; + +public class HttpGetUtil { + + public static final String USER_AGENT_PREFIX = "MobilityData GTFS-Validator"; + + /** + * @param validatorVersion version of the validator + * @return the user agent string in the format: "MobilityData GTFS-Validator/{validatorVersion} + * (Java {java version})" + */ + public static String getUserAgent(String validatorVersion) { + return USER_AGENT_PREFIX + + "/" + + (validatorVersion != null ? validatorVersion : "") + + " (Java " + + System.getProperty("java.version") + + ")"; + } + + /** + * Downloads data from network. + * + * @param sourceUrl the fully qualified URL + * @param outputStream the output stream + * @param validatorVersion the version of the validator + * @throws IOException if no file could not be found at the specified location + * @throws URISyntaxException if URL is malformed + */ + public static void loadFromUrl(URL sourceUrl, OutputStream outputStream, String validatorVersion) + throws IOException, URISyntaxException { + try (CloseableHttpClient httpClient = HttpClients.createDefault()) { + HttpGet httpGet = new HttpGet(sourceUrl.toURI()); + httpGet.setHeader("User-Agent", getUserAgent(validatorVersion)); + try (CloseableHttpResponse httpResponse = httpClient.execute(httpGet)) { + httpResponse.getEntity().writeTo(outputStream); + } + } + } +} diff --git a/web/service/src/main/java/org/mobilitydata/gtfsvalidator/web/service/controller/ValidationController.java b/web/service/src/main/java/org/mobilitydata/gtfsvalidator/web/service/controller/ValidationController.java index 61c8f1b3d1..765fa4d2be 100644 --- a/web/service/src/main/java/org/mobilitydata/gtfsvalidator/web/service/controller/ValidationController.java +++ b/web/service/src/main/java/org/mobilitydata/gtfsvalidator/web/service/controller/ValidationController.java @@ -62,7 +62,8 @@ public CreateJobResponse createJob(@RequestBody CreateJobRequest body) { storageHelper.saveJobMetadata(new JobMetadata(jobId, body.getCountryCode())); } if (!Strings.isNullOrEmpty(body.getUrl())) { - storageHelper.saveJobFileFromUrl(jobId, body.getUrl()); + var validatorVersion = versionResolver.resolveCurrentVersion(); + storageHelper.saveJobFileFromUrl(jobId, body.getUrl(), validatorVersion.orElse(null)); } else { uploadUrl = storageHelper.generateUniqueUploadUrl(jobId); } diff --git a/web/service/src/main/java/org/mobilitydata/gtfsvalidator/web/service/util/StorageHelper.java b/web/service/src/main/java/org/mobilitydata/gtfsvalidator/web/service/util/StorageHelper.java index 9ae9015b12..4d69a8aee2 100644 --- a/web/service/src/main/java/org/mobilitydata/gtfsvalidator/web/service/util/StorageHelper.java +++ b/web/service/src/main/java/org/mobilitydata/gtfsvalidator/web/service/util/StorageHelper.java @@ -1,9 +1,11 @@ package org.mobilitydata.gtfsvalidator.web.service.util; import com.fasterxml.jackson.databind.ObjectMapper; +import com.google.cloud.WriteChannel; import com.google.cloud.storage.*; import java.io.*; import java.net.URL; +import java.nio.channels.Channels; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; @@ -11,6 +13,7 @@ import java.util.Map; import java.util.UUID; import java.util.concurrent.TimeUnit; +import org.mobilitydata.gtfsvalidator.util.HttpGetUtil; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.context.ApplicationContext; @@ -101,18 +104,20 @@ public JobMetadata getJobMetadata(String jobId) { * * @param jobId * @param url + * @param validatorVersion * @throws Exception */ - public void saveJobFileFromUrl(String jobId, String url) throws Exception { - // Read file into memory - var urlInputStream = new BufferedInputStream(new URL(url).openStream()); - - // Upload to GCS - var blobId = BlobId.of(USER_UPLOAD_BUCKET_NAME, jobId + "/" + jobId + ".zip"); - var mimeType = "application/zip"; - var blobInfo = BlobInfo.newBuilder(blobId).setContentType(mimeType).build(); - var fileBytes = urlInputStream.readAllBytes(); - storage.create(blobInfo, fileBytes); + public void saveJobFileFromUrl(String jobId, String url, String validatorVersion) + throws Exception { + var blobId = BlobId.of(USER_UPLOAD_BUCKET_NAME, jobId + "/" + FILE_NAME); + var blobInfo = BlobInfo.newBuilder(blobId).setContentType("application/zip").build(); + URL signedURL = + storage.signUrl( + blobInfo, 1, TimeUnit.HOURS, Storage.SignUrlOption.httpMethod(HttpMethod.POST)); + try (WriteChannel writer = storage.writer(signedURL)) { + OutputStream outputStream = Channels.newOutputStream(writer); + HttpGetUtil.loadFromUrl(new URL(url), outputStream, validatorVersion); + } } /** Generates a job-specific signed URL for uploading a file to GCS. */ diff --git a/web/service/src/test/java/org/mobilitydata/gtfsvalidator/web/service/controller/CreateJobEndpointTest.java b/web/service/src/test/java/org/mobilitydata/gtfsvalidator/web/service/controller/CreateJobEndpointTest.java index 556abf5fce..86e4bee783 100644 --- a/web/service/src/test/java/org/mobilitydata/gtfsvalidator/web/service/controller/CreateJobEndpointTest.java +++ b/web/service/src/test/java/org/mobilitydata/gtfsvalidator/web/service/controller/CreateJobEndpointTest.java @@ -4,9 +4,12 @@ import static org.mockito.Mockito.*; import com.fasterxml.jackson.databind.ObjectMapper; +import java.io.IOException; import java.net.URL; +import java.util.Optional; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.mobilitydata.gtfsvalidator.util.VersionResolver; import org.mobilitydata.gtfsvalidator.web.service.util.JobMetadata; import org.mobilitydata.gtfsvalidator.web.service.util.StorageHelper; import org.mobilitydata.gtfsvalidator.web.service.util.ValidationHandler; @@ -32,10 +35,14 @@ public class CreateJobEndpointTest { @Captor ArgumentCaptor jobMetadataCaptor; + @MockBean private VersionResolver versionResolver; + private final ObjectMapper mapper = new ObjectMapper(); private String testJobId; private String testUploadUrl; + private final String VALIDATOR_TEST_VERSION = "1.0.0"; + private void makeCreateJobRequestAndCheckResult( CreateJobRequest request, String expectedJobId, String expectedUploadUrl) throws Exception { var json = mapper.writeValueAsString(request); @@ -51,10 +58,11 @@ private void makeCreateJobRequestAndCheckResult( } @BeforeEach - public void setUp() { + public void setUp() throws IOException { testJobId = "123"; testUploadUrl = "https://gcs.io/bucket/123"; doReturn(testJobId).when(storageHelper).createNewJobId(); + doReturn(Optional.of(VALIDATOR_TEST_VERSION)).when(versionResolver).resolveCurrentVersion(); } @Test @@ -67,7 +75,7 @@ public void createJobWithNoUrlNoCountryCode() throws Exception { // should not call saveJobMetadata verify(storageHelper, times(0)).saveJobMetadata(any(JobMetadata.class)); // should not call saveJobFileFromUrl - verify(storageHelper, times(0)).saveJobFileFromUrl(anyString(), anyString()); + verify(storageHelper, times(0)).saveJobFileFromUrl(anyString(), anyString(), anyString()); } @Test @@ -85,7 +93,7 @@ public void createJobWithCountryCodeButNoUrl() throws Exception { assert jobMetadata.getCountryCode().equals("US"); // should not call saveJobFileFromUrl - verify(storageHelper, times(0)).saveJobFileFromUrl(anyString(), anyString()); + verify(storageHelper, times(0)).saveJobFileFromUrl(anyString(), anyString(), anyString()); } @Test @@ -99,7 +107,7 @@ public void createJobWithUrlButNoCountryCode() throws Exception { // should not call saveJobMetadata verify(storageHelper, times(0)).saveJobMetadata(any(JobMetadata.class)); // should saveJobFileFromUrl - verify(storageHelper, times(1)).saveJobFileFromUrl(testJobId, url); + verify(storageHelper, times(1)).saveJobFileFromUrl(testJobId, url, VALIDATOR_TEST_VERSION); } @Test @@ -117,7 +125,7 @@ public void createJobWithUrlAndCountryCode() throws Exception { assert jobMetadata.getJobId().equals(testJobId); assert jobMetadata.getCountryCode().equals("US"); // should saveJobFileFromUrl - verify(storageHelper, times(1)).saveJobFileFromUrl(testJobId, url); + verify(storageHelper, times(1)).saveJobFileFromUrl(testJobId, url, VALIDATOR_TEST_VERSION); } @Test @@ -140,7 +148,7 @@ public void createJobShouldReturn500ErrorIfSaveJobFileFromUrlThrowsException() t doReturn(testJobId).when(storageHelper).createNewJobId(); doThrow(new RuntimeException("test exception")) .when(storageHelper) - .saveJobFileFromUrl(any(), any()); + .saveJobFileFromUrl(anyString(), anyString(), anyString()); String url = "http://myfilehost.com/myfile.zip"; var request = new CreateJobRequest("US", url); var json = mapper.writeValueAsString(request);