Fix misuse of read and skipBytes (#2557)

* Fix misuse of read and skipBytes

* improve exception messages

* don't throw EOF exception if the stride steps outside the stream

jme3-core/src/main/java/com/jme3/util/LittleEndien.java

@@ -62,6 +62,9 @@ public class LittleEndien extends InputStream implements DataInput {
         return in.read(buf);
     }
 
+    /**
+     * Attempts(!) to read up to len bytes into buf at offset off.
+     */
     @Override
     public int read(byte[] buf, int off, int len) throws IOException {
         return in.read(buf, off, len);
@@ -142,14 +145,24 @@ public class LittleEndien extends InputStream implements DataInput {
 
     @Override
     public void readFully(byte b[]) throws IOException {
-        in.read(b, 0, b.length);
+        readFully(b, 0, b.length);
     }
 
     @Override
     public void readFully(byte b[], int off, int len) throws IOException {
-        in.read(b, off, len);
+        int totalRead = 0;
+        while (totalRead < len) {
+            int bytesRead = in.read(b, off + totalRead, len - totalRead);
+            if (bytesRead < 0) {
+                throw new EOFException("Reached end of stream before reading fully.");
+            }
+            totalRead += bytesRead;
+        }
     }
 
+    /**
+     * Attempts(!) to skip n bytes in the input stream.
+     */
     @Override
     public int skipBytes(int n) throws IOException {
         return (int) in.skip(n);

jme3-core/src/plugins/java/com/jme3/audio/plugins/WAVLoader.java

@@ -38,6 +38,7 @@ import com.jme3.audio.AudioData;
 import com.jme3.audio.AudioKey;
 import com.jme3.audio.AudioStream;
 import com.jme3.audio.SeekableStream;
+import com.jme3.export.binary.ByteUtils;
 import com.jme3.util.BufferUtils;
 import com.jme3.util.LittleEndien;
 import java.io.BufferedInputStream;
@@ -112,7 +113,7 @@ public class WAVLoader implements AssetLoader {
             }
             InputStream newStream = info.openStream();
             try {
-                newStream.skip(resetOffset);
+                ByteUtils.skipFully(newStream, resetOffset);
                 this.in = new BufferedInputStream(newStream);
             } catch (IOException ex) {
                 // Resource could have gotten lost, etc.
@@ -172,7 +173,7 @@ public class WAVLoader implements AssetLoader {
         // Skip any extra parameters in the format chunk (e.g., for non-PCM formats)
         int remainingChunkBytes = chunkSize - 16;
         if (remainingChunkBytes > 0) {
-            in.skipBytes(remainingChunkBytes);
+            ByteUtils.skipFully((InputStream)in, remainingChunkBytes);
         }
     }
 

jme3-core/src/plugins/java/com/jme3/export/binary/ByteUtils.java

@@ -32,6 +32,8 @@
 package com.jme3.export.binary;
 
 import java.io.ByteArrayOutputStream;
+import java.io.DataInput;
+import java.io.EOFException;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
@@ -50,6 +52,77 @@ public class ByteUtils {
     private ByteUtils() {
     }
 
+
+    public static void readFully(InputStream in, byte[] b) throws IOException {
+        int bytesRead = 0;
+        int read = 0;
+        while (bytesRead < b.length && (read = in.read(b, bytesRead, b.length - bytesRead)) != -1) {
+            bytesRead += read;
+        }
+        if (bytesRead < b.length) {
+            throw new IOException(
+                    "End of stream reached prematurely after " + bytesRead + " of " + b.length + " bytes.");
+        }
+    }
+
+
+    public static void skipFully(InputStream in, long n) throws IOException {
+        skipFully(in, n, true);
+    }
+
+    public static void skipFully(InputStream in, long n, boolean throwOnEOF) throws IOException {
+        while (n > 0) {
+            long skipped = in.skip(n);
+            if (skipped > 0 && skipped <= n) { // skipped some bytes
+                n -= skipped;
+            } else if (skipped == 0) { // skipped nothing
+                // distinguish between EOF and no bytes available
+                if (in.read() == -1) {
+                    if (throwOnEOF) {
+                        throw new EOFException();
+                    } else {
+                        return;
+                    }
+                } else {
+                    // stream was just hangling 
+                    n--;
+                }
+            } else {
+                throw new IOException(
+                        "Unable to skip exactly " + n + " bytes. Only " + skipped + " bytes were skipped.");
+            }
+        }
+    }
+
+    public static void skipFully(DataInput in, int n) throws IOException {
+        skipFully(in, n, true);
+    }
+
+    public static void skipFully(DataInput in, int n, boolean throwOnEOF) throws IOException {
+        while (n > 0) {
+            long skipped = in.skipBytes(n);
+            if (skipped > 0 && skipped <= n) { // skipped some bytes
+                n -= skipped;
+            } else if (skipped == 0) { // skipped nothing
+                // distinguish between EOF and no bytes available
+                try {
+                    in.readByte();
+                } catch (EOFException e) {
+                    if (throwOnEOF) {
+                        throw e;
+                    } else {
+                        return;
+                    }
+                }
+                n--;
+            } else {
+                throw new IOException(
+                        "Unable to skip exactly " + n + " bytes. Only " + skipped + " bytes were skipped.");
+            }
+        }
+    }
+
+
     /**
      * Takes an InputStream and returns the complete byte content of it
      * 
@@ -125,7 +198,7 @@ public class ByteUtils {
         byte[] byteArray = new byte[2];
 
         // Read in the next 2 bytes
-        inputStream.read(byteArray);
+        readFully(inputStream, byteArray);
 
         short number = convertShortFromBytes(byteArray);
 
@@ -187,7 +260,7 @@ public class ByteUtils {
         byte[] byteArray = new byte[4];
 
         // Read in the next 4 bytes
-        inputStream.read(byteArray);
+        readFully(inputStream, byteArray);
 
         int number = convertIntFromBytes(byteArray);
 
@@ -263,7 +336,7 @@ public class ByteUtils {
         byte[] byteArray = new byte[8];
 
         // Read in the next 8 bytes
-        inputStream.read(byteArray);
+        readFully(inputStream, byteArray);
 
         long number = convertLongFromBytes(byteArray);
 
@@ -326,7 +399,7 @@ public class ByteUtils {
         byte[] byteArray = new byte[8];
 
         // Read in the next 8 bytes
-        inputStream.read(byteArray);
+        readFully(inputStream, byteArray);
 
         double number = convertDoubleFromBytes(byteArray);
 
@@ -382,7 +455,7 @@ public class ByteUtils {
         byte[] byteArray = new byte[4];
 
         // Read in the next 4 bytes
-        inputStream.read(byteArray);
+        readFully(inputStream, byteArray);
 
         float number = convertFloatFromBytes(byteArray);
 
@@ -440,7 +513,7 @@ public class ByteUtils {
         byte[] byteArray = new byte[1];
 
         // Read in the next byte
-        inputStream.read(byteArray);
+        readFully(inputStream, byteArray);
 
         return convertBooleanFromBytes(byteArray);
     }
@@ -470,9 +543,7 @@ public class ByteUtils {
      *             if bytes greater than the length of the store.
      */
     public static byte[] readData(byte[] store, int bytes, InputStream is) throws IOException {
-        for (int i = 0; i < bytes; i++) {
-            store[i] = (byte)is.read();
-        }
+        readFully(is, store);
         return store;
     }
 

jme3-core/src/plugins/java/com/jme3/texture/plugins/DDSLoader.java

@@ -34,6 +34,7 @@ package com.jme3.texture.plugins;
 import com.jme3.asset.AssetInfo;
 import com.jme3.asset.AssetLoader;
 import com.jme3.asset.TextureKey;
+import com.jme3.export.binary.ByteUtils;
 import com.jme3.texture.Image;
 import com.jme3.texture.Image.Format;
 import com.jme3.texture.Texture;
@@ -160,7 +161,7 @@ public class DDSLoader implements AssetLoader {
             }
         }
 
-        in.skipBytes(4); // skip reserved value
+        ByteUtils.skipFully(in, 4);  // skip reserved value
     }
 
     private void setPixelFormat(int dxgiFormat) throws IOException {
@@ -227,13 +228,13 @@ public class DDSLoader implements AssetLoader {
         pitchOrSize = in.readInt();
         depth = in.readInt();
         mipMapCount = in.readInt();
-        in.skipBytes(44);
+        ByteUtils.skipFully(in, 44);
         pixelFormat = null;
         directx10 = false;
         readPixelFormat();
         caps1 = in.readInt();
         caps2 = in.readInt();
-        in.skipBytes(12);
+        ByteUtils.skipFully(in, 12);
         texture3D = false;
 
         if (!directx10) {
@@ -292,7 +293,7 @@ public class DDSLoader implements AssetLoader {
             compressed = true;
             int fourcc = in.readInt();
             int swizzle = in.readInt();
-            in.skipBytes(16);
+            ByteUtils.skipFully(in, 16);
 
             switch (fourcc) {
                 case PF_DXT1:

jme3-core/src/plugins/java/com/jme3/texture/plugins/HDRLoader.java

@@ -34,6 +34,7 @@ package com.jme3.texture.plugins;
 import com.jme3.asset.AssetInfo;
 import com.jme3.asset.AssetLoader;
 import com.jme3.asset.TextureKey;
+import com.jme3.export.binary.ByteUtils;
 import com.jme3.math.FastMath;
 import com.jme3.texture.Image;
 import com.jme3.texture.Image.Format;
@@ -180,16 +181,14 @@ public class HDRLoader implements AssetLoader {
         return true;
     }
     
-    private boolean decodeScanlineUncompressed(InputStream in, int width) throws IOException{
+    private void decodeScanlineUncompressed(InputStream in, int width) throws IOException{
         byte[] rgbe = new byte[4];
         
         for (int i = 0; i < width; i+=3){
-            if (in.read(rgbe) < 1)
-                return false;
-
+            ByteUtils.readFully(in, rgbe);
             writeRGBE(rgbe);
         }
-        return true;
+        
     }
     
     private void decodeScanline(InputStream in, int width) throws IOException{
@@ -200,7 +199,7 @@ public class HDRLoader implements AssetLoader {
         
         // check format
         byte[] data = new byte[4];
-        in.read(data);
+        ByteUtils.readFully(in, data);
         if (data[0] != 0x02 || data[1] != 0x02 || (data[2] & 0x80) != 0){
             // not RLE data
             decodeScanlineUncompressed(in, width-1);

jme3-core/src/plugins/java/com/jme3/texture/plugins/PFMLoader.java

@@ -34,6 +34,7 @@ package com.jme3.texture.plugins;
 import com.jme3.asset.AssetInfo;
 import com.jme3.asset.AssetLoader;
 import com.jme3.asset.TextureKey;
+import com.jme3.export.binary.ByteUtils;
 import com.jme3.texture.Image;
 import com.jme3.texture.Image.Format;
 import com.jme3.texture.image.ColorSpace;
@@ -115,12 +116,7 @@ public class PFMLoader implements AssetLoader {
             if (!needYFlip)
                 imageData.position(scanLineBytes * y);
 
-            int read = 0;
-            int off = 0;
-            do {
-                read = in.read(scanline, off, scanline.length - off);
-                off += read;
-            } while (read > 0);
+            ByteUtils.readFully(in, scanline);
 
             if (needEndianFlip){
                 flipScanline(scanline);

jme3-core/src/plugins/java/com/jme3/texture/plugins/TGALoader.java

@@ -34,6 +34,7 @@ package com.jme3.texture.plugins;
 import com.jme3.asset.AssetInfo;
 import com.jme3.asset.AssetLoader;
 import com.jme3.asset.TextureKey;
+import com.jme3.export.binary.ByteUtils;
 import com.jme3.math.FastMath;
 import com.jme3.texture.Image;
 import com.jme3.texture.Image.Format;
@@ -158,7 +159,7 @@ public final class TGALoader implements AssetLoader {
 
         // Skip image ID
         if (idLength > 0) {
-            dis.skip(idLength);
+            ByteUtils.skipFully((InputStream) dis, idLength);
         }
 
         ColorMapEntry[] cMapEntries = null;
@@ -168,7 +169,7 @@ public final class TGALoader implements AssetLoader {
             int bitsPerColor = Math.min(cMapDepth / 3, 8);
 
             byte[] cMapData = new byte[bytesInColorMap];
-            dis.read(cMapData);
+            ByteUtils.readFully((InputStream) dis, cMapData);
 
             // Only go to the trouble of constructing the color map
             // table if this is declared a color mapped image.

jme3-core/src/plugins/java/com/jme3/texture/plugins/ktx/KTXLoader.java

@@ -34,6 +34,7 @@ package com.jme3.texture.plugins.ktx;
 import com.jme3.asset.AssetInfo;
 import com.jme3.asset.AssetLoader;
 import com.jme3.asset.TextureKey;
+import com.jme3.export.binary.ByteUtils;
 import com.jme3.renderer.Caps;
 import com.jme3.renderer.opengl.GLImageFormat;
 import com.jme3.renderer.opengl.GLImageFormats;
@@ -95,7 +96,7 @@ public class KTXLoader implements AssetLoader {
 
         DataInput in = new DataInputStream(stream);
         try {
-            stream.read(fileId, 0, 12);
+            ByteUtils.readFully(stream, fileId);
             if (!checkFileIdentifier(fileId)) {
                 throw new IllegalArgumentException("Unrecognized ktx file identifier : " + new String(fileId) + " should be " + new String(fileIdentifier));
             }
@@ -193,13 +194,13 @@ public class KTXLoader implements AssetLoader {
                         }
                         //cube padding
                         if (numberOfFaces == 6 && numberOfArrayElements == 0) {
-                            in.skipBytes(3 - ((nbPixelRead + 3) % 4));
+                            ByteUtils.skipFully(in, 3 - ((nbPixelRead + 3) % 4));
                         }
                     }
                 }
                 //mip padding
                 log.log(Level.FINE, "skipping {0}", (3 - ((imageSize + 3) % 4)));
-                in.skipBytes(3 - ((imageSize + 3) % 4));
+                ByteUtils.skipFully(in, 3 - ((imageSize + 3) % 4));
                 offset+=imageSize;
             }
             //there are loaded mip maps we set the sizes
@@ -305,7 +306,7 @@ public class KTXLoader implements AssetLoader {
             //padding
             int padding = 3 - ((keyAndValueByteSize + 3) % 4);            
             if (padding > 0) {
-                in.skipBytes(padding);
+                ByteUtils.skipFully(in, padding);
             }
             i += 4 + keyAndValueByteSize + padding;
         }

jme3-desktop/src/main/java/com/jme3/cursors/plugins/CursorLoader.java

@@ -33,6 +33,7 @@ package com.jme3.cursors.plugins;
 
 import com.jme3.asset.AssetInfo;
 import com.jme3.asset.AssetLoader;
+import com.jme3.export.binary.ByteUtils;
 import com.jme3.util.BufferUtils;
 import com.jme3.util.LittleEndien;
 import java.awt.geom.AffineTransform;
@@ -127,13 +128,13 @@ public class CursorLoader implements AssetLoader {
                     nextInt = getNext(leIn);
                     while (nextInt >= 0) {
                         if (nextInt == 0x68696e61) {
-//                            System.out.println("we have 'anih' header");
-                            leIn.skipBytes(8); // internal struct length (always 36)
+//                            System.out.println("we have 'anih' header"); 
+                            ByteUtils.skipFully((DataInput) leIn, 8);  // internal struct length (always 36)
                             numIcons = leIn.readInt();
                             steps = leIn.readInt(); // number of blits for ani cycles
                             width = leIn.readInt();
                             height = leIn.readInt();
-                            leIn.skipBytes(8);
+                            ByteUtils.skipFully((DataInput) leIn, 8);
                             jiffy = leIn.readInt();
                             flag = leIn.readInt();
                             nextInt = leIn.readInt();
@@ -162,7 +163,8 @@ public class CursorLoader implements AssetLoader {
                             nextInt = leIn.readInt();
                             if (nextInt == 0x4f464e49) { // Got an INFO, skip its length
                                 // this part consist  of Author, title, etc
-                                leIn.skipBytes(length - 4);
+            
+                                ByteUtils.skipFully((DataInput) leIn, length - 4);
 //                                System.out.println(" Discarding INFO (skipped = " + skipped + ")");
                                 nextInt = leIn.readInt();
                             } else if (nextInt == 0x6d617266) { // found a 'fram' for animation
@@ -177,10 +179,10 @@ public class CursorLoader implements AssetLoader {
                                         if (i > 0) {
                                             // skip 'icon' header and length as they are
                                             // known already and won't change.
-                                            leIn.skipBytes(8);
+                                            ByteUtils.skipFully((DataInput) leIn, 8);
                                         }
                                         byte[] data = new byte[icoLength];
-                                        ((InputStream) leIn).read(data, 0, icoLength);
+                                        ByteUtils.readFully((InputStream) leIn, data);
                                         // in case the header didn't have width or height
                                         // get it from first image.
                                         if (width == 0 || height == 0 && i == 1) {
@@ -216,7 +218,7 @@ public class CursorLoader implements AssetLoader {
             ByteArrayOutputStream out = new ByteArrayOutputStream();
             byte[] buffer = new byte[16384];
             int bytesRead;
-            while ((bytesRead = in.read(buffer)) >= 0) {
+            while ((bytesRead = in.read(buffer)) != -1) {
                 out.write(buffer, 0, bytesRead);
             }
             icoimages = out.toByteArray();

jme3-plugins/src/gltf/java/com/jme3/scene/plugins/gltf/GltfUtils.java

@@ -36,6 +36,7 @@ import com.jme3.plugins.json.JsonElement;
 import com.jme3.plugins.json.JsonObject;
 import com.jme3.asset.AssetInfo;
 import com.jme3.asset.AssetLoadException;
+import com.jme3.export.binary.ByteUtils;
 import com.jme3.math.*;
 import com.jme3.plugins.json.Json;
 import com.jme3.plugins.json.JsonParser;
@@ -338,14 +339,14 @@ public class GltfUtils {
         int dataLength = componentSize * numComponents;
         int stride = Math.max(dataLength, byteStride);
         int end = count * stride + byteOffset;
-        stream.skipBytes(byteOffset);
+        ByteUtils.skipFully((InputStream) stream, byteOffset);
         while (index < end) {
             for (int i = 0; i < numComponents; i++) {
                 buffer.put(stream.readShort());
             }
 
             if (dataLength < stride) {
-                stream.skipBytes(stride - dataLength);
+                ByteUtils.skipFully((InputStream) stream, stride - dataLength, false);
             }
             index += stride;
         }
@@ -358,13 +359,13 @@ public class GltfUtils {
         int dataLength = componentSize * numComponents;
         int stride = Math.max(dataLength, byteStride);
         int end = count * stride + byteOffset;
-        stream.skipBytes(byteOffset);
+        ByteUtils.skipFully((InputStream) stream, byteOffset);
         while (index < end) {
             for (int i = 0; i < numComponents; i++) {
                 buffer.put(stream.readInt());
             }
             if (dataLength < stride) {
-                stream.skipBytes(stride - dataLength);
+                ByteUtils.skipFully((InputStream) stream, stride - dataLength, false);
             }
             index += stride;
         }
@@ -376,13 +377,13 @@ public class GltfUtils {
         int dataLength = componentSize * numComponents;
         int stride = Math.max(dataLength, byteStride);
         int end = count * stride + byteOffset;
-        stream.skipBytes(byteOffset);
+        ByteUtils.skipFully((InputStream) stream, byteOffset);
         while (index < end) {
             for (int i = 0; i < numComponents; i++) {
                 buffer.put(readAsFloat(stream, format));
             }
             if (dataLength < stride) {
-                stream.skipBytes(stride - dataLength);
+                ByteUtils.skipFully((InputStream) stream, stride - dataLength, false);
             }
             index += stride;
         }
@@ -423,7 +424,7 @@ public class GltfUtils {
         int dataLength = componentSize * numComponents;
         int stride = Math.max(dataLength, byteStride);
         int end = count * stride + byteOffset;
-        stream.skipBytes(byteOffset);
+        ByteUtils.skipFully((InputStream) stream, byteOffset);
 
         if (dataLength == stride) {
             read(stream, array, end - index);
@@ -438,7 +439,7 @@ public class GltfUtils {
             System.arraycopy(buffer, 0, array, arrayIndex, numComponents);
             arrayIndex += numComponents;
             if (dataLength < stride) {
-                stream.skipBytes(stride - dataLength);
+                ByteUtils.skipFully((InputStream) stream, stride - dataLength, false);
             }
             index += stride;
         }
@@ -461,7 +462,7 @@ public class GltfUtils {
         int dataLength = componentSize * numComponents;
         int stride = Math.max(dataLength, byteStride);
         int end = count * stride + byteOffset;
-        stream.skipBytes(byteOffset);
+        ByteUtils.skipFully((InputStream) stream, byteOffset);
         int arrayIndex = 0;
         while (index < end) {
             for (int i = 0; i < numComponents; i++) {
@@ -473,7 +474,7 @@ public class GltfUtils {
                 arrayIndex++;
             }
             if (dataLength < stride) {
-                stream.skipBytes(stride - dataLength);
+                ByteUtils.skipFully((InputStream) stream, stride - dataLength, false);
             }
             index += stride;
         }
@@ -563,7 +564,7 @@ public class GltfUtils {
         int dataLength = componentSize * numComponents;
         int stride = Math.max(dataLength, byteStride);
         int end = count * stride + byteOffset;
-        stream.skipBytes(byteOffset);
+        ByteUtils.skipFully((InputStream) stream, byteOffset);
         int arrayIndex = 0;
         while (index < end) {
             for (int i = 0; i < numComponents; i++) {
@@ -571,7 +572,7 @@ public class GltfUtils {
                 arrayIndex++;
             }
             if (dataLength < stride) {
-                stream.skipBytes(stride - dataLength);
+                ByteUtils.skipFully((InputStream) stream, stride - dataLength, false);
             }
             index += stride;
         }
@@ -583,7 +584,7 @@ public class GltfUtils {
         int dataLength = componentSize * numComponents;
         int stride = Math.max(dataLength, byteStride);
         int end = count * stride + byteOffset;
-        stream.skipBytes(byteOffset);
+        ByteUtils.skipFully((InputStream) stream, byteOffset);
         int arrayIndex = 0;
         while (index < end) {
             array[arrayIndex] = new Vector3f(
@@ -594,7 +595,7 @@ public class GltfUtils {
 
             arrayIndex++;
             if (dataLength < stride) {
-                stream.skipBytes(stride - dataLength);
+                ByteUtils.skipFully((InputStream) stream, stride - dataLength, false);
             }
 
             index += stride;
@@ -607,7 +608,7 @@ public class GltfUtils {
         int dataLength = componentSize * numComponents;
         int stride = Math.max(dataLength, byteStride);
         int end = count * stride + byteOffset;
-        stream.skipBytes(byteOffset);
+        ByteUtils.skipFully((InputStream) stream, byteOffset);
         int arrayIndex = 0;
         while (index < end) {
             array[arrayIndex] = new Quaternion(
@@ -619,7 +620,7 @@ public class GltfUtils {
 
             arrayIndex++;
             if (dataLength < stride) {
-                stream.skipBytes(stride - dataLength);
+                ByteUtils.skipFully((InputStream) stream, stride - dataLength, false);
             }
             index += stride;
         }
@@ -631,7 +632,7 @@ public class GltfUtils {
         int dataLength = componentSize * numComponents;
         int stride = Math.max(dataLength, byteStride);
         int end = count * stride + byteOffset;
-        stream.skipBytes(byteOffset);
+        ByteUtils.skipFully((InputStream) stream, byteOffset);
         int arrayIndex = 0;
         while (index < end) {
 
@@ -657,7 +658,7 @@ public class GltfUtils {
 
             arrayIndex++;
             if (dataLength < stride) {
-                stream.skipBytes(stride - dataLength);
+                ByteUtils.skipFully((InputStream) stream, stride - dataLength, false);
             }
 
             index += stride;