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

Enforce upload size limit on putResourceFromUrl api #8562

Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
d4bcff3
Check content-length of resource at url
tylerjmchugh Dec 13, 2024
a0e2fdc
Remove unnecessary throw
tylerjmchugh Dec 13, 2024
b5b6dbf
Fallback to InputStream.available() if content length is not found or…
tylerjmchugh Dec 17, 2024
2b54b4d
Fallback maxUploadSize for unit tests
tylerjmchugh Dec 17, 2024
4d6a9e7
Fix default value always used
tylerjmchugh Dec 18, 2024
263d091
Remove content-length check as it cannot be trusted
tylerjmchugh Dec 18, 2024
00770fa
Download file to a temp file to check size
tylerjmchugh Dec 20, 2024
f4232fa
Add back the content-length check as a preliminary check
tylerjmchugh Dec 30, 2024
c5193fa
Stream file instead of using temp file
tylerjmchugh Dec 30, 2024
cd7418e
Rollback for JCloud
tylerjmchugh Dec 30, 2024
e73fb98
Fix rollback for JCloud
tylerjmchugh Dec 30, 2024
c0b921b
Fix abstract store and implement jcloud rollback
tylerjmchugh Jan 2, 2025
5878548
Fix typo and remove unused import
tylerjmchugh Jan 2, 2025
6e5d0d0
Fix unit tests
tylerjmchugh Jan 2, 2025
fba9139
Fix tests (mock response code)
tylerjmchugh Jan 2, 2025
afaef1b
Fix unit test, refactor, and add docs
tylerjmchugh Jan 2, 2025
bbc9526
Fix exception handling and use bounded input stream instead of custom…
tylerjmchugh Jan 3, 2025
a6b1d0e
Improvements
tylerjmchugh Jan 6, 2025
3800bd7
Add documentation
tylerjmchugh Jan 6, 2025
9a2798f
Rename exception
tylerjmchugh Jan 6, 2025
da12ecb
Remove unneeded changes
tylerjmchugh Jan 6, 2025
0c27ae1
Update docs
tylerjmchugh Jan 6, 2025
b3d21f6
Fix comment
tylerjmchugh Jan 7, 2025
d557358
Update exception handling
tylerjmchugh Jan 9, 2025
a539228
Update jcloud exception handling and comments
tylerjmchugh Jan 10, 2025
4315340
Add file header
tylerjmchugh Jan 13, 2025
084fbf7
Add comment
tylerjmchugh Jan 13, 2025
c9778a4
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
Expand Up @@ -26,7 +26,9 @@

import jeeves.server.context.ServiceContext;
import org.apache.commons.io.FilenameUtils;
import org.apache.commons.io.input.BoundedInputStream;
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;
Expand All @@ -35,13 +37,15 @@
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.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.web.multipart.MultipartFile;

import java.io.BufferedInputStream;
import java.io.IOException;
import java.io.InputStream;
import java.net.HttpURLConnection;
import java.net.URL;
Expand All @@ -59,6 +63,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,7 +164,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();
Expand All @@ -180,6 +187,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("?")) {
Expand Down Expand Up @@ -230,7 +254,20 @@ public final MetadataResource putResource(ServiceContext context, String metadat
if (filename == null) {
filename = getFilenameFromUrl(fileUrl);
}
return putResource(context, metadataUuid, filename, fileUrl.openStream(), null, visibility, approved);

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)});
}

try (InputStream is = new LimitedInputStream(fileUrl.openStream(), maxUploadSize+1)) {
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.GeonetMaxUploadSizeExceededException;
import org.fao.geonet.api.exception.ResourceAlreadyExistException;
import org.fao.geonet.api.exception.ResourceNotFoundException;
import org.fao.geonet.constants.Geonet;
Expand All @@ -35,6 +36,8 @@
import org.fao.geonet.kernel.GeonetworkDataDirectory;
import org.fao.geonet.kernel.setting.SettingManager;
import org.fao.geonet.lib.Lib;
import org.fao.geonet.util.FileUtil;
import org.fao.geonet.util.LimitedInputStream;
import org.fao.geonet.utils.IO;
import org.fao.geonet.utils.Log;
import org.springframework.beans.factory.annotation.Autowired;
Expand Down Expand Up @@ -203,6 +206,14 @@ public MetadataResource putResource(final ServiceContext context, final String m
checkResourceId(filename);
Path filePath = getPath(context, metadataId, visibility, filename, approved);
Files.copy(is, filePath, StandardCopyOption.REPLACE_EXISTING);
if (is instanceof LimitedInputStream && ((LimitedInputStream) is).isLimitReached()) {
Files.deleteIfExists(filePath);
throw new GeonetMaxUploadSizeExceededException("uploadedResourceSizeExceededException")
.withMessageKey("exception.maxUploadSizeExceeded",
new String[]{FileUtil.humanizeFileSize(maxUploadSize)})
.withDescriptionKey("exception.maxUploadSizeExceededUnknownSize.description",
new String[]{FileUtil.humanizeFileSize(maxUploadSize)});
}
if (changeDate != null) {
IO.touch(filePath, FileTime.from(changeDate.getTime(), TimeUnit.MILLISECONDS));
}
Expand Down
102 changes: 102 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,102 @@
package org.fao.geonet.util;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add the file header.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in latest push


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

public class LimitedInputStream extends InputStream {
private final InputStream in;
private final long max;
private long pos;
private long mark;
private boolean propagateClose;

public LimitedInputStream(InputStream in, long size) {
this.pos = 0L;
this.mark = -1L;
this.propagateClose = true;
this.max = size;
this.in = in;
}

public LimitedInputStream(InputStream in) {
this(in, -1L);
}

public int read() throws IOException {
if (this.max >= 0L && this.pos >= this.max) {
return -1;
} else {
int result = this.in.read();
++this.pos;
return result;
}
}

public int read(byte[] b) throws IOException {
return this.read(b, 0, b.length);
}

public int read(byte[] b, int off, int len) throws IOException {
if (this.max >= 0L && this.pos >= this.max) {
return -1;
} else {
long maxRead = this.max >= 0L ? Math.min((long)len, this.max - this.pos) : (long)len;
int bytesRead = this.in.read(b, off, (int)maxRead);
if (bytesRead == -1) {
return -1;
} else {
this.pos += (long)bytesRead;
return bytesRead;
}
}
}

public long skip(long n) throws IOException {
long toSkip = this.max >= 0L ? Math.min(n, this.max - this.pos) : n;
long skippedBytes = this.in.skip(toSkip);
this.pos += skippedBytes;
return skippedBytes;
}

public int available() throws IOException {
return this.max >= 0L && this.pos >= this.max ? 0 : this.in.available();
}

public boolean isLimitReached() {
return this.pos >= this.max;
}

public String toString() {
return this.in.toString();
}

public void close() throws IOException {
if (this.propagateClose) {
this.in.close();
}

}

public synchronized void reset() throws IOException {
this.in.reset();
this.pos = this.mark;
}

public synchronized void mark(int readlimit) {
this.in.mark(readlimit);
this.mark = this.pos;
}

public boolean markSupported() {
return this.in.markSupported();
}

public boolean isPropagateClose() {
return this.propagateClose;
}

public void setPropagateClose(boolean propagateClose) {
this.propagateClose = propagateClose;
}
}
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.GeonetMaxUploadSizeExceededException;
import org.fao.geonet.api.exception.ResourceNotFoundException;
import org.fao.geonet.constants.Geonet;
import org.fao.geonet.domain.MetadataResource;
Expand All @@ -41,6 +42,8 @@
import org.fao.geonet.languages.IsoLanguagesMapper;
import org.fao.geonet.lib.Lib;
import org.fao.geonet.resources.JCloudConfiguration;
import org.fao.geonet.util.FileUtil;
import org.fao.geonet.util.LimitedInputStream;
import org.fao.geonet.utils.IO;
import org.fao.geonet.utils.Log;
import org.jclouds.blobstore.ContainerNotFoundException;
Expand Down Expand Up @@ -312,6 +315,14 @@ protected MetadataResource putResource(final ServiceContext context, final Strin
jCloudConfiguration.getClient().getBlobStore().putBlob(jCloudConfiguration.getContainerName(), blob, multipart());
Blob blobResults = jCloudConfiguration.getClient().getBlobStore().getBlob(jCloudConfiguration.getContainerName(), key);

if (is instanceof LimitedInputStream && ((LimitedInputStream) is).isLimitExceeded()) {
delResource(context, metadataUuid, visibility, filename, approved);
throw new GeonetMaxUploadSizeExceededException("uploadedResourceSizeExceededException")
.withMessageKey("exception.maxUploadSizeExceeded",
new String[]{FileUtil.humanizeFileSize(maxUploadSize)})
.withDescriptionKey("exception.maxUploadSizeExceededUnknownSize.description",
new String[]{FileUtil.humanizeFileSize(maxUploadSize)});
}
return createResourceDescription(context, metadataUuid, visibility, filename, blobResults.getMetadata(), metadataId, approved);
} finally {
locks.remove(key);
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
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,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
Loading