Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Backport 4.2.x] Enforce upload size limit on putResourceFromUrl api #8601

Merged
merged 28 commits into from
Jan 14, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
e726edf
Check content-length of resource at url
tylerjmchugh Dec 13, 2024
e600d52
Remove unnecessary throw
tylerjmchugh Dec 13, 2024
6359d88
Fallback to InputStream.available() if content length is not found or…
tylerjmchugh Dec 17, 2024
7eb477c
Fallback maxUploadSize for unit tests
tylerjmchugh Dec 17, 2024
6fbc933
Fix default value always used
tylerjmchugh Dec 18, 2024
19a3535
Remove content-length check as it cannot be trusted
tylerjmchugh Dec 18, 2024
358d2dd
Download file to a temp file to check size
tylerjmchugh Dec 20, 2024
369bf7e
Add back the content-length check as a preliminary check
tylerjmchugh Dec 30, 2024
88fee39
Stream file instead of using temp file
tylerjmchugh Dec 30, 2024
3490b3f
Rollback for JCloud
tylerjmchugh Dec 30, 2024
09787de
Fix rollback for JCloud
tylerjmchugh Dec 30, 2024
0c0abed
Fix abstract store and implement jcloud rollback
tylerjmchugh Jan 2, 2025
f052cdd
Fix typo and remove unused import
tylerjmchugh Jan 2, 2025
9745308
Fix unit tests
tylerjmchugh Jan 2, 2025
ee19d04
Fix tests (mock response code)
tylerjmchugh Jan 2, 2025
8ceab27
Fix unit test, refactor, and add docs
tylerjmchugh Jan 2, 2025
99f7259
Fix exception handling and use bounded input stream instead of custom…
tylerjmchugh Jan 3, 2025
4a926d5
Improvements
tylerjmchugh Jan 6, 2025
ba4e77d
Add documentation
tylerjmchugh Jan 6, 2025
4aa0f76
Rename exception
tylerjmchugh Jan 6, 2025
54ed3cf
Remove unneeded changes
tylerjmchugh Jan 6, 2025
658cb47
Update docs
tylerjmchugh Jan 6, 2025
f2caf1c
Fix comment
tylerjmchugh Jan 7, 2025
7d70fbf
Update exception handling
tylerjmchugh Jan 9, 2025
4b6b35f
Update jcloud exception handling and comments
tylerjmchugh Jan 10, 2025
bd8670c
Add file header
tylerjmchugh Jan 13, 2025
6b3462e
Add comment
tylerjmchugh Jan 13, 2025
2a0be80
Fix whitespace
tylerjmchugh Jan 13, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
//=============================================================================
//=== Copyright (C) 2001-2025 Food and Agriculture Organization of the
//=== United Nations (FAO-UN), United Nations World Food Programme (WFP)
//=== and United Nations Environment Programme (UNEP)
//===
//=== This library is free software; you can redistribute it and/or
//=== modify it under the terms of the GNU Lesser General Public
//=== License as published by the Free Software Foundation; either
//=== version 2.1 of the License, or (at your option) any later version.
//===
//=== This library is distributed in the hope that it will be useful,
//=== but WITHOUT ANY WARRANTY; without even the implied warranty of
//=== MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
//=== Lesser General Public License for more details.
//===
//=== You should have received a copy of the GNU Lesser General Public
//=== License along with this library; if not, write to the Free Software
//=== Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
//===
//=== Contact: Jeroen Ticheler - FAO - Viale delle Terme di Caracalla 2,
//=== Rome - Italy. email: geonetwork@osgeo.org
//==============================================================================

package org.fao.geonet.api.exception;

import org.springframework.web.multipart.MaxUploadSizeExceededException;

/**
* Custom exception to be thrown when the size of a remote file to be uploaded to the store exceeds the maximum upload size.
*/
public class InputStreamLimitExceededException extends MaxUploadSizeExceededException {
private final long remoteFileSize;

/**
* Create a new InputStreamLimitExceededException with an unknown remote file size.
*
* @param maxUploadSize the maximum upload size allowed
*/
public InputStreamLimitExceededException(long maxUploadSize) {
this(maxUploadSize, -1L);
}

/**
* Create a new InputStreamLimitExceededException with a known remote file size.
*
* @param maxUploadSize the maximum upload size allowed
* @param remoteFileSize the size of the remote file
*/
public InputStreamLimitExceededException(long maxUploadSize, long remoteFileSize) {
super(maxUploadSize);
this.remoteFileSize = remoteFileSize;
}

/**
* Get the size of the remote file.
*
* @return the size of the remote file or -1 if the size is unknown
*/
public long getRemoteFileSize() {
return this.remoteFileSize;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,16 +28,21 @@
import org.apache.commons.io.FilenameUtils;
import org.fao.geonet.ApplicationContextHolder;
import org.fao.geonet.api.exception.NotAllowedException;
import org.fao.geonet.api.exception.InputStreamLimitExceededException;
import org.fao.geonet.api.exception.ResourceNotFoundException;
import org.fao.geonet.domain.AbstractMetadata;
import org.fao.geonet.domain.MetadataResource;
import org.fao.geonet.domain.MetadataResourceVisibility;
import org.fao.geonet.kernel.AccessManager;
import org.fao.geonet.kernel.datamanager.IMetadataUtils;
import org.fao.geonet.repository.MetadataRepository;
import org.fao.geonet.util.LimitedInputStream;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.context.ApplicationContext;
import org.springframework.http.ContentDisposition;
import org.springframework.http.HttpHeaders;
import org.springframework.web.multipart.MultipartFile;

import java.io.BufferedInputStream;
Expand All @@ -59,6 +64,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}")
protected int maxUploadSize;

@Override
public final List<MetadataResource> getResources(final ServiceContext context, final String metadataUuid, final Sort sort,
final String filter) throws Exception {
Expand Down Expand Up @@ -157,29 +165,6 @@ protected int canDownload(ServiceContext context, String metadataUuid, MetadataR
return metadataId;
}

protected String getFilenameFromHeader(final URL fileUrl) throws IOException {
HttpURLConnection connection = null;
try {
connection = (HttpURLConnection) fileUrl.openConnection();
connection.setRequestMethod("HEAD");
connection.connect();
String contentDisposition = connection.getHeaderField("Content-Disposition");

if (contentDisposition != null && contentDisposition.contains("filename=")) {
String filename = contentDisposition.split("filename=")[1].replace("\"", "").trim();
return filename.isEmpty() ? null : filename;
}
return null;
} catch (Exception e) {
log.error("Error retrieving resource filename from header", e);
return null;
} finally {
if (connection != null) {
connection.disconnect();
}
}
}

protected String getFilenameFromUrl(final URL fileUrl) {
String fileName = FilenameUtils.getName(fileUrl.getPath());
if (fileName.contains("?")) {
Expand Down Expand Up @@ -226,11 +211,38 @@ 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 {
String filename = getFilenameFromHeader(fileUrl);
if (filename == null) {

// Open a connection to the URL
HttpURLConnection connection = (HttpURLConnection) fileUrl.openConnection();
connection.setInstanceFollowRedirects(true);
connection.setRequestMethod("GET");

// Check if the response code is OK
int responseCode = connection.getResponseCode();
if (responseCode != HttpURLConnection.HTTP_OK) {
throw new IOException("Unexpected response code: " + responseCode);
}

// Extract filename from Content-Disposition header if present otherwise use the filename from the URL
String contentDisposition = connection.getHeaderField(HttpHeaders.CONTENT_DISPOSITION);
String filename = null;
if (contentDisposition != null) {
filename = ContentDisposition.parse(contentDisposition).getFilename();
}
if (filename == null || filename.isEmpty()) {
filename = getFilenameFromUrl(fileUrl);
}
return putResource(context, metadataUuid, filename, fileUrl.openStream(), null, visibility, approved);

// Check if the content length is within the allowed limit
long contentLength = connection.getContentLengthLong();
if (contentLength > maxUploadSize) {
throw new InputStreamLimitExceededException(maxUploadSize, contentLength);
}

// Upload the resource while ensuring the input stream does not exceed the maximum allowed size.
try (LimitedInputStream is = new LimitedInputStream(connection.getInputStream(), maxUploadSize)) {
return putResource(context, metadataUuid, filename, is, null, visibility, approved);
}
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
package org.fao.geonet.api.records.attachments;

import jeeves.server.context.ServiceContext;
import org.fao.geonet.api.exception.InputStreamLimitExceededException;
import org.fao.geonet.api.exception.ResourceAlreadyExistException;
import org.fao.geonet.api.exception.ResourceNotFoundException;
import org.fao.geonet.constants.Geonet;
Expand Down Expand Up @@ -202,7 +203,12 @@ public MetadataResource putResource(final ServiceContext context, final String m
int metadataId = canEdit(context, metadataUuid, approved);
checkResourceId(filename);
Path filePath = getPath(context, metadataId, visibility, filename, approved);
Files.copy(is, filePath, StandardCopyOption.REPLACE_EXISTING);
try {
Files.copy(is, filePath, StandardCopyOption.REPLACE_EXISTING);
} catch (InputStreamLimitExceededException e) {
Files.deleteIfExists(filePath);
throw e;
}
if (changeDate != null) {
IO.touch(filePath, FileTime.from(changeDate.getTime(), TimeUnit.MILLISECONDS));
}
Expand Down
53 changes: 53 additions & 0 deletions core/src/main/java/org/fao/geonet/util/LimitedInputStream.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
//=============================================================================
//=== Copyright (C) 2001-2025 Food and Agriculture Organization of the
//=== United Nations (FAO-UN), United Nations World Food Programme (WFP)
//=== and United Nations Environment Programme (UNEP)
//===
//=== This library is free software; you can redistribute it and/or
//=== modify it under the terms of the GNU Lesser General Public
//=== License as published by the Free Software Foundation; either
//=== version 2.1 of the License, or (at your option) any later version.
//===
//=== This library is distributed in the hope that it will be useful,
//=== but WITHOUT ANY WARRANTY; without even the implied warranty of
//=== MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
//=== Lesser General Public License for more details.
//===
//=== You should have received a copy of the GNU Lesser General Public
//=== License along with this library; if not, write to the Free Software
//=== Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
//===
//=== Contact: Jeroen Ticheler - FAO - Viale delle Terme di Caracalla 2,
//=== Rome - Italy. email: geonetwork@osgeo.org
//==============================================================================

package org.fao.geonet.util;

import org.fao.geonet.api.exception.InputStreamLimitExceededException;

import java.io.IOException;
import java.io.InputStream;

/**
* Implementation of {@link org.apache.commons.fileupload.util.LimitedInputStream} that throws a
* {@link InputStreamLimitExceededException} when the configured limit is exceeded.
*/
public class LimitedInputStream extends org.apache.commons.fileupload.util.LimitedInputStream {


/**
* Creates a new instance.
*
* @param inputStream The input stream, which shall be limited.
* @param pSizeMax The limit; no more than this number of bytes
* shall be returned by the source stream.
*/
public LimitedInputStream(InputStream inputStream, long pSizeMax) {
super(inputStream, pSizeMax);
}

@Override
protected void raiseError(long pSizeMax, long pCount) throws IOException {
throw new InputStreamLimitExceededException(pSizeMax);
}
}
2 changes: 2 additions & 0 deletions core/src/test/resources/WEB-INF/config.properties
Original file line number Diff line number Diff line change
Expand Up @@ -20,5 +20,7 @@ es.index.checker.interval=0/5 * * * * ?

thesaurus.cache.maxsize=400000

api.params.maxUploadSize=100000000

language.default=eng
language.forceDefault=false
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,7 @@ api.exception.unsatisfiedRequestParameter=Unsatisfied request parameter
api.exception.unsatisfiedRequestParameter.description=Unsatisfied request parameter.
exception.maxUploadSizeExceeded=Maximum upload size of {0} exceeded.
exception.maxUploadSizeExceeded.description=The request was rejected because its size ({0}) exceeds the configured maximum ({1}).
exception.maxUploadSizeExceededUnknownSize.description=The request was rejected because its size exceeds the configured maximum ({0}).
exception.resourceNotFound.metadata=Metadata not found
exception.resourceNotFound.metadata.description=Metadata with UUID ''{0}'' not found.
exception.resourceNotFound.resource=Metadata resource ''{0}'' not found
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ api.exception.unsatisfiedRequestParameter=Param\u00E8tre de demande non satisfai
api.exception.unsatisfiedRequestParameter.description=Param\u00E8tre de demande non satisfait.
exception.maxUploadSizeExceeded=La taille maximale du t\u00E9l\u00E9chargement de {0} a \u00E9t\u00E9 exc\u00E9d\u00E9e.
exception.maxUploadSizeExceeded.description=La demande a \u00E9t\u00E9 refus\u00E9e car sa taille ({0}) exc\u00E8de le maximum configur\u00E9 ({1}).
exception.maxUploadSizeExceededUnknownSize.description=La demande a \u00E9t\u00E9 refus\u00E9e car sa taille exc\u00E8de le maximum configur\u00E9 ({0}).
exception.resourceNotFound.metadata=Fiches introuvables
exception.resourceNotFound.metadata.description=La fiche ''{0}'' est introuvable.
exception.resourceNotFound.resource=Ressource ''{0}'' introuvable
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@

import org.apache.commons.collections.MapUtils;
import org.fao.geonet.ApplicationContextHolder;
import org.fao.geonet.api.exception.InputStreamLimitExceededException;
import org.fao.geonet.api.exception.ResourceNotFoundException;
import org.fao.geonet.constants.Geonet;
import org.fao.geonet.domain.MetadataResource;
Expand All @@ -47,6 +48,7 @@
import org.jclouds.blobstore.domain.*;
import org.jclouds.blobstore.options.CopyOptions;
import org.jclouds.blobstore.options.ListContainerOptions;
import org.jclouds.http.HttpResponseException;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.util.StringUtils;

Expand Down Expand Up @@ -308,8 +310,17 @@ protected MetadataResource putResource(final ServiceContext context, final Strin
Log.info(Geonet.RESOURCES,
String.format("Put(2) blob '%s' with version label '%s'.", key, properties.get(jCloudConfiguration.getExternalResourceManagementVersionPropertyName())));

// Upload the Blob in multiple chunks to supports large files.
jCloudConfiguration.getClient().getBlobStore().putBlob(jCloudConfiguration.getContainerName(), blob, multipart());
try {
// Upload the Blob in multiple chunks to supports large files.
jCloudConfiguration.getClient().getBlobStore().putBlob(jCloudConfiguration.getContainerName(), blob, multipart());
} catch (HttpResponseException e) {
// This is special logic for the jcloud store as the InputStreamLimitExceededException gets wrapped in a HttpResponseException
Throwable cause = e.getCause();
if (cause instanceof InputStreamLimitExceededException) {
throw (InputStreamLimitExceededException) cause;
}
throw e;
}
Blob blobResults = jCloudConfiguration.getClient().getBlobStore().getBlob(jCloudConfiguration.getContainerName(), key);

return createResourceDescription(context, metadataUuid, visibility, filename, blobResults.getMetadata(), metadataId, approved);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -156,15 +156,33 @@ public Object securityHandler(final HttpServletRequest request, final Exception
})
public ApiError maxFileExceededHandler(final Exception exception, final HttpServletRequest request) {
Exception ex;
long contentLength = request.getContentLengthLong();
// As MaxUploadSizeExceededException is a spring exception, we need to convert it to a localized exception so that it can be translated.
// Convert exception to a localized exception so that it can be translated.
if (exception instanceof MaxUploadSizeExceededException) {
ex = new GeonetMaxUploadSizeExceededException("uploadedResourceSizeExceededException", exception)
.withMessageKey("exception.maxUploadSizeExceeded",
new String[]{FileUtil.humanizeFileSize(((MaxUploadSizeExceededException) exception).getMaxUploadSize())})
.withDescriptionKey("exception.maxUploadSizeExceeded.description",
new String[]{FileUtil.humanizeFileSize(contentLength),
FileUtil.humanizeFileSize(((MaxUploadSizeExceededException) exception).getMaxUploadSize())});
long maxUploadSize = ((MaxUploadSizeExceededException) exception).getMaxUploadSize();
long contentLength = exception instanceof InputStreamLimitExceededException ?
((InputStreamLimitExceededException) exception).getRemoteFileSize() :
request.getContentLengthLong();

// This can occur if the content length header is present on a resource but does not reflect the actual file size.
// This could indicate an attempt to bypass the maximum upload size.
if (contentLength > 0 && contentLength < maxUploadSize) {
Log.warning(Geonet.RESOURCES, "Request content length is less than the maximum upload size but still caused an exception.");
}

if (contentLength > maxUploadSize) {
ex = new GeonetMaxUploadSizeExceededException("uploadedResourceSizeExceededException", exception)
.withMessageKey("exception.maxUploadSizeExceeded",
new String[]{FileUtil.humanizeFileSize(maxUploadSize)})
.withDescriptionKey("exception.maxUploadSizeExceeded.description",
new String[]{FileUtil.humanizeFileSize(contentLength),
FileUtil.humanizeFileSize(maxUploadSize)});
} else {
ex = new GeonetMaxUploadSizeExceededException("uploadedResourceSizeExceededException", exception)
.withMessageKey("exception.maxUploadSizeExceeded",
new String[]{FileUtil.humanizeFileSize(maxUploadSize)})
.withDescriptionKey("exception.maxUploadSizeExceededUnknownSize.description",
new String[]{FileUtil.humanizeFileSize(maxUploadSize)});
}
} else {
ex = exception;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@
import org.springframework.web.multipart.MultipartFile;

import java.io.IOException;
import java.net.HttpURLConnection;
import java.net.URL;
import java.net.URLConnection;
import java.net.URLStreamHandler;
import java.nio.file.Files;
import java.nio.file.Path;
Expand Down Expand Up @@ -69,15 +69,19 @@ public static URL getMockUrl(final String filename,
final Path file = Paths.get(resources, filename);

assertTrue("Mock file " + filename + " not found", Files.exists(file));
final URLConnection mockConnection = Mockito.mock(URLConnection.class);
final HttpURLConnection mockConnection = Mockito.mock(HttpURLConnection.class);

Mockito.when(mockConnection.getInputStream()).thenReturn(
Files.newInputStream(file)
);

Mockito.when(mockConnection.getResponseCode()).thenReturn(HttpURLConnection.HTTP_OK);

Mockito.when(mockConnection.getContentLengthLong()).thenReturn(-1L);

final URLStreamHandler handler = new URLStreamHandler() {
@Override
protected URLConnection openConnection(final URL arg0) {
protected HttpURLConnection openConnection(final URL arg0) {
return mockConnection;
}
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,7 @@ api.exception.unsatisfiedRequestParameter=Unsatisfied request parameter
api.exception.unsatisfiedRequestParameter.description=Unsatisfied request parameter.
exception.maxUploadSizeExceeded=Maximum upload size of {0} exceeded.
exception.maxUploadSizeExceeded.description=The request was rejected because its size ({0}) exceeds the configured maximum ({1}).
exception.maxUploadSizeExceededUnknownSize.description=The request was rejected because its size exceeds the configured maximum ({0}).
exception.resourceNotFound.metadata=Metadata not found
exception.resourceNotFound.metadata.description=Metadata with UUID ''{0}'' not found.
exception.resourceNotFound.resource=Metadata resource ''{0}'' not found
Expand Down
Loading
Loading