From d4bcff3d26a836f32e4224a56650b3b8311d2876 Mon Sep 17 00:00:00 2001 From: tylerjmchugh Date: Fri, 13 Dec 2024 15:42:26 -0500 Subject: [PATCH 1/6] Check content-length of resource at url --- .../records/attachments/AbstractStore.java | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/core/src/main/java/org/fao/geonet/api/records/attachments/AbstractStore.java b/core/src/main/java/org/fao/geonet/api/records/attachments/AbstractStore.java index e9b6a59593e..202c88c2963 100644 --- a/core/src/main/java/org/fao/geonet/api/records/attachments/AbstractStore.java +++ b/core/src/main/java/org/fao/geonet/api/records/attachments/AbstractStore.java @@ -27,6 +27,7 @@ import jeeves.server.context.ServiceContext; import org.apache.commons.io.FilenameUtils; import org.fao.geonet.ApplicationContextHolder; +import org.fao.geonet.api.exception.GeonetMaxUploadSizeExceededException; import org.fao.geonet.api.exception.NotAllowedException; import org.fao.geonet.api.exception.ResourceNotFoundException; import org.fao.geonet.domain.AbstractMetadata; @@ -35,8 +36,10 @@ import org.fao.geonet.kernel.AccessManager; import org.fao.geonet.kernel.datamanager.IMetadataUtils; import org.fao.geonet.repository.MetadataRepository; +import org.fao.geonet.util.FileUtil; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.springframework.beans.factory.annotation.Value; import org.springframework.context.ApplicationContext; import org.springframework.web.multipart.MultipartFile; @@ -59,6 +62,9 @@ public abstract class AbstractStore implements Store { protected static final String RESOURCE_MANAGEMENT_EXTERNAL_PROPERTIES_ESCAPED_SEPARATOR = "\\:"; private static final Logger log = LoggerFactory.getLogger(AbstractStore.class); + @Value("${api.params.maxUploadSize}") + private int maxUploadSize; + @Override public final List getResources(final ServiceContext context, final String metadataUuid, final Sort sort, final String filter) throws Exception { @@ -180,6 +186,23 @@ protected String getFilenameFromHeader(final URL fileUrl) throws IOException { } } + protected long getContentLengthFromHeader(final URL fileUrl) { + HttpURLConnection connection = null; + try { + connection = (HttpURLConnection) fileUrl.openConnection(); + connection.setRequestMethod("HEAD"); + connection.connect(); + return connection.getContentLengthLong(); + } catch (Exception e) { + log.error("Error retrieving resource content length from header", e); + return -1; + } finally { + if (connection != null) { + connection.disconnect(); + } + } + } + protected String getFilenameFromUrl(final URL fileUrl) { String fileName = FilenameUtils.getName(fileUrl.getPath()); if (fileName.contains("?")) { @@ -226,6 +249,15 @@ public final MetadataResource putResource(ServiceContext context, String metadat @Override public final MetadataResource putResource(ServiceContext context, String metadataUuid, URL fileUrl, MetadataResourceVisibility visibility, Boolean approved) throws Exception { + long contentLength = getContentLengthFromHeader(fileUrl); + if (contentLength > maxUploadSize) { + throw new GeonetMaxUploadSizeExceededException("uploadedResourceSizeExceededException") + .withMessageKey("exception.maxUploadSizeExceeded", + new String[]{FileUtil.humanizeFileSize(maxUploadSize)}) + .withDescriptionKey("exception.maxUploadSizeExceeded.description", + new String[]{FileUtil.humanizeFileSize(contentLength), + FileUtil.humanizeFileSize(maxUploadSize)}); + } String filename = getFilenameFromHeader(fileUrl); if (filename == null) { filename = getFilenameFromUrl(fileUrl); From a0e2fdc4a68e3d29b813c526da53193fa76576bb Mon Sep 17 00:00:00 2001 From: tylerjmchugh Date: Fri, 13 Dec 2024 15:42:56 -0500 Subject: [PATCH 2/6] Remove unnecessary throw --- .../org/fao/geonet/api/records/attachments/AbstractStore.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/core/src/main/java/org/fao/geonet/api/records/attachments/AbstractStore.java b/core/src/main/java/org/fao/geonet/api/records/attachments/AbstractStore.java index 202c88c2963..07378ab553a 100644 --- a/core/src/main/java/org/fao/geonet/api/records/attachments/AbstractStore.java +++ b/core/src/main/java/org/fao/geonet/api/records/attachments/AbstractStore.java @@ -44,7 +44,6 @@ import org.springframework.web.multipart.MultipartFile; import java.io.BufferedInputStream; -import java.io.IOException; import java.io.InputStream; import java.net.HttpURLConnection; import java.net.URL; @@ -163,7 +162,7 @@ protected int canDownload(ServiceContext context, String metadataUuid, MetadataR return metadataId; } - protected String getFilenameFromHeader(final URL fileUrl) throws IOException { + protected String getFilenameFromHeader(final URL fileUrl) { HttpURLConnection connection = null; try { connection = (HttpURLConnection) fileUrl.openConnection(); From b5b6dbf284075c6567f19c72fdd9c4433ef72a1f Mon Sep 17 00:00:00 2001 From: tylerjmchugh Date: Tue, 17 Dec 2024 12:49:14 -0500 Subject: [PATCH 3/6] Fallback to InputStream.available() if content length is not found or smaller --- .../records/attachments/AbstractStore.java | 24 +++++++++++-------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/core/src/main/java/org/fao/geonet/api/records/attachments/AbstractStore.java b/core/src/main/java/org/fao/geonet/api/records/attachments/AbstractStore.java index 07378ab553a..58e7a009544 100644 --- a/core/src/main/java/org/fao/geonet/api/records/attachments/AbstractStore.java +++ b/core/src/main/java/org/fao/geonet/api/records/attachments/AbstractStore.java @@ -248,20 +248,24 @@ public final MetadataResource putResource(ServiceContext context, String metadat @Override public final MetadataResource putResource(ServiceContext context, String metadataUuid, URL fileUrl, MetadataResourceVisibility visibility, Boolean approved) throws Exception { - long contentLength = getContentLengthFromHeader(fileUrl); - if (contentLength > maxUploadSize) { - throw new GeonetMaxUploadSizeExceededException("uploadedResourceSizeExceededException") - .withMessageKey("exception.maxUploadSizeExceeded", - new String[]{FileUtil.humanizeFileSize(maxUploadSize)}) - .withDescriptionKey("exception.maxUploadSizeExceeded.description", - new String[]{FileUtil.humanizeFileSize(contentLength), - FileUtil.humanizeFileSize(maxUploadSize)}); - } String filename = getFilenameFromHeader(fileUrl); if (filename == null) { filename = getFilenameFromUrl(fileUrl); } - return putResource(context, metadataUuid, filename, fileUrl.openStream(), null, visibility, approved); + try (InputStream is = fileUrl.openStream()) { + long contentLength = getContentLengthFromHeader(fileUrl); + long availableBytes = is.available(); + long fileSize = availableBytes > contentLength ? availableBytes : contentLength; + if (fileSize > maxUploadSize) { + throw new GeonetMaxUploadSizeExceededException("uploadedResourceSizeExceededException") + .withMessageKey("exception.maxUploadSizeExceeded", + new String[]{FileUtil.humanizeFileSize(maxUploadSize)}) + .withDescriptionKey("exception.maxUploadSizeExceeded.description", + new String[]{FileUtil.humanizeFileSize(fileSize), + FileUtil.humanizeFileSize(maxUploadSize)}); + } + return putResource(context, metadataUuid, filename, is, null, visibility, approved); + } } @Override From 2b54b4dd8a0ba9550fee1d8e040672e4a48e4ed2 Mon Sep 17 00:00:00 2001 From: tylerjmchugh Date: Tue, 17 Dec 2024 14:54:49 -0500 Subject: [PATCH 4/6] Fallback maxUploadSize for unit tests --- .../org/fao/geonet/api/records/attachments/AbstractStore.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/org/fao/geonet/api/records/attachments/AbstractStore.java b/core/src/main/java/org/fao/geonet/api/records/attachments/AbstractStore.java index 58e7a009544..3beeb82b386 100644 --- a/core/src/main/java/org/fao/geonet/api/records/attachments/AbstractStore.java +++ b/core/src/main/java/org/fao/geonet/api/records/attachments/AbstractStore.java @@ -61,8 +61,8 @@ public abstract class AbstractStore implements Store { protected static final String RESOURCE_MANAGEMENT_EXTERNAL_PROPERTIES_ESCAPED_SEPARATOR = "\\:"; private static final Logger log = LoggerFactory.getLogger(AbstractStore.class); - @Value("${api.params.maxUploadSize}") - private int maxUploadSize; + @Value("${api.params.maxUploadSize:100000000}") + private long maxUploadSize; @Override public final List getResources(final ServiceContext context, final String metadataUuid, final Sort sort, From 4d6a9e77078468cb3e653eb9226f591da8ed726e Mon Sep 17 00:00:00 2001 From: tylerjmchugh Date: Wed, 18 Dec 2024 09:12:25 -0500 Subject: [PATCH 5/6] Fix default value always used --- .../org/fao/geonet/api/records/attachments/AbstractStore.java | 4 ++-- core/src/test/resources/WEB-INF/config.properties | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/org/fao/geonet/api/records/attachments/AbstractStore.java b/core/src/main/java/org/fao/geonet/api/records/attachments/AbstractStore.java index 3beeb82b386..58e7a009544 100644 --- a/core/src/main/java/org/fao/geonet/api/records/attachments/AbstractStore.java +++ b/core/src/main/java/org/fao/geonet/api/records/attachments/AbstractStore.java @@ -61,8 +61,8 @@ public abstract class AbstractStore implements Store { protected static final String RESOURCE_MANAGEMENT_EXTERNAL_PROPERTIES_ESCAPED_SEPARATOR = "\\:"; private static final Logger log = LoggerFactory.getLogger(AbstractStore.class); - @Value("${api.params.maxUploadSize:100000000}") - private long maxUploadSize; + @Value("${api.params.maxUploadSize}") + private int maxUploadSize; @Override public final List getResources(final ServiceContext context, final String metadataUuid, final Sort sort, diff --git a/core/src/test/resources/WEB-INF/config.properties b/core/src/test/resources/WEB-INF/config.properties index 4bb53114f98..e59ce755660 100644 --- a/core/src/test/resources/WEB-INF/config.properties +++ b/core/src/test/resources/WEB-INF/config.properties @@ -20,5 +20,7 @@ es.index.checker.interval=0/5 * * * * ? thesaurus.cache.maxsize=400000 +api.params.maxUploadSize=100000000 + language.default=eng language.forceDefault=false From 263d091341079bfb4b06124a32e8fe214b77dcfd Mon Sep 17 00:00:00 2001 From: tylerjmchugh Date: Wed, 18 Dec 2024 09:46:35 -0500 Subject: [PATCH 6/6] Remove content-length check as it cannot be trusted --- .../records/attachments/AbstractStore.java | 25 +++---------------- 1 file changed, 3 insertions(+), 22 deletions(-) diff --git a/core/src/main/java/org/fao/geonet/api/records/attachments/AbstractStore.java b/core/src/main/java/org/fao/geonet/api/records/attachments/AbstractStore.java index 58e7a009544..386a376f9af 100644 --- a/core/src/main/java/org/fao/geonet/api/records/attachments/AbstractStore.java +++ b/core/src/main/java/org/fao/geonet/api/records/attachments/AbstractStore.java @@ -185,23 +185,6 @@ protected String getFilenameFromHeader(final URL fileUrl) { } } - protected long getContentLengthFromHeader(final URL fileUrl) { - HttpURLConnection connection = null; - try { - connection = (HttpURLConnection) fileUrl.openConnection(); - connection.setRequestMethod("HEAD"); - connection.connect(); - return connection.getContentLengthLong(); - } catch (Exception e) { - log.error("Error retrieving resource content length from header", e); - return -1; - } finally { - if (connection != null) { - connection.disconnect(); - } - } - } - protected String getFilenameFromUrl(final URL fileUrl) { String fileName = FilenameUtils.getName(fileUrl.getPath()); if (fileName.contains("?")) { @@ -253,15 +236,13 @@ public final MetadataResource putResource(ServiceContext context, String metadat filename = getFilenameFromUrl(fileUrl); } try (InputStream is = fileUrl.openStream()) { - long contentLength = getContentLengthFromHeader(fileUrl); - long availableBytes = is.available(); - long fileSize = availableBytes > contentLength ? availableBytes : contentLength; - if (fileSize > maxUploadSize) { + int availableBytes = is.available(); + if (availableBytes > maxUploadSize) { throw new GeonetMaxUploadSizeExceededException("uploadedResourceSizeExceededException") .withMessageKey("exception.maxUploadSizeExceeded", new String[]{FileUtil.humanizeFileSize(maxUploadSize)}) .withDescriptionKey("exception.maxUploadSizeExceeded.description", - new String[]{FileUtil.humanizeFileSize(fileSize), + new String[]{FileUtil.humanizeFileSize(availableBytes), FileUtil.humanizeFileSize(maxUploadSize)}); } return putResource(context, metadataUuid, filename, is, null, visibility, approved);