From e875c905d310153165c5bdb24ead55d98596923a Mon Sep 17 00:00:00 2001 From: Robin Rosenberg Date: Sun, 15 Jan 2012 11:22:52 +0100 Subject: [PATCH] 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 --- .../eclipse/jgit/junit/TestRepository.java | 11 +-- .../eclipse/jgit/diff/DiffFormatterTest.java | 4 +- .../jgit/storage/file/AbbreviationTest.java | 4 +- .../storage/file/ConcurrentRepackTest.java | 6 +- .../org/eclipse/jgit/dircache/DirCache.java | 4 +- .../src/org/eclipse/jgit/lib/Repository.java | 3 +- .../jgit/storage/file/PackIndexWriter.java | 4 +- .../eclipse/jgit/transport/DaemonClient.java | 4 +- .../jgit/transport/TransportGitAnon.java | 6 +- .../jgit/transport/TransportLocal.java | 6 +- .../jgit/transport/WalkPushConnection.java | 8 +- .../eclipse/jgit/util/TemporaryBuffer.java | 4 +- .../util/io/SafeBufferedOutputStream.java | 85 +++++++++++++++++++ 13 files changed, 119 insertions(+), 30 deletions(-) create mode 100644 org.eclipse.jgit/src/org/eclipse/jgit/util/io/SafeBufferedOutputStream.java diff --git a/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/TestRepository.java b/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/TestRepository.java index eea7811c0..f4a3a6289 100644 --- a/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/TestRepository.java +++ b/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 { 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 { pack.setReadOnly(); idx = nameFor(odb, name, ".idx"); - out = new BufferedOutputStream(new FileOutputStream(idx)); + out = new SafeBufferedOutputStream(new FileOutputStream(idx)); try { pw.writeIndex(out); } finally { diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/diff/DiffFormatterTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/diff/DiffFormatterTest.java index 8dca7b426..a183a1ce7 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/diff/DiffFormatterTest.java +++ b/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()); diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/storage/file/AbbreviationTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/storage/file/AbbreviationTest.java index cfb526d63..c099bb0bd 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/storage/file/AbbreviationTest.java +++ b/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); diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/storage/file/ConcurrentRepackTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/storage/file/ConcurrentRepackTest.java index ba9aa2499..0a5e9feb8 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/storage/file/ConcurrentRepackTest.java +++ b/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 { diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCache.java b/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCache.java index 346b3b35f..394c1a565 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCache.java +++ b/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; diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java index cf74df638..33607ccbd 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java +++ b/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) { diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/PackIndexWriter.java b/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/PackIndexWriter.java index 6bd73adcb..dad559a63 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/PackIndexWriter.java +++ b/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]; } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/DaemonClient.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/DaemonClient.java index 23e3379f9..22f343899 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/DaemonClient.java +++ b/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); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/TransportGitAnon.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/TransportGitAnon.java index dedd1f694..102884a06 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/TransportGitAnon.java +++ b/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); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/TransportLocal.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/TransportLocal.java index 5c4855118..5411f3d69 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/TransportLocal.java +++ b/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(); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/WalkPushConnection.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/WalkPushConnection.java index 81aa4ab2b..985b98e1d 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/WalkPushConnection.java +++ b/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(); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/TemporaryBuffer.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/TemporaryBuffer.java index 8167c776b..5e47fcc01 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/util/TemporaryBuffer.java +++ b/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); } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/io/SafeBufferedOutputStream.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/io/SafeBufferedOutputStream.java new file mode 100644 index 000000000..fa1584105 --- /dev/null +++ b/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. + *

+ * 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. + *

+ */ +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(); + } + } +}