Skip to content

Commit 8b2a00d

Browse files
Enforce upload size limit on putResourceFromUrl api (#8562)
* Check content-length of resource at url * Remove unnecessary throw * Fallback to InputStream.available() if content length is not found or smaller * Fallback maxUploadSize for unit tests * Fix default value always used * Remove content-length check as it cannot be trusted * Download file to a temp file to check size * Add back the content-length check as a preliminary check * Stream file instead of using temp file * Rollback for JCloud * Fix rollback for JCloud * Fix abstract store and implement jcloud rollback * Fix typo and remove unused import * Fix unit tests * Fix tests (mock response code) * Fix unit test, refactor, and add docs * Fix exception handling and use bounded input stream instead of custom input stream * Improvements * Add documentation * Rename exception * Remove unneeded changes * Update docs * Fix comment Co-authored-by: Ian <[email protected]> * Update exception handling * Update jcloud exception handling and comments * Add file header * Add comment * Fix whitespace --------- Co-authored-by: Ian <[email protected]>
1 parent d46fa20 commit 8b2a00d

File tree

12 files changed

+212
-40
lines changed

12 files changed

+212
-40
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
//=============================================================================
2+
//=== Copyright (C) 2001-2025 Food and Agriculture Organization of the
3+
//=== United Nations (FAO-UN), United Nations World Food Programme (WFP)
4+
//=== and United Nations Environment Programme (UNEP)
5+
//===
6+
//=== This library is free software; you can redistribute it and/or
7+
//=== modify it under the terms of the GNU Lesser General Public
8+
//=== License as published by the Free Software Foundation; either
9+
//=== version 2.1 of the License, or (at your option) any later version.
10+
//===
11+
//=== This library is distributed in the hope that it will be useful,
12+
//=== but WITHOUT ANY WARRANTY; without even the implied warranty of
13+
//=== MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
14+
//=== Lesser General Public License for more details.
15+
//===
16+
//=== You should have received a copy of the GNU Lesser General Public
17+
//=== License along with this library; if not, write to the Free Software
18+
//=== Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
19+
//===
20+
//=== Contact: Jeroen Ticheler - FAO - Viale delle Terme di Caracalla 2,
21+
//=== Rome - Italy. email: [email protected]
22+
//==============================================================================
23+
24+
package org.fao.geonet.api.exception;
25+
26+
import org.springframework.web.multipart.MaxUploadSizeExceededException;
27+
28+
/**
29+
* Custom exception to be thrown when the size of a remote file to be uploaded to the store exceeds the maximum upload size.
30+
*/
31+
public class InputStreamLimitExceededException extends MaxUploadSizeExceededException {
32+
private final long remoteFileSize;
33+
34+
/**
35+
* Create a new InputStreamLimitExceededException with an unknown remote file size.
36+
*
37+
* @param maxUploadSize the maximum upload size allowed
38+
*/
39+
public InputStreamLimitExceededException(long maxUploadSize) {
40+
this(maxUploadSize, -1L);
41+
}
42+
43+
/**
44+
* Create a new InputStreamLimitExceededException with a known remote file size.
45+
*
46+
* @param maxUploadSize the maximum upload size allowed
47+
* @param remoteFileSize the size of the remote file
48+
*/
49+
public InputStreamLimitExceededException(long maxUploadSize, long remoteFileSize) {
50+
super(maxUploadSize);
51+
this.remoteFileSize = remoteFileSize;
52+
}
53+
54+
/**
55+
* Get the size of the remote file.
56+
*
57+
* @return the size of the remote file or -1 if the size is unknown
58+
*/
59+
public long getRemoteFileSize() {
60+
return this.remoteFileSize;
61+
}
62+
}

core/src/main/java/org/fao/geonet/api/records/attachments/AbstractStore.java

+38-26
Original file line numberDiff line numberDiff line change
@@ -28,16 +28,21 @@
2828
import org.apache.commons.io.FilenameUtils;
2929
import org.fao.geonet.ApplicationContextHolder;
3030
import org.fao.geonet.api.exception.NotAllowedException;
31+
import org.fao.geonet.api.exception.InputStreamLimitExceededException;
3132
import org.fao.geonet.api.exception.ResourceNotFoundException;
3233
import org.fao.geonet.domain.AbstractMetadata;
3334
import org.fao.geonet.domain.MetadataResource;
3435
import org.fao.geonet.domain.MetadataResourceVisibility;
3536
import org.fao.geonet.kernel.AccessManager;
3637
import org.fao.geonet.kernel.datamanager.IMetadataUtils;
3738
import org.fao.geonet.repository.MetadataRepository;
39+
import org.fao.geonet.util.LimitedInputStream;
3840
import org.slf4j.Logger;
3941
import org.slf4j.LoggerFactory;
42+
import org.springframework.beans.factory.annotation.Value;
4043
import org.springframework.context.ApplicationContext;
44+
import org.springframework.http.ContentDisposition;
45+
import org.springframework.http.HttpHeaders;
4146
import org.springframework.web.multipart.MultipartFile;
4247

4348
import java.io.BufferedInputStream;
@@ -59,6 +64,9 @@ public abstract class AbstractStore implements Store {
5964
protected static final String RESOURCE_MANAGEMENT_EXTERNAL_PROPERTIES_ESCAPED_SEPARATOR = "\\:";
6065
private static final Logger log = LoggerFactory.getLogger(AbstractStore.class);
6166

67+
@Value("${api.params.maxUploadSize}")
68+
protected int maxUploadSize;
69+
6270
@Override
6371
public final List<MetadataResource> getResources(final ServiceContext context, final String metadataUuid, final Sort sort,
6472
final String filter) throws Exception {
@@ -157,29 +165,6 @@ protected int canDownload(ServiceContext context, String metadataUuid, MetadataR
157165
return metadataId;
158166
}
159167

160-
protected String getFilenameFromHeader(final URL fileUrl) throws IOException {
161-
HttpURLConnection connection = null;
162-
try {
163-
connection = (HttpURLConnection) fileUrl.openConnection();
164-
connection.setRequestMethod("HEAD");
165-
connection.connect();
166-
String contentDisposition = connection.getHeaderField("Content-Disposition");
167-
168-
if (contentDisposition != null && contentDisposition.contains("filename=")) {
169-
String filename = contentDisposition.split("filename=")[1].replace("\"", "").trim();
170-
return filename.isEmpty() ? null : filename;
171-
}
172-
return null;
173-
} catch (Exception e) {
174-
log.error("Error retrieving resource filename from header", e);
175-
return null;
176-
} finally {
177-
if (connection != null) {
178-
connection.disconnect();
179-
}
180-
}
181-
}
182-
183168
protected String getFilenameFromUrl(final URL fileUrl) {
184169
String fileName = FilenameUtils.getName(fileUrl.getPath());
185170
if (fileName.contains("?")) {
@@ -226,11 +211,38 @@ public final MetadataResource putResource(ServiceContext context, String metadat
226211
@Override
227212
public final MetadataResource putResource(ServiceContext context, String metadataUuid, URL fileUrl,
228213
MetadataResourceVisibility visibility, Boolean approved) throws Exception {
229-
String filename = getFilenameFromHeader(fileUrl);
230-
if (filename == null) {
214+
215+
// Open a connection to the URL
216+
HttpURLConnection connection = (HttpURLConnection) fileUrl.openConnection();
217+
connection.setInstanceFollowRedirects(true);
218+
connection.setRequestMethod("GET");
219+
220+
// Check if the response code is OK
221+
int responseCode = connection.getResponseCode();
222+
if (responseCode != HttpURLConnection.HTTP_OK) {
223+
throw new IOException("Unexpected response code: " + responseCode);
224+
}
225+
226+
// Extract filename from Content-Disposition header if present otherwise use the filename from the URL
227+
String contentDisposition = connection.getHeaderField(HttpHeaders.CONTENT_DISPOSITION);
228+
String filename = null;
229+
if (contentDisposition != null) {
230+
filename = ContentDisposition.parse(contentDisposition).getFilename();
231+
}
232+
if (filename == null || filename.isEmpty()) {
231233
filename = getFilenameFromUrl(fileUrl);
232234
}
233-
return putResource(context, metadataUuid, filename, fileUrl.openStream(), null, visibility, approved);
235+
236+
// Check if the content length is within the allowed limit
237+
long contentLength = connection.getContentLengthLong();
238+
if (contentLength > maxUploadSize) {
239+
throw new InputStreamLimitExceededException(maxUploadSize, contentLength);
240+
}
241+
242+
// Upload the resource while ensuring the input stream does not exceed the maximum allowed size.
243+
try (LimitedInputStream is = new LimitedInputStream(connection.getInputStream(), maxUploadSize)) {
244+
return putResource(context, metadataUuid, filename, is, null, visibility, approved);
245+
}
234246
}
235247

236248
@Override

core/src/main/java/org/fao/geonet/api/records/attachments/FilesystemStore.java

+7-1
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
package org.fao.geonet.api.records.attachments;
2727

2828
import jeeves.server.context.ServiceContext;
29+
import org.fao.geonet.api.exception.InputStreamLimitExceededException;
2930
import org.fao.geonet.api.exception.ResourceAlreadyExistException;
3031
import org.fao.geonet.api.exception.ResourceNotFoundException;
3132
import org.fao.geonet.constants.Geonet;
@@ -202,7 +203,12 @@ public MetadataResource putResource(final ServiceContext context, final String m
202203
int metadataId = canEdit(context, metadataUuid, approved);
203204
checkResourceId(filename);
204205
Path filePath = getPath(context, metadataId, visibility, filename, approved);
205-
Files.copy(is, filePath, StandardCopyOption.REPLACE_EXISTING);
206+
try {
207+
Files.copy(is, filePath, StandardCopyOption.REPLACE_EXISTING);
208+
} catch (InputStreamLimitExceededException e) {
209+
Files.deleteIfExists(filePath);
210+
throw e;
211+
}
206212
if (changeDate != null) {
207213
IO.touch(filePath, FileTime.from(changeDate.getTime(), TimeUnit.MILLISECONDS));
208214
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
//=============================================================================
2+
//=== Copyright (C) 2001-2025 Food and Agriculture Organization of the
3+
//=== United Nations (FAO-UN), United Nations World Food Programme (WFP)
4+
//=== and United Nations Environment Programme (UNEP)
5+
//===
6+
//=== This library is free software; you can redistribute it and/or
7+
//=== modify it under the terms of the GNU Lesser General Public
8+
//=== License as published by the Free Software Foundation; either
9+
//=== version 2.1 of the License, or (at your option) any later version.
10+
//===
11+
//=== This library is distributed in the hope that it will be useful,
12+
//=== but WITHOUT ANY WARRANTY; without even the implied warranty of
13+
//=== MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
14+
//=== Lesser General Public License for more details.
15+
//===
16+
//=== You should have received a copy of the GNU Lesser General Public
17+
//=== License along with this library; if not, write to the Free Software
18+
//=== Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
19+
//===
20+
//=== Contact: Jeroen Ticheler - FAO - Viale delle Terme di Caracalla 2,
21+
//=== Rome - Italy. email: [email protected]
22+
//==============================================================================
23+
24+
package org.fao.geonet.util;
25+
26+
import org.fao.geonet.api.exception.InputStreamLimitExceededException;
27+
28+
import java.io.IOException;
29+
import java.io.InputStream;
30+
31+
/**
32+
* Implementation of {@link org.apache.commons.fileupload.util.LimitedInputStream} that throws a
33+
* {@link InputStreamLimitExceededException} when the configured limit is exceeded.
34+
*/
35+
public class LimitedInputStream extends org.apache.commons.fileupload.util.LimitedInputStream {
36+
37+
38+
/**
39+
* Creates a new instance.
40+
*
41+
* @param inputStream The input stream, which shall be limited.
42+
* @param pSizeMax The limit; no more than this number of bytes
43+
* shall be returned by the source stream.
44+
*/
45+
public LimitedInputStream(InputStream inputStream, long pSizeMax) {
46+
super(inputStream, pSizeMax);
47+
}
48+
49+
@Override
50+
protected void raiseError(long pSizeMax, long pCount) throws IOException {
51+
throw new InputStreamLimitExceededException(pSizeMax);
52+
}
53+
}

core/src/test/resources/WEB-INF/config.properties

+2
Original file line numberDiff line numberDiff line change
@@ -20,5 +20,7 @@ es.index.checker.interval=0/5 * * * * ?
2020

2121
thesaurus.cache.maxsize=400000
2222

23+
api.params.maxUploadSize=100000000
24+
2325
language.default=eng
2426
language.forceDefault=false

core/src/test/resources/org/fao/geonet/api/Messages.properties

+1
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,7 @@ api.exception.unsatisfiedRequestParameter=Unsatisfied request parameter
178178
api.exception.unsatisfiedRequestParameter.description=Unsatisfied request parameter.
179179
exception.maxUploadSizeExceeded=Maximum upload size of {0} exceeded.
180180
exception.maxUploadSizeExceeded.description=The request was rejected because its size ({0}) exceeds the configured maximum ({1}).
181+
exception.maxUploadSizeExceededUnknownSize.description=The request was rejected because its size exceeds the configured maximum ({0}).
181182
exception.resourceNotFound.metadata=Metadata not found
182183
exception.resourceNotFound.metadata.description=Metadata with UUID ''{0}'' not found.
183184
exception.resourceNotFound.resource=Metadata resource ''{0}'' not found

core/src/test/resources/org/fao/geonet/api/Messages_fre.properties

+1
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,7 @@ api.exception.unsatisfiedRequestParameter=Param\u00E8tre de demande non satisfai
172172
api.exception.unsatisfiedRequestParameter.description=Param\u00E8tre de demande non satisfait.
173173
exception.maxUploadSizeExceeded=La taille maximale du t\u00E9l\u00E9chargement de {0} a \u00E9t\u00E9 exc\u00E9d\u00E9e.
174174
exception.maxUploadSizeExceeded.description=La demande a \u00E9t\u00E9 refus\u00E9e car sa taille ({0}) exc\u00E8de le maximum configur\u00E9 ({1}).
175+
exception.maxUploadSizeExceededUnknownSize.description=La demande a \u00E9t\u00E9 refus\u00E9e car sa taille exc\u00E8de le maximum configur\u00E9 ({0}).
175176
exception.resourceNotFound.metadata=Fiches introuvables
176177
exception.resourceNotFound.metadata.description=La fiche ''{0}'' est introuvable.
177178
exception.resourceNotFound.resource=Ressource ''{0}'' introuvable

datastorages/jcloud/src/main/java/org/fao/geonet/api/records/attachments/JCloudStore.java

+13-2
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030

3131
import org.apache.commons.collections.MapUtils;
3232
import org.fao.geonet.ApplicationContextHolder;
33+
import org.fao.geonet.api.exception.InputStreamLimitExceededException;
3334
import org.fao.geonet.api.exception.ResourceNotFoundException;
3435
import org.fao.geonet.constants.Geonet;
3536
import org.fao.geonet.domain.MetadataResource;
@@ -47,6 +48,7 @@
4748
import org.jclouds.blobstore.domain.*;
4849
import org.jclouds.blobstore.options.CopyOptions;
4950
import org.jclouds.blobstore.options.ListContainerOptions;
51+
import org.jclouds.http.HttpResponseException;
5052
import org.springframework.beans.factory.annotation.Autowired;
5153
import org.springframework.util.StringUtils;
5254

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

311-
// Upload the Blob in multiple chunks to supports large files.
312-
jCloudConfiguration.getClient().getBlobStore().putBlob(jCloudConfiguration.getContainerName(), blob, multipart());
313+
try {
314+
// Upload the Blob in multiple chunks to supports large files.
315+
jCloudConfiguration.getClient().getBlobStore().putBlob(jCloudConfiguration.getContainerName(), blob, multipart());
316+
} catch (HttpResponseException e) {
317+
// This is special logic for the jcloud store as the InputStreamLimitExceededException gets wrapped in a HttpResponseException
318+
Throwable cause = e.getCause();
319+
if (cause instanceof InputStreamLimitExceededException) {
320+
throw (InputStreamLimitExceededException) cause;
321+
}
322+
throw e;
323+
}
313324
Blob blobResults = jCloudConfiguration.getClient().getBlobStore().getBlob(jCloudConfiguration.getContainerName(), key);
314325

315326
return createResourceDescription(context, metadataUuid, visibility, filename, blobResults.getMetadata(), metadataId, approved);

services/src/main/java/org/fao/geonet/api/GlobalExceptionController.java

+26-8
Original file line numberDiff line numberDiff line change
@@ -156,15 +156,33 @@ public Object securityHandler(final HttpServletRequest request, final Exception
156156
})
157157
public ApiError maxFileExceededHandler(final Exception exception, final HttpServletRequest request) {
158158
Exception ex;
159-
long contentLength = request.getContentLengthLong();
160-
// As MaxUploadSizeExceededException is a spring exception, we need to convert it to a localized exception so that it can be translated.
159+
// Convert exception to a localized exception so that it can be translated.
161160
if (exception instanceof MaxUploadSizeExceededException) {
162-
ex = new GeonetMaxUploadSizeExceededException("uploadedResourceSizeExceededException", exception)
163-
.withMessageKey("exception.maxUploadSizeExceeded",
164-
new String[]{FileUtil.humanizeFileSize(((MaxUploadSizeExceededException) exception).getMaxUploadSize())})
165-
.withDescriptionKey("exception.maxUploadSizeExceeded.description",
166-
new String[]{FileUtil.humanizeFileSize(contentLength),
167-
FileUtil.humanizeFileSize(((MaxUploadSizeExceededException) exception).getMaxUploadSize())});
161+
long maxUploadSize = ((MaxUploadSizeExceededException) exception).getMaxUploadSize();
162+
long contentLength = exception instanceof InputStreamLimitExceededException ?
163+
((InputStreamLimitExceededException) exception).getRemoteFileSize() :
164+
request.getContentLengthLong();
165+
166+
// This can occur if the content length header is present on a resource but does not reflect the actual file size.
167+
// This could indicate an attempt to bypass the maximum upload size.
168+
if (contentLength > 0 && contentLength < maxUploadSize) {
169+
Log.warning(Geonet.RESOURCES, "Request content length is less than the maximum upload size but still caused an exception.");
170+
}
171+
172+
if (contentLength > maxUploadSize) {
173+
ex = new GeonetMaxUploadSizeExceededException("uploadedResourceSizeExceededException", exception)
174+
.withMessageKey("exception.maxUploadSizeExceeded",
175+
new String[]{FileUtil.humanizeFileSize(maxUploadSize)})
176+
.withDescriptionKey("exception.maxUploadSizeExceeded.description",
177+
new String[]{FileUtil.humanizeFileSize(contentLength),
178+
FileUtil.humanizeFileSize(maxUploadSize)});
179+
} else {
180+
ex = new GeonetMaxUploadSizeExceededException("uploadedResourceSizeExceededException", exception)
181+
.withMessageKey("exception.maxUploadSizeExceeded",
182+
new String[]{FileUtil.humanizeFileSize(maxUploadSize)})
183+
.withDescriptionKey("exception.maxUploadSizeExceededUnknownSize.description",
184+
new String[]{FileUtil.humanizeFileSize(maxUploadSize)});
185+
}
168186
} else {
169187
ex = exception;
170188
}

services/src/test/java/org/fao/geonet/api/records/attachments/AbstractStoreTest.java

+7-3
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,8 @@
3939
import org.springframework.web.multipart.MultipartFile;
4040

4141
import java.io.IOException;
42+
import java.net.HttpURLConnection;
4243
import java.net.URL;
43-
import java.net.URLConnection;
4444
import java.net.URLStreamHandler;
4545
import java.nio.file.Files;
4646
import java.nio.file.Path;
@@ -69,15 +69,19 @@ public static URL getMockUrl(final String filename,
6969
final Path file = Paths.get(resources, filename);
7070

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

7474
Mockito.when(mockConnection.getInputStream()).thenReturn(
7575
Files.newInputStream(file)
7676
);
7777

78+
Mockito.when(mockConnection.getResponseCode()).thenReturn(HttpURLConnection.HTTP_OK);
79+
80+
Mockito.when(mockConnection.getContentLengthLong()).thenReturn(-1L);
81+
7882
final URLStreamHandler handler = new URLStreamHandler() {
7983
@Override
80-
protected URLConnection openConnection(final URL arg0) {
84+
protected HttpURLConnection openConnection(final URL arg0) {
8185
return mockConnection;
8286
}
8387
};

web/src/main/webapp/WEB-INF/classes/org/fao/geonet/api/Messages.properties

+1
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,7 @@ api.exception.unsatisfiedRequestParameter=Unsatisfied request parameter
183183
api.exception.unsatisfiedRequestParameter.description=Unsatisfied request parameter.
184184
exception.maxUploadSizeExceeded=Maximum upload size of {0} exceeded.
185185
exception.maxUploadSizeExceeded.description=The request was rejected because its size ({0}) exceeds the configured maximum ({1}).
186+
exception.maxUploadSizeExceededUnknownSize.description=The request was rejected because its size exceeds the configured maximum ({0}).
186187
exception.resourceNotFound.metadata=Metadata not found
187188
exception.resourceNotFound.metadata.description=Metadata with UUID ''{0}'' not found.
188189
exception.resourceNotFound.resource=Metadata resource ''{0}'' not found

0 commit comments

Comments
 (0)