From 07fdc50c0727674ac0c7f27394b81654f73abc2f Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Mon, 20 Feb 2017 10:51:27 -0800 Subject: [PATCH] Fix bad test fix from 0bff481 "Limit receive commands" In 0bff481d45db74db81a3b1b86f7401443a60d970 to accurately use the two limits it was necessary to move the LimitedInputStream out of the PacketLineIn and further down to the PackParser. Unfortuantely this didn't survive review, as a buggy test failed and the "fix" was to drop this part of the code. The maxPackSizeLimit should apply to the pack stream, not the pkt-line framing used to send commands to control the ReceivePack instance. The commands are controlled using a different limit. The failing test allowed too many bytes in the pack and was only failing because it was including the command framing. The correct fix for the test was simply to drop the limit lower, to more closely match the actual pack size. Change-Id: I47d3885b9d7d527e153df7ac9c62fc2865ceecf4 --- .../http/test/GitServletResponseTests.java | 2 +- .../jgit/transport/BaseReceivePack.java | 23 +++++++++++-------- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/GitServletResponseTests.java b/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/GitServletResponseTests.java index 1c953084d..de7891c44 100644 --- a/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/GitServletResponseTests.java +++ b/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/GitServletResponseTests.java @@ -269,7 +269,7 @@ public class GitServletResponseTests extends HttpTestCase { Transport t; // this maxPackSize leads to an unPackError - maxPackSize = 400; + maxPackSize = 100; // this PostReceiveHook when called after an unsuccesfull unpack will // lead to an IllegalStateException postHook = new PostReceiveHook() { diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/BaseReceivePack.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/BaseReceivePack.java index 080fbd643..a7e72f1de 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/BaseReceivePack.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/BaseReceivePack.java @@ -1058,14 +1058,6 @@ public abstract class BaseReceivePack { rawOut = o; } - if (maxPackSizeLimit >= 0) - rawIn = new LimitedInputStream(rawIn, maxPackSizeLimit) { - @Override - protected void limitExceeded() throws TooLargePackException { - throw new TooLargePackException(limit); - } - }; - pckIn = new PacketLineIn(rawIn); pckOut = new PacketLineOut(rawOut); pckOut.setFlushOnEnd(false); @@ -1369,7 +1361,7 @@ public abstract class BaseReceivePack { if (getRefLogIdent() != null) lockMsg += " from " + getRefLogIdent().toExternalString(); //$NON-NLS-1$ - parser = ins.newPackParser(rawIn); + parser = ins.newPackParser(packInputStream()); parser.setAllowThin(true); parser.setNeedNewObjectIds(checkReferencedIsReachable); parser.setNeedBaseObjectIds(checkReferencedIsReachable); @@ -1389,6 +1381,19 @@ public abstract class BaseReceivePack { timeoutIn.setTimeout(timeout * 1000); } + private InputStream packInputStream() { + InputStream packIn = rawIn; + if (maxPackSizeLimit >= 0) { + packIn = new LimitedInputStream(packIn, maxPackSizeLimit) { + @Override + protected void limitExceeded() throws TooLargePackException { + throw new TooLargePackException(limit); + } + }; + } + return packIn; + } + private boolean needCheckConnectivity() { return isCheckReceivedObjects() || isCheckReferencedObjectsAreReachable()