You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-commits@hadoop.apache.org by je...@apache.org on 2013/10/23 20:03:08 UTC
svn commit: r1535086 - in
/hadoop/common/trunk/hadoop-common-project/hadoop-common: CHANGES.txt
src/main/java/org/apache/hadoop/fs/HarFileSystem.java
Author: jeagles
Date: Wed Oct 23 18:03:08 2013
New Revision: 1535086
URL: http://svn.apache.org/r1535086
Log:
HADOOP-9016. HarFsInputStream.skip(long) must never return negative value. (Ivan A. Veselovsky via jeagles)
Modified:
hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/HarFileSystem.java
Modified: hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
URL: http://svn.apache.org/viewvc/hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt?rev=1535086&r1=1535085&r2=1535086&view=diff
==============================================================================
--- hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt (original)
+++ hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt Wed Oct 23 18:03:08 2013
@@ -411,6 +411,9 @@ Release 2.3.0 - UNRELEASED
HADOOP-9981. globStatus should minimize its listStatus and getFileStatus
calls. (Contributed by Colin Patrick McCabe)
+ HADOOP-9016. HarFsInputStream.skip(long) must never return negative value.
+ (Ivan A. Veselovsky via jeagles)
+
Release 2.2.1 - UNRELEASED
INCOMPATIBLE CHANGES
Modified: hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/HarFileSystem.java
URL: http://svn.apache.org/viewvc/hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/HarFileSystem.java?rev=1535086&r1=1535085&r2=1535086&view=diff
==============================================================================
--- hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/HarFileSystem.java (original)
+++ hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/HarFileSystem.java Wed Oct 23 18:03:08 2013
@@ -898,11 +898,15 @@ public class HarFileSystem extends FileS
private long position, start, end;
//The underlying data input stream that the
// underlying filesystem will return.
- private FSDataInputStream underLyingStream;
+ private final FSDataInputStream underLyingStream;
//one byte buffer
- private byte[] oneBytebuff = new byte[1];
+ private final byte[] oneBytebuff = new byte[1];
+
HarFsInputStream(FileSystem fs, Path path, long start,
long length, int bufferSize) throws IOException {
+ if (length < 0) {
+ throw new IllegalArgumentException("Negative length ["+length+"]");
+ }
underLyingStream = fs.open(path, bufferSize);
underLyingStream.seek(start);
// the start of this file in the part file
@@ -916,7 +920,7 @@ public class HarFileSystem extends FileS
@Override
public synchronized int available() throws IOException {
long remaining = end - underLyingStream.getPos();
- if (remaining > (long)Integer.MAX_VALUE) {
+ if (remaining > Integer.MAX_VALUE) {
return Integer.MAX_VALUE;
}
return (int) remaining;
@@ -948,10 +952,14 @@ public class HarFileSystem extends FileS
return (ret <= 0) ? -1: (oneBytebuff[0] & 0xff);
}
+ // NB: currently this method actually never executed becusae
+ // java.io.DataInputStream.read(byte[]) directly delegates to
+ // method java.io.InputStream.read(byte[], int, int).
+ // However, potentially it can be invoked, so leave it intact for now.
@Override
public synchronized int read(byte[] b) throws IOException {
- int ret = read(b, 0, b.length);
- if (ret != -1) {
+ final int ret = read(b, 0, b.length);
+ if (ret > 0) {
position += ret;
}
return ret;
@@ -980,15 +988,19 @@ public class HarFileSystem extends FileS
public synchronized long skip(long n) throws IOException {
long tmpN = n;
if (tmpN > 0) {
- if (position + tmpN > end) {
- tmpN = end - position;
- }
+ final long actualRemaining = end - position;
+ if (tmpN > actualRemaining) {
+ tmpN = actualRemaining;
+ }
underLyingStream.seek(tmpN + position);
position += tmpN;
return tmpN;
- }
- return (tmpN < 0)? -1 : 0;
- }
+ }
+ // NB: the contract is described in java.io.InputStream.skip(long):
+ // this method returns the number of bytes actually skipped, so,
+ // the return value should never be negative.
+ return 0;
+ }
@Override
public synchronized long getPos() throws IOException {
@@ -996,14 +1008,23 @@ public class HarFileSystem extends FileS
}
@Override
- public synchronized void seek(long pos) throws IOException {
- if (pos < 0 || (start + pos > end)) {
- throw new IOException("Failed to seek: EOF");
- }
+ public synchronized void seek(final long pos) throws IOException {
+ validatePosition(pos);
position = start + pos;
underLyingStream.seek(position);
}
+ private void validatePosition(final long pos) throws IOException {
+ if (pos < 0) {
+ throw new IOException("Negative position: "+pos);
+ }
+ final long length = end - start;
+ if (pos > length) {
+ throw new IOException("Position behind the end " +
+ "of the stream (length = "+length+"): " + pos);
+ }
+ }
+
@Override
public boolean seekToNewSource(long targetPos) throws IOException {
// do not need to implement this
@@ -1020,7 +1041,12 @@ public class HarFileSystem extends FileS
throws IOException {
int nlength = length;
if (start + nlength + pos > end) {
- nlength = (int) (end - (start + pos));
+ // length corrected to the real remaining length:
+ nlength = (int) (end - start - pos);
+ }
+ if (nlength <= 0) {
+ // EOS:
+ return -1;
}
return underLyingStream.read(pos + start , b, offset, nlength);
}