This is an automated email from the ASF dual-hosted git repository. desruisseaux pushed a commit to branch geoapi-4.0 in repository https://gitbox.apache.org/repos/asf/sis.git
commit 49a3d73017f0b54a3e47aaca4a91384df17432a7 Author: Martin Desruisseaux <[email protected]> AuthorDate: Wed Apr 13 11:53:07 2022 +0200 Fix documentation and add a size limit for safety when reading auxiliary files. --- .../apache/sis/internal/storage/PRJDataStore.java | 30 +++++++++++++++++----- .../org/apache/sis/internal/storage/Resources.java | 5 ++++ .../sis/internal/storage/Resources.properties | 1 + .../sis/internal/storage/Resources_fr.properties | 1 + .../apache/sis/internal/storage/ascii/Store.java | 3 +-- .../sis/internal/storage/io/IOUtilities.java | 2 +- .../org/apache/sis/storage/StorageConnector.java | 16 ++++++++---- .../apache/sis/storage/StorageConnectorTest.java | 2 +- 8 files changed, 45 insertions(+), 15 deletions(-) diff --git a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/PRJDataStore.java b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/PRJDataStore.java index c624b16ff4..ad49f1d563 100644 --- a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/PRJDataStore.java +++ b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/PRJDataStore.java @@ -38,8 +38,9 @@ import org.opengis.parameter.ParameterValueGroup; import org.opengis.referencing.crs.CoordinateReferenceSystem; import org.apache.sis.storage.DataOptionKey; import org.apache.sis.storage.DataStore; -import org.apache.sis.storage.DataStoreException; import org.apache.sis.storage.DataStoreProvider; +import org.apache.sis.storage.DataStoreException; +import org.apache.sis.storage.DataStoreContentException; import org.apache.sis.storage.StorageConnector; import org.apache.sis.internal.storage.io.IOUtilities; import org.apache.sis.internal.storage.wkt.StoreFormat; @@ -68,12 +69,19 @@ import org.apache.sis.util.Classes; * @module */ public abstract class PRJDataStore extends URIDataStore { + /** + * Maximal length (in bytes) of auxiliary files. This is an arbitrary restriction, we could let + * the buffer growth indefinitely instead. But a large auxiliary file is probably an error and + * we do not want an {@link OutOfMemoryError} because of that. + */ + private static final int MAXIMAL_LENGTH = 64 * 1024; + /** * The filename extension of {@code "*.prj"} files. * * @see #getComponentFiles() */ - protected static final String PRJ = "prj"; + private static final String PRJ = "prj"; /** * Character encoding in {@code *.prj} or other auxiliary files, @@ -157,12 +165,15 @@ public abstract class PRJDataStore extends URIDataStore { * * @param extension the filename extension of the auxiliary file to open. * @param encoding the encoding to use for reading the file content, or {@code null} for default. - * @return a stream opened on the specified file, or {@code null} if the file is not found. + * @return a stream opened on the specified file. * @throws NoSuchFileException if the auxiliary file has not been found (when opened from path). * @throws FileNotFoundException if the auxiliary file has not been found (when opened from URL). * @throws IOException if another error occurred while opening the stream. + * @throws DataStoreException if the auxiliary file content seems too large. */ - protected final String readAuxiliaryFile(final String extension, Charset encoding) throws IOException { + protected final String readAuxiliaryFile(final String extension, Charset encoding) + throws IOException, DataStoreException + { if (encoding == null) { encoding = Charset.defaultCharset(); } @@ -174,19 +185,22 @@ public abstract class PRJDataStore extends URIDataStore { */ final InputStream stream; Path path = getSpecifiedPath(); + final Object source; // In case an error message is produced. if (path != null) { final String base = getBaseFilename(path); - path = path.resolveSibling(base.concat(extension)); + path = path.resolveSibling(base.concat(extension)); stream = Files.newInputStream(path); + source = path; } else { final URL url = IOUtilities.toAuxiliaryURL(location, extension); if (url == null) { return null; } stream = url.openStream(); + source = url; } /* - * Reads the auxiliary file fully. + * Reads the auxiliary file fully, with an arbitrary size limit. */ try (InputStreamReader reader = new InputStreamReader(stream, encoding)) { char[] buffer = new char[1024]; @@ -194,6 +208,10 @@ public abstract class PRJDataStore extends URIDataStore { while ((count = reader.read(buffer, offset, buffer.length - offset)) >= 0) { offset += count; if (offset >= buffer.length) { + if (offset >= MAXIMAL_LENGTH) { + throw new DataStoreContentException(Resources.forLocale(listeners.getLocale()) + .getString(Resources.Keys.AuxiliaryFileTooLarge_1, IOUtilities.filename(source))); + } buffer = Arrays.copyOf(buffer, offset*2); } } diff --git a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/Resources.java b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/Resources.java index 4e65e5a169..34e207be31 100644 --- a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/Resources.java +++ b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/Resources.java @@ -64,6 +64,11 @@ public final class Resources extends IndexedResourceBundle { */ public static final short AmbiguousName_4 = 15; + /** + * Auxiliary file “{0}” seems too large. + */ + public static final short AuxiliaryFileTooLarge_1 = 71; + /** * Can not create resources based on the content of “{0}” directory. */ diff --git a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/Resources.properties b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/Resources.properties index e0118b5299..f6bbcfd450 100644 --- a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/Resources.properties +++ b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/Resources.properties @@ -20,6 +20,7 @@ # For resources shared by all modules in the Apache SIS project, see "org.apache.sis.util.resources" package. # AmbiguousName_4 = Name \u201c{3}\u201d is ambiguous because it can be understood as either \u201c{1}\u201d or \u201c{2}\u201d in the context of \u201c{0}\u201d data. +AuxiliaryFileTooLarge_1 = Auxiliary file \u201c{0}\u201d seems too large. CanNotCreateFolderStore_1 = Can not create resources based on the content of \u201c{0}\u201d directory. CanNotDeriveTypeFromFeature_1 = Can not infer the feature type resulting from \u201c{0}\u201d filtering. CanNotGetCommonMetadata_2 = Can not get metadata common to \u201c{0}\u201d files. The reason is: {1} diff --git a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/Resources_fr.properties b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/Resources_fr.properties index 8595b62a4a..0dcaf8238a 100644 --- a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/Resources_fr.properties +++ b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/Resources_fr.properties @@ -25,6 +25,7 @@ # U+00A0 NO-BREAK SPACE before : # AmbiguousName_4 = Le nom \u00ab\u202f{3}\u202f\u00bb est ambigu\u00eb car il peut \u00eatre interpr\u00e9t\u00e9 aussi bien comme \u00ab\u202f{1}\u202f\u00bb ou \u00ab\u202f{2}\u202f\u00bb dans le contexte des donn\u00e9es de \u00ab\u202f{0}\u202f\u00bb. +AuxiliaryFileTooLarge_1 = Le fichier auxiliaire \u00ab\u202f{0}\u202f\u00bb semble trop grand. CanNotCreateFolderStore_1 = Ne peut pas cr\u00e9er des ressources bas\u00e9es sur le contenu du r\u00e9pertoire \u00ab\u202f{0}\u202f\u00bb. CanNotDeriveTypeFromFeature_1 = Ne peut pas d\u00e9terminer le type de donn\u00e9es r\u00e9sultant du filtrage \u00ab\u202f{0}\u202f\u00bb. CanNotGetCommonMetadata_2 = Ne peut pas obtenir les m\u00e9ta-donn\u00e9es communes aux fichiers \u00ab\u202f{0}\u202f\u00bb. La raison est\u00a0: {1} diff --git a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/ascii/Store.java b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/ascii/Store.java index aea96a47b6..2409b120e0 100644 --- a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/ascii/Store.java +++ b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/ascii/Store.java @@ -48,7 +48,6 @@ import org.apache.sis.internal.storage.RangeArgument; import org.apache.sis.internal.storage.Resources; import org.apache.sis.internal.storage.io.ChannelDataInput; import org.apache.sis.measure.NumberRange; -import org.apache.sis.metadata.iso.DefaultMetadata; import org.apache.sis.metadata.sql.MetadataStoreException; import org.apache.sis.referencing.operation.matrix.Matrix3; import org.apache.sis.referencing.operation.transform.MathTransforms; @@ -142,7 +141,7 @@ class Store extends PRJDataStore implements GridCoverageResource { /** * The metadata object, or {@code null} if not yet created. */ - private DefaultMetadata metadata; + private Metadata metadata; /** * The full coverage, read when first requested then cached. diff --git a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/io/IOUtilities.java b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/io/IOUtilities.java index 3439243ddb..cc7b2a8b55 100644 --- a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/io/IOUtilities.java +++ b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/io/IOUtilities.java @@ -108,7 +108,7 @@ public final class IOUtilities extends Static { * {@link URI} or {@link CharSequence} instance. If no extension is found, returns an empty string. * If the given object is of unknown type, return {@code null}. * - * @param path the path as an instance of one of the above-cited types, or {@code null}. + * @param path the filename extension (may be an empty string), or {@code null} if unknown. * @return the extension in the given path, or an empty string if none, or {@code null} * if the given object is null or of unknown type. */ diff --git a/storage/sis-storage/src/main/java/org/apache/sis/storage/StorageConnector.java b/storage/sis-storage/src/main/java/org/apache/sis/storage/StorageConnector.java index 3fd18f6bed..1f074814cc 100644 --- a/storage/sis-storage/src/main/java/org/apache/sis/storage/StorageConnector.java +++ b/storage/sis-storage/src/main/java/org/apache/sis/storage/StorageConnector.java @@ -196,9 +196,9 @@ public class StorageConnector implements Serializable { add(OutputStream.class, StorageConnector::createOutputStream); add(Reader.class, StorageConnector::createReader); add(Connection.class, StorageConnector::createConnection); - add(ChannelDataInput.class, (s) -> s.createChannelDataInput(false)); // Undocumented case (SIS internal) - add(ChannelDataOutput.class, (s) -> s.createChannelDataOutput()); // Undocumented case (SIS internal) - add(ChannelFactory.class, (s) -> null); // Undocumented. Shall not cache. + add(ChannelDataInput.class, (s) -> s.createChannelDataInput(false)); // Undocumented case (SIS internal) + add(ChannelDataOutput.class, StorageConnector::createChannelDataOutput); // Undocumented case (SIS internal) + add(ChannelFactory.class, (s) -> null); // Undocumented. Shall not cache. /* * ChannelFactory may have been created as a side effect of creating a ReadableByteChannel. * Caller should have asked for another type (e.g. InputStream) before to ask for that type. @@ -1089,8 +1089,8 @@ public class StorageConnector implements Serializable { /* * If no ChannelDataInput has been created by the above code, get the input as an ImageInputStream and * read an arbitrary amount of bytes. Read only a small amount of bytes because, at the contrary of the - * buffer created in createChannelDataInput(boolean), the buffer created here is unlikely to be used for - * the reading process after the recognition of the file format. + * buffer created in `createChannelDataInput(boolean)`, the buffer created here is unlikely to be used + * for the reading process after the recognition of the file format. */ final ImageInputStream in = getStorageAs(ImageInputStream.class); if (in != null) { @@ -1179,6 +1179,12 @@ public class StorageConnector implements Serializable { views.put(ImageInputStream.class, views.get(source)); // Share the same Coupled instance. return (ImageInputStream) input; } else { + /* + * We do not invoke `ImageIO.createImageInputStream(Object)` because we do not know + * how the stream will use the `storage` object. It may read in advance some bytes, + * which can invalidate the storage for use outside the `ImageInputStream`. Instead + * creating image input/output streams is left to caller's responsibility. + */ addView(ImageInputStream.class, null); // Remember that there is no view. return null; } diff --git a/storage/sis-storage/src/test/java/org/apache/sis/storage/StorageConnectorTest.java b/storage/sis-storage/src/test/java/org/apache/sis/storage/StorageConnectorTest.java index 8231ead7b0..b861b07eca 100644 --- a/storage/sis-storage/src/test/java/org/apache/sis/storage/StorageConnectorTest.java +++ b/storage/sis-storage/src/test/java/org/apache/sis/storage/StorageConnectorTest.java @@ -303,7 +303,7 @@ public final strictfp class StorageConnectorTest extends TestCase { /** * Tests the {@link StorageConnector#getStorageAs(Class)} method for the {@link ChannelDataInput} type. * The initial value should not be an instance of {@link ChannelImageInputStream} in order to avoid initializing - * the Image I/O classes. However after a call to {@code getStorageAt(ChannelImageInputStream.class)}, the type + * the Image I/O classes. However after a call to {@code getStorageAs(ChannelImageInputStream.class)}, the type * should have been promoted. * * @throws DataStoreException if an error occurred while using the storage connector.
