Make sure all bytes are written to files on close, or get an error.

Java's BufferedOutputStream swallows any errors that occur when flushing
the buffer in close().

This class overrides close to make sure an error during the final
flush is reported back to the caller.

Change-Id: I74a82b31505fadf8378069c5f6554f1033c28f9b
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>

org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/TestRepository.java

@@ -43,9 +43,9 @@
 
 package org.eclipse.jgit.junit;
 
-import static org.junit.Assert.*;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
 
-import java.io.BufferedOutputStream;
 import java.io.File;
 import java.io.FileOutputStream;
 import java.io.IOException;
@@ -61,10 +61,10 @@ import java.util.Set;
 import org.eclipse.jgit.dircache.DirCache;
 import org.eclipse.jgit.dircache.DirCacheBuilder;
 import org.eclipse.jgit.dircache.DirCacheEditor;
-import org.eclipse.jgit.dircache.DirCacheEntry;
 import org.eclipse.jgit.dircache.DirCacheEditor.DeletePath;
 import org.eclipse.jgit.dircache.DirCacheEditor.DeleteTree;
 import org.eclipse.jgit.dircache.DirCacheEditor.PathEdit;
+import org.eclipse.jgit.dircache.DirCacheEntry;
 import org.eclipse.jgit.errors.IncorrectObjectTypeException;
 import org.eclipse.jgit.errors.MissingObjectException;
 import org.eclipse.jgit.errors.ObjectWritingException;
@@ -97,6 +97,7 @@ import org.eclipse.jgit.storage.pack.PackWriter;
 import org.eclipse.jgit.treewalk.TreeWalk;
 import org.eclipse.jgit.treewalk.filter.PathFilterGroup;
 import org.eclipse.jgit.util.FileUtils;
+import org.eclipse.jgit.util.io.SafeBufferedOutputStream;
 
 /**
  * Wrapper to make creating test data easier.
@@ -623,7 +624,7 @@ public class TestRepository<R extends Repository> {
 				OutputStream out;
 
 				pack = nameFor(odb, name, ".pack");
-				out = new BufferedOutputStream(new FileOutputStream(pack));
+				out = new SafeBufferedOutputStream(new FileOutputStream(pack));
 				try {
 					pw.writePack(m, m, out);
 				} finally {
@@ -632,7 +633,7 @@ public class TestRepository<R extends Repository> {
 				pack.setReadOnly();
 
 				idx = nameFor(odb, name, ".idx");
-				out = new BufferedOutputStream(new FileOutputStream(idx));
+				out = new SafeBufferedOutputStream(new FileOutputStream(idx));
 				try {
 					pw.writeIndex(out);
 				} finally {

org.eclipse.jgit.test/tst/org/eclipse/jgit/diff/DiffFormatterTest.java

@@ -45,7 +45,6 @@ package org.eclipse.jgit.diff;
 
 import static org.junit.Assert.assertEquals;
 
-import java.io.BufferedOutputStream;
 import java.io.ByteArrayOutputStream;
 import java.io.File;
 
@@ -62,6 +61,7 @@ import org.eclipse.jgit.treewalk.FileTreeIterator;
 import org.eclipse.jgit.treewalk.filter.PathFilter;
 import org.eclipse.jgit.util.RawParseUtils;
 import org.eclipse.jgit.util.io.DisabledOutputStream;
+import org.eclipse.jgit.util.io.SafeBufferedOutputStream;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
@@ -267,7 +267,7 @@ public class DiffFormatterTest extends RepositoryTestCase {
 		write(new File(folder, "folder.txt"), "folder change");
 
 		ByteArrayOutputStream os = new ByteArrayOutputStream();
-		DiffFormatter df = new DiffFormatter(new BufferedOutputStream(os));
+		DiffFormatter df = new DiffFormatter(new SafeBufferedOutputStream(os));
 		df.setRepository(db);
 		df.setPathFilter(PathFilter.create("folder"));
 		DirCacheIterator oldTree = new DirCacheIterator(db.readDirCache());

org.eclipse.jgit.test/tst/org/eclipse/jgit/storage/file/AbbreviationTest.java

@@ -50,7 +50,6 @@ import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
-import java.io.BufferedOutputStream;
 import java.io.ByteArrayOutputStream;
 import java.io.File;
 import java.io.FileOutputStream;
@@ -69,6 +68,7 @@ import org.eclipse.jgit.lib.ObjectReader;
 import org.eclipse.jgit.revwalk.RevBlob;
 import org.eclipse.jgit.transport.PackedObjectInfo;
 import org.eclipse.jgit.util.FileUtils;
+import org.eclipse.jgit.util.io.SafeBufferedOutputStream;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
@@ -178,7 +178,7 @@ public class AbbreviationTest extends LocalDiskRepositoryTestCase {
 		File idxFile = new File(packDir, packName + ".idx");
 		File packFile = new File(packDir, packName + ".pack");
 		FileUtils.mkdir(packDir, true);
-		OutputStream dst = new BufferedOutputStream(new FileOutputStream(
+		OutputStream dst = new SafeBufferedOutputStream(new FileOutputStream(
 				idxFile));
 		try {
 			PackIndexWriter writer = new PackIndexWriterV2(dst);

org.eclipse.jgit.test/tst/org/eclipse/jgit/storage/file/ConcurrentRepackTest.java

@@ -51,7 +51,6 @@ import static org.junit.Assert.assertNotSame;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
-import java.io.BufferedOutputStream;
 import java.io.File;
 import java.io.FileOutputStream;
 import java.io.IOException;
@@ -72,6 +71,7 @@ import org.eclipse.jgit.revwalk.RevObject;
 import org.eclipse.jgit.revwalk.RevWalk;
 import org.eclipse.jgit.storage.pack.PackWriter;
 import org.eclipse.jgit.util.FileUtils;
+import org.eclipse.jgit.util.io.SafeBufferedOutputStream;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
@@ -236,14 +236,14 @@ public class ConcurrentRepackTest extends RepositoryTestCase {
 		NullProgressMonitor m = NullProgressMonitor.INSTANCE;
 		OutputStream out;
 
-		out = new BufferedOutputStream(new FileOutputStream(files[0]));
+		out = new SafeBufferedOutputStream(new FileOutputStream(files[0]));
 		try {
 			pw.writePack(m, m, out);
 		} finally {
 			out.close();
 		}
 
-		out = new BufferedOutputStream(new FileOutputStream(files[1]));
+		out = new SafeBufferedOutputStream(new FileOutputStream(files[1]));
 		try {
 			pw.writeIndex(out);
 		} finally {

org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCache.java

@@ -46,7 +46,6 @@
 package org.eclipse.jgit.dircache;
 
 import java.io.BufferedInputStream;
-import java.io.BufferedOutputStream;
 import java.io.EOFException;
 import java.io.File;
 import java.io.FileInputStream;
@@ -77,6 +76,7 @@ import org.eclipse.jgit.util.IO;
 import org.eclipse.jgit.util.MutableInteger;
 import org.eclipse.jgit.util.NB;
 import org.eclipse.jgit.util.TemporaryBuffer;
+import org.eclipse.jgit.util.io.SafeBufferedOutputStream;
 
 /**
  * Support for the Git dircache (aka index file).
@@ -536,7 +536,7 @@ public class DirCache {
 		final LockFile tmp = myLock;
 		requireLocked(tmp);
 		try {
-			writeTo(new BufferedOutputStream(tmp.getOutputStream()));
+			writeTo(new SafeBufferedOutputStream(tmp.getOutputStream()));
 		} catch (IOException err) {
 			tmp.unlock();
 			throw err;

org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java

@@ -86,6 +86,7 @@ import org.eclipse.jgit.util.FS;
 import org.eclipse.jgit.util.FileUtils;
 import org.eclipse.jgit.util.IO;
 import org.eclipse.jgit.util.RawParseUtils;
+import org.eclipse.jgit.util.io.SafeBufferedOutputStream;
 
 /**
  * Represents a Git repository.
@@ -1277,7 +1278,7 @@ public abstract class Repository {
 			throws FileNotFoundException, IOException {
 		File headsFile = new File(getDirectory(), filename);
 		if (heads != null) {
-			BufferedOutputStream bos = new BufferedOutputStream(
+			BufferedOutputStream bos = new SafeBufferedOutputStream(
 					new FileOutputStream(headsFile));
 			try {
 				for (ObjectId id : heads) {

org.eclipse.jgit/src/org/eclipse/jgit/storage/file/PackIndexWriter.java

@@ -56,6 +56,7 @@ import org.eclipse.jgit.lib.Constants;
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.transport.PackedObjectInfo;
 import org.eclipse.jgit.util.NB;
+import org.eclipse.jgit.util.io.SafeBufferedOutputStream;
 
 /**
  * Creates a table of contents to support random access by {@link PackFile}.
@@ -161,7 +162,8 @@ public abstract class PackIndexWriter {
 	 */
 	protected PackIndexWriter(final OutputStream dst) {
 		out = new DigestOutputStream(dst instanceof BufferedOutputStream ? dst
-				: new BufferedOutputStream(dst), Constants.newMessageDigest());
+				: new SafeBufferedOutputStream(dst),
+				Constants.newMessageDigest());
 		tmp = new byte[4 + Constants.OBJECT_ID_LENGTH];
 	}
 

org.eclipse.jgit/src/org/eclipse/jgit/transport/DaemonClient.java

@@ -44,7 +44,6 @@
 package org.eclipse.jgit.transport;
 
 import java.io.BufferedInputStream;
-import java.io.BufferedOutputStream;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
@@ -53,6 +52,7 @@ import java.net.Socket;
 
 import org.eclipse.jgit.transport.resolver.ServiceNotAuthorizedException;
 import org.eclipse.jgit.transport.resolver.ServiceNotEnabledException;
+import org.eclipse.jgit.util.io.SafeBufferedOutputStream;
 
 /** Active network client of {@link Daemon}. */
 public class DaemonClient {
@@ -95,7 +95,7 @@ public class DaemonClient {
 	void execute(final Socket sock) throws IOException,
 			ServiceNotEnabledException, ServiceNotAuthorizedException {
 		rawIn = new BufferedInputStream(sock.getInputStream());
-		rawOut = new BufferedOutputStream(sock.getOutputStream());
+		rawOut = new SafeBufferedOutputStream(sock.getOutputStream());
 
 		if (0 < daemon.getTimeout())
 			sock.setSoTimeout(daemon.getTimeout() * 1000);

org.eclipse.jgit/src/org/eclipse/jgit/transport/TransportGitAnon.java

@@ -46,7 +46,6 @@
 package org.eclipse.jgit.transport;
 
 import java.io.BufferedInputStream;
-import java.io.BufferedOutputStream;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
@@ -63,6 +62,7 @@ import org.eclipse.jgit.JGitText;
 import org.eclipse.jgit.errors.NotSupportedException;
 import org.eclipse.jgit.errors.TransportException;
 import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.util.io.SafeBufferedOutputStream;
 
 /**
  * Transport through a git-daemon waiting for anonymous TCP connections.
@@ -173,7 +173,7 @@ class TransportGitAnon extends TcpTransport implements PackTransport {
 				OutputStream sOut = sock.getOutputStream();
 
 				sIn = new BufferedInputStream(sIn);
-				sOut = new BufferedOutputStream(sOut);
+				sOut = new SafeBufferedOutputStream(sOut);
 
 				init(sIn, sOut);
 				service("git-upload-pack", pckOut);
@@ -212,7 +212,7 @@ class TransportGitAnon extends TcpTransport implements PackTransport {
 				OutputStream sOut = sock.getOutputStream();
 
 				sIn = new BufferedInputStream(sIn);
-				sOut = new BufferedOutputStream(sOut);
+				sOut = new SafeBufferedOutputStream(sOut);
 
 				init(sIn, sOut);
 				service("git-receive-pack", pckOut);

org.eclipse.jgit/src/org/eclipse/jgit/transport/TransportLocal.java

@@ -48,7 +48,6 @@
 package org.eclipse.jgit.transport;
 
 import java.io.BufferedInputStream;
-import java.io.BufferedOutputStream;
 import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
@@ -67,6 +66,7 @@ import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.lib.RepositoryCache;
 import org.eclipse.jgit.storage.file.FileRepository;
 import org.eclipse.jgit.util.io.MessageWriter;
+import org.eclipse.jgit.util.io.SafeBufferedOutputStream;
 import org.eclipse.jgit.util.io.StreamCopyThread;
 
 /**
@@ -301,7 +301,7 @@ class TransportLocal extends Transport implements PackTransport {
 			OutputStream upOut = uploadPack.getOutputStream();
 
 			upIn = new BufferedInputStream(upIn);
-			upOut = new BufferedOutputStream(upOut);
+			upOut = new SafeBufferedOutputStream(upOut);
 
 			init(upIn, upOut);
 			readAdvertisedRefs();
@@ -431,7 +431,7 @@ class TransportLocal extends Transport implements PackTransport {
 			OutputStream rpOut = receivePack.getOutputStream();
 
 			rpIn = new BufferedInputStream(rpIn);
-			rpOut = new BufferedOutputStream(rpOut);
+			rpOut = new SafeBufferedOutputStream(rpOut);
 
 			init(rpIn, rpOut);
 			readAdvertisedRefs();

org.eclipse.jgit/src/org/eclipse/jgit/transport/WalkPushConnection.java

@@ -45,7 +45,6 @@ package org.eclipse.jgit.transport;
 
 import static org.eclipse.jgit.transport.WalkRemoteObjectDatabase.ROOT_DIR;
 
-import java.io.BufferedOutputStream;
 import java.io.IOException;
 import java.io.OutputStream;
 import java.util.ArrayList;
@@ -65,11 +64,12 @@ import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.ObjectIdRef;
 import org.eclipse.jgit.lib.ProgressMonitor;
 import org.eclipse.jgit.lib.Ref;
+import org.eclipse.jgit.lib.Ref.Storage;
 import org.eclipse.jgit.lib.RefWriter;
 import org.eclipse.jgit.lib.Repository;
-import org.eclipse.jgit.lib.Ref.Storage;
 import org.eclipse.jgit.storage.pack.PackWriter;
 import org.eclipse.jgit.transport.RemoteRefUpdate.Status;
+import org.eclipse.jgit.util.io.SafeBufferedOutputStream;
 
 /**
  * Generic push support for dumb transport protocols.
@@ -259,7 +259,7 @@ class WalkPushConnection extends BaseConnection implements PushConnection {
 			final String wt = "Put " + base.substring(0, 12);
 			OutputStream os = dest.writeFile(pathPack, monitor, wt + "..pack");
 			try {
-				os = new BufferedOutputStream(os);
+				os = new SafeBufferedOutputStream(os);
 				writer.writePack(monitor, monitor, os);
 			} finally {
 				os.close();
@@ -267,7 +267,7 @@ class WalkPushConnection extends BaseConnection implements PushConnection {
 
 			os = dest.writeFile(pathIdx, monitor, wt + "..idx");
 			try {
-				os = new BufferedOutputStream(os);
+				os = new SafeBufferedOutputStream(os);
 				writer.writeIndex(os);
 			} finally {
 				os.close();

org.eclipse.jgit/src/org/eclipse/jgit/util/TemporaryBuffer.java

@@ -44,7 +44,6 @@
 
 package org.eclipse.jgit.util;
 
-import java.io.BufferedOutputStream;
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.FileOutputStream;
@@ -56,6 +55,7 @@ import java.util.ArrayList;
 import org.eclipse.jgit.JGitText;
 import org.eclipse.jgit.lib.NullProgressMonitor;
 import org.eclipse.jgit.lib.ProgressMonitor;
+import org.eclipse.jgit.util.io.SafeBufferedOutputStream;
 
 /**
  * A fully buffered output stream.
@@ -309,7 +309,7 @@ public abstract class TemporaryBuffer extends OutputStream {
 			overflow.write(b.buffer, 0, b.count);
 		blocks = null;
 
-		overflow = new BufferedOutputStream(overflow, Block.SZ);
+		overflow = new SafeBufferedOutputStream(overflow, Block.SZ);
 		overflow.write(last.buffer, 0, last.count);
 	}
 

org.eclipse.jgit/src/org/eclipse/jgit/util/io/SafeBufferedOutputStream.java

@@ -0,0 +1,85 @@
+/*
+ * Copyright (C) 2011, Robin Rosenberg
+ * and other copyright owners as documented in the project's IP log.
+ *
+ * This program and the accompanying materials are made available
+ * under the terms of the Eclipse Distribution License v1.0 which
+ * accompanies this distribution, is reproduced below, and is
+ * available at http://www.eclipse.org/org/documents/edl-v10.php
+ *
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * - Redistributions of source code must retain the above copyright
+ *   notice, this list of conditions and the following disclaimer.
+ *
+ * - Redistributions in binary form must reproduce the above
+ *   copyright notice, this list of conditions and the following
+ *   disclaimer in the documentation and/or other materials provided
+ *   with the distribution.
+ *
+ * - Neither the name of the Eclipse Foundation, Inc. nor the
+ *   names of its contributors may be used to endorse or promote
+ *   products derived from this software without specific prior
+ *   written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
+ * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+package org.eclipse.jgit.util.io;
+
+import java.io.BufferedOutputStream;
+import java.io.IOException;
+import java.io.OutputStream;
+
+/**
+ * A BufferedOutputStream that throws an error if the final flush fails on
+ * close.
+ * <p>
+ * Java's BufferedOutputStream swallows errors that occur when the output stream
+ * tries to write the final bytes to the output during close. This may result in
+ * corrupted files without notice.
+ * </p>
+ */
+public class SafeBufferedOutputStream extends BufferedOutputStream {
+
+	/**
+	 * @see BufferedOutputStream#BufferedOutputStream(OutputStream)
+	 * @param out
+	 */
+	public SafeBufferedOutputStream(OutputStream out) {
+		super(out);
+	}
+
+	/**
+	 * @see BufferedOutputStream#BufferedOutputStream(OutputStream, int)
+	 * @param out
+	 * @param size
+	 */
+	public SafeBufferedOutputStream(OutputStream out, int size) {
+		super(out);
+	}
+
+	@Override
+	public void close() throws IOException {
+		try {
+			flush();
+		} finally {
+			super.close();
+		}
+	}
+}