From bfbaf2c9d9ff156eba917e112ccec1d7cef252a0 Mon Sep 17 00:00:00 2001 From: Jerome Lacoste Date: Fri, 4 Feb 2011 12:15:05 +0100 Subject: [PATCH] JENKINS-8686 IO stream copies are not done properly. First pass at making sure we don't stop copying when the reader reads 0 characters. I will now also review the callers for our internal code, and check if we can use IOUtils methods instead. --- core/src/main/java/hudson/Proc.java | 6 +++--- core/src/main/java/hudson/Util.java | 6 +++--- core/src/main/java/hudson/model/LargeText.java | 2 +- .../hudson/org/apache/tools/tar/TarInputStream.java | 2 +- .../src/main/java/hudson/util/ChunkedInputStream.java | 4 ++-- .../main/java/hudson/util/HeadBufferingStream.java | 2 +- core/src/main/java/hudson/util/StreamCopyThread.java | 4 ++-- core/src/main/java/hudson/util/io/ZipArchiver.java | 11 +++++++---- .../src/test/java/hudson/maven/agent/LaunchTest.java | 2 +- .../main/java/hudson/remoting/BinarySafeStream.java | 5 ++++- .../java/hudson/remoting/FastPipedInputStream.java | 4 ++-- .../main/java/hudson/remoting/RemoteClassLoader.java | 9 ++++++--- .../main/java/hudson/remoting/RemoteInputStream.java | 4 ++-- .../main/java/hudson/remoting/forward/CopyThread.java | 4 ++-- .../src/test/java/hudson/remoting/CopyThread.java | 11 +++++++---- .../src/test/java/hudson/remoting/TestCallable.java | 9 ++++++--- 16 files changed, 50 insertions(+), 35 deletions(-) diff --git a/core/src/main/java/hudson/Proc.java b/core/src/main/java/hudson/Proc.java index 4a215a1e0e..eee3cf0f73 100644 --- a/core/src/main/java/hudson/Proc.java +++ b/core/src/main/java/hudson/Proc.java @@ -306,7 +306,7 @@ public abstract class Proc { try { byte[] buf = new byte[8192]; int len; - while ((len = in.read(buf)) > 0) { + while ((len = in.read(buf)) >= 0) { out.write(buf, 0, len); out.flush(); } @@ -315,7 +315,7 @@ public abstract class Proc { out.close(); } } catch (IOException e) { - // TODO: what to do? + throw new RuntimeException(e); } } } @@ -331,7 +331,7 @@ public abstract class Proc { } /** - * Retemoly launched process via {@link Channel}. + * Remotely launched process via {@link hudson.remoting.Channel}. */ public static final class RemoteProc extends Proc { private final Future process; diff --git a/core/src/main/java/hudson/Util.java b/core/src/main/java/hudson/Util.java index 412ab84d92..1fd489ce50 100644 --- a/core/src/main/java/hudson/Util.java +++ b/core/src/main/java/hudson/Util.java @@ -422,14 +422,14 @@ public class Util { public static void copyStream(InputStream in,OutputStream out) throws IOException { byte[] buf = new byte[8192]; int len; - while((len=in.read(buf))>0) + while((len=in.read(buf))>=0) out.write(buf,0,len); } public static void copyStream(Reader in, Writer out) throws IOException { char[] buf = new char[8192]; int len; - while((len=in.read(buf))>0) + while((len=in.read(buf))>=0) out.write(buf,0,len); } @@ -519,7 +519,7 @@ public class Util { DigestInputStream in =new DigestInputStream(source,md5); try { - while(in.read(garbage)>0) + while(in.read(garbage)>=0) ; // simply discard the input } finally { in.close(); diff --git a/core/src/main/java/hudson/model/LargeText.java b/core/src/main/java/hudson/model/LargeText.java index c7606d90ab..739db31a6c 100644 --- a/core/src/main/java/hudson/model/LargeText.java +++ b/core/src/main/java/hudson/model/LargeText.java @@ -365,7 +365,7 @@ public class LargeText { return in.read(buf); } - public int read(byte[] buf, int offset, int length) throws IOException { + public int read(byte[] buf, int offset, int length) throws IOException { // FIXME-CHECK-CALLERS return in.read(buf,offset,length); } } diff --git a/core/src/main/java/hudson/org/apache/tools/tar/TarInputStream.java b/core/src/main/java/hudson/org/apache/tools/tar/TarInputStream.java index f2d10c1c39..c9bff3332c 100644 --- a/core/src/main/java/hudson/org/apache/tools/tar/TarInputStream.java +++ b/core/src/main/java/hudson/org/apache/tools/tar/TarInputStream.java @@ -291,7 +291,7 @@ public class TarInputStream extends FilterInputStream { * @throws IOException on error */ public int read() throws IOException { - int num = this.read(this.oneBuf, 0, 1); + int num = this.read(this.oneBuf, 0, 1); // FIXME check if read() guarantees to return at least 1 character, otherwise bug lurking return num == -1 ? -1 : ((int) this.oneBuf[0]) & 0xFF; } diff --git a/core/src/main/java/hudson/util/ChunkedInputStream.java b/core/src/main/java/hudson/util/ChunkedInputStream.java index 5691eeb80d..e8bfb00a1e 100644 --- a/core/src/main/java/hudson/util/ChunkedInputStream.java +++ b/core/src/main/java/hudson/util/ChunkedInputStream.java @@ -138,7 +138,7 @@ public class ChunkedInputStream extends InputStream { * @throws IOException if an IO problem occurs. */ @Override - public int read (byte[] b, int off, int len) throws IOException { + public int read (byte[] b, int off, int len) throws IOException { // FIXME-CHECK-CALLERS if (closed) { throw new IOException("Attempted read from closed stream."); @@ -168,7 +168,7 @@ public class ChunkedInputStream extends InputStream { * @throws IOException if an IO problem occurs. */ @Override - public int read (byte[] b) throws IOException { + public int read (byte[] b) throws IOException { // FIXME-CHECK-CALLERS return read(b, 0, b.length); } diff --git a/core/src/main/java/hudson/util/HeadBufferingStream.java b/core/src/main/java/hudson/util/HeadBufferingStream.java index 64240c7840..e3afe6087e 100644 --- a/core/src/main/java/hudson/util/HeadBufferingStream.java +++ b/core/src/main/java/hudson/util/HeadBufferingStream.java @@ -57,7 +57,7 @@ public class HeadBufferingStream extends FilterInputStream { } @Override - public int read(byte b[], int off, int len) throws IOException { + public int read(byte b[], int off, int len) throws IOException { // FIXME-CHECK-CALLERS int r = in.read(b, off, len); if(r>0) { int sp = space(); diff --git a/core/src/main/java/hudson/util/StreamCopyThread.java b/core/src/main/java/hudson/util/StreamCopyThread.java index 80bdeb1e35..23bddac994 100644 --- a/core/src/main/java/hudson/util/StreamCopyThread.java +++ b/core/src/main/java/hudson/util/StreamCopyThread.java @@ -57,7 +57,7 @@ public class StreamCopyThread extends Thread { try { byte[] buf = new byte[8192]; int len; - while ((len = in.read(buf)) > 0) + while ((len = in.read(buf)) >= 0) out.write(buf, 0, len); } finally { // it doesn't make sense not to close InputStream that's already EOF-ed, @@ -67,7 +67,7 @@ public class StreamCopyThread extends Thread { out.close(); } } catch (IOException e) { - // TODO: what to do? + throw new RuntimeException(e); } } } diff --git a/core/src/main/java/hudson/util/io/ZipArchiver.java b/core/src/main/java/hudson/util/io/ZipArchiver.java index 19e9b678fc..5899c1919a 100644 --- a/core/src/main/java/hudson/util/io/ZipArchiver.java +++ b/core/src/main/java/hudson/util/io/ZipArchiver.java @@ -57,10 +57,13 @@ final class ZipArchiver extends Archiver { } else { zip.putNextEntry(new ZipEntry(relativePath)); FileInputStream in = new FileInputStream(f); - int len; - while((len=in.read(buf))>0) - zip.write(buf,0,len); - in.close(); + try { + int len; + while((len=in.read(buf))>=0) + zip.write(buf,0,len); + } finally { + in.close(); + } zip.closeEntry(); } entriesWritten++; diff --git a/maven-agent/src/test/java/hudson/maven/agent/LaunchTest.java b/maven-agent/src/test/java/hudson/maven/agent/LaunchTest.java index 62a974dc50..3245a4c58c 100644 --- a/maven-agent/src/test/java/hudson/maven/agent/LaunchTest.java +++ b/maven-agent/src/test/java/hudson/maven/agent/LaunchTest.java @@ -90,7 +90,7 @@ public class LaunchTest extends TestCase { public static void copyStream(InputStream in, OutputStream out) throws IOException { byte[] buf = new byte[8192]; int len; - while((len=in.read(buf))>0) + while((len=in.read(buf))>=0) out.write(buf,0,len); } } diff --git a/remoting/src/main/java/hudson/remoting/BinarySafeStream.java b/remoting/src/main/java/hudson/remoting/BinarySafeStream.java index c777e91bdc..c725c0e7e8 100644 --- a/remoting/src/main/java/hudson/remoting/BinarySafeStream.java +++ b/remoting/src/main/java/hudson/remoting/BinarySafeStream.java @@ -186,7 +186,10 @@ public final class BinarySafeStream { int avail = super.available(); if(avail >0) { byte[] buf = new byte[avail]; - baos.write(buf,0,super.read(buf)); + int len2 = super.read(buf); + if (len2 > 0) { + baos.write(buf,0,len2); + } } StringBuilder buf = new StringBuilder("Invalid encoded sequence encountered:"); for (byte ch : baos.toByteArray()) diff --git a/remoting/src/main/java/hudson/remoting/FastPipedInputStream.java b/remoting/src/main/java/hudson/remoting/FastPipedInputStream.java index 9d55c7efe0..a6f4430a35 100644 --- a/remoting/src/main/java/hudson/remoting/FastPipedInputStream.java +++ b/remoting/src/main/java/hudson/remoting/FastPipedInputStream.java @@ -143,11 +143,11 @@ public class FastPipedInputStream extends InputStream { public int read() throws IOException { byte[] b = new byte[1]; - return read(b, 0, b.length) == -1 ? -1 : (255 & b[0]); + return read(b, 0, b.length) == -1 ? -1 : (255 & b[0]); // FIXME check if read() guarantees to return at least 1 character, otherwise bug lurking } @Override - public int read(byte[] b) throws IOException { + public int read(byte[] b) throws IOException { // FIXME-CHECK-CALLERS return read(b, 0, b.length); } diff --git a/remoting/src/main/java/hudson/remoting/RemoteClassLoader.java b/remoting/src/main/java/hudson/remoting/RemoteClassLoader.java index 84271f370d..1cbe32bdcb 100644 --- a/remoting/src/main/java/hudson/remoting/RemoteClassLoader.java +++ b/remoting/src/main/java/hudson/remoting/RemoteClassLoader.java @@ -453,9 +453,12 @@ final class RemoteClassLoader extends URLClassLoader { byte[] buf = new byte[8192]; int len; - while((len=in.read(buf))>0) - baos.write(buf,0,len); - in.close(); + try { + while((len=in.read(buf))>=0) + baos.write(buf,0,len); + } finally { + in.close(); + } return baos.toByteArray(); } diff --git a/remoting/src/main/java/hudson/remoting/RemoteInputStream.java b/remoting/src/main/java/hudson/remoting/RemoteInputStream.java index 26bb3250c9..41a583b338 100644 --- a/remoting/src/main/java/hudson/remoting/RemoteInputStream.java +++ b/remoting/src/main/java/hudson/remoting/RemoteInputStream.java @@ -68,11 +68,11 @@ public class RemoteInputStream extends InputStream implements Serializable { return core.read(); } - public int read(byte[] b) throws IOException { + public int read(byte[] b) throws IOException { // FIXME-CHECK-CALLERS return core.read(b); } - public int read(byte[] b, int off, int len) throws IOException { + public int read(byte[] b, int off, int len) throws IOException { // FIXME-CHECK-CALLERS return core.read(b, off, len); } diff --git a/remoting/src/main/java/hudson/remoting/forward/CopyThread.java b/remoting/src/main/java/hudson/remoting/forward/CopyThread.java index 08e04001bf..7bc49e2617 100644 --- a/remoting/src/main/java/hudson/remoting/forward/CopyThread.java +++ b/remoting/src/main/java/hudson/remoting/forward/CopyThread.java @@ -24,14 +24,14 @@ final class CopyThread extends Thread { try { byte[] buf = new byte[8192]; int len; - while ((len = in.read(buf)) > 0) + while ((len = in.read(buf)) >= 0) out.write(buf, 0, len); } finally { in.close(); out.close(); } } catch (IOException e) { - // TODO: what to do? + throw new RuntimeException(e); } } } diff --git a/remoting/src/test/java/hudson/remoting/CopyThread.java b/remoting/src/test/java/hudson/remoting/CopyThread.java index db934ec452..fcf13e9388 100644 --- a/remoting/src/test/java/hudson/remoting/CopyThread.java +++ b/remoting/src/test/java/hudson/remoting/CopyThread.java @@ -46,11 +46,14 @@ class Copier extends Thread { try { byte[] buf = new byte[8192]; int len; - while((len=in.read(buf))>0) - out.write(buf,0,len); - in.close(); + try { + while((len=in.read(buf))>=0) + out.write(buf,0,len); + } finally { + in.close(); + } } catch (IOException e) { - // TODO: what to do? + throw new RuntimeException(e); } } } diff --git a/remoting/src/test/java/hudson/remoting/TestCallable.java b/remoting/src/test/java/hudson/remoting/TestCallable.java index 6f0f50516a..22050d74c3 100644 --- a/remoting/src/test/java/hudson/remoting/TestCallable.java +++ b/remoting/src/test/java/hudson/remoting/TestCallable.java @@ -45,9 +45,12 @@ public class TestCallable implements Callable { byte[] buf = new byte[8192]; int len; - while((len=in.read(buf))>0) - baos.write(buf,0,len); - in.close(); + try { + while((len=in.read(buf))>=0) + baos.write(buf,0,len); + } finally { + in.close(); + } r[1] = baos.toByteArray();