Browse Source

Limit receive commands

Place a configurable upper bound on the amount of command data
received from clients during `git push`.  The limit is applied to the
encoded wire protocol format, not the JGit in-memory representation.
This allows clients to flexibly use the limit; shorter reference names
allow for more commands, longer reference names permit fewer commands
per batch.

Based on data gathered from many repositories at $DAY_JOB, the average
reference name is well under 200 bytes when encoded in UTF-8 (the wire
encoding).  The new 3 MiB default receive.maxCommandBytes allows about
11,155 references in a single `git push` invocation.  A Gerrit Code
Review system with six-digit change numbers could still encode 29,399
references in the 3 MiB maxCommandBytes limit.

Change-Id: I84317d396d25ab1b46820e43ae2b73943646032c
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
stable-4.7
Shawn Pearce 8 years ago committed by Matthias Sohn
parent
commit
0bff481d45
  1. 23
      org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/PushConnectionTest.java
  2. 1
      org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
  3. 1
      org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
  4. 81
      org.eclipse.jgit/src/org/eclipse/jgit/transport/BaseReceivePack.java
  5. 66
      org.eclipse.jgit/src/org/eclipse/jgit/transport/PacketLineIn.java

23
org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/PushConnectionTest.java

@ -173,4 +173,27 @@ public class PushConnectionTest {
} }
} }
} }
@Test
public void limitCommandBytes() throws IOException {
Map<String, RemoteRefUpdate> updates = new HashMap<>();
for (int i = 0; i < 4; i++) {
RemoteRefUpdate rru = new RemoteRefUpdate(
null, null, obj2, "refs/test/T" + i,
false, null, ObjectId.zeroId());
updates.put(rru.getRemoteName(), rru);
}
server.getConfig().setInt("receive", null, "maxCommandBytes", 170);
try (Transport tn = testProtocol.open(uri, client, "server");
PushConnection connection = tn.openPush()) {
try {
connection.push(NullProgressMonitor.INSTANCE, updates);
fail("server did not abort");
} catch (TransportException e) {
String msg = e.getMessage();
assertEquals("remote: Too many commands", msg);
}
}
}
} }

1
org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties

@ -609,6 +609,7 @@ tagOnRepoWithoutHEADCurrentlyNotSupported=Tag on repository without HEAD current
theFactoryMustNotBeNull=The factory must not be null theFactoryMustNotBeNull=The factory must not be null
timeIsUncertain=Time is uncertain timeIsUncertain=Time is uncertain
timerAlreadyTerminated=Timer already terminated timerAlreadyTerminated=Timer already terminated
tooManyCommands=Too many commands
tooManyIncludeRecursions=Too many recursions; circular includes in config file(s)? tooManyIncludeRecursions=Too many recursions; circular includes in config file(s)?
topologicalSortRequired=Topological sort required. topologicalSortRequired=Topological sort required.
transactionAborted=transaction aborted transactionAborted=transaction aborted

1
org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java

@ -669,6 +669,7 @@ public class JGitText extends TranslationBundle {
/***/ public String theFactoryMustNotBeNull; /***/ public String theFactoryMustNotBeNull;
/***/ public String timeIsUncertain; /***/ public String timeIsUncertain;
/***/ public String timerAlreadyTerminated; /***/ public String timerAlreadyTerminated;
/***/ public String tooManyCommands;
/***/ public String tooManyIncludeRecursions; /***/ public String tooManyIncludeRecursions;
/***/ public String topologicalSortRequired; /***/ public String topologicalSortRequired;
/***/ public String transportExceptionBadRef; /***/ public String transportExceptionBadRef;

81
org.eclipse.jgit/src/org/eclipse/jgit/transport/BaseReceivePack.java

@ -97,6 +97,7 @@ import org.eclipse.jgit.revwalk.RevObject;
import org.eclipse.jgit.revwalk.RevSort; import org.eclipse.jgit.revwalk.RevSort;
import org.eclipse.jgit.revwalk.RevTree; import org.eclipse.jgit.revwalk.RevTree;
import org.eclipse.jgit.revwalk.RevWalk; import org.eclipse.jgit.revwalk.RevWalk;
import org.eclipse.jgit.transport.PacketLineIn.InputOverLimitIOException;
import org.eclipse.jgit.transport.ReceiveCommand.Result; import org.eclipse.jgit.transport.ReceiveCommand.Result;
import org.eclipse.jgit.util.io.InterruptTimer; import org.eclipse.jgit.util.io.InterruptTimer;
import org.eclipse.jgit.util.io.LimitedInputStream; import org.eclipse.jgit.util.io.LimitedInputStream;
@ -244,6 +245,8 @@ public abstract class BaseReceivePack {
String userAgent; String userAgent;
private Set<ObjectId> clientShallowCommits; private Set<ObjectId> clientShallowCommits;
private List<ReceiveCommand> commands; private List<ReceiveCommand> commands;
private long maxCommandBytes;
private long maxDiscardBytes;
private StringBuilder advertiseError; private StringBuilder advertiseError;
@ -318,6 +321,8 @@ public abstract class BaseReceivePack {
allowNonFastForwards = rc.allowNonFastForwards; allowNonFastForwards = rc.allowNonFastForwards;
allowOfsDelta = rc.allowOfsDelta; allowOfsDelta = rc.allowOfsDelta;
allowPushOptions = rc.allowPushOptions; allowPushOptions = rc.allowPushOptions;
maxCommandBytes = rc.maxCommandBytes;
maxDiscardBytes = rc.maxDiscardBytes;
advertiseRefsHook = AdvertiseRefsHook.DEFAULT; advertiseRefsHook = AdvertiseRefsHook.DEFAULT;
refFilter = RefFilter.DEFAULT; refFilter = RefFilter.DEFAULT;
advertisedHaves = new HashSet<ObjectId>(); advertisedHaves = new HashSet<ObjectId>();
@ -338,7 +343,8 @@ public abstract class BaseReceivePack {
final boolean allowNonFastForwards; final boolean allowNonFastForwards;
final boolean allowOfsDelta; final boolean allowOfsDelta;
final boolean allowPushOptions; final boolean allowPushOptions;
final long maxCommandBytes;
final long maxDiscardBytes;
final SignedPushConfig signedPush; final SignedPushConfig signedPush;
ReceiveConfig(final Config config) { ReceiveConfig(final Config config) {
@ -350,6 +356,12 @@ public abstract class BaseReceivePack {
true); true);
allowPushOptions = config.getBoolean("receive", "pushoptions", //$NON-NLS-1$ //$NON-NLS-2$ allowPushOptions = config.getBoolean("receive", "pushoptions", //$NON-NLS-1$ //$NON-NLS-2$
false); false);
maxCommandBytes = config.getLong("receive", //$NON-NLS-1$
"maxCommandBytes", //$NON-NLS-1$
3 << 20);
maxDiscardBytes = config.getLong("receive", //$NON-NLS-1$
"maxCommandDiscardBytes", //$NON-NLS-1$
-1);
signedPush = SignedPushConfig.KEY.parse(config); signedPush = SignedPushConfig.KEY.parse(config);
} }
} }
@ -728,6 +740,38 @@ public abstract class BaseReceivePack {
timeout = seconds; timeout = seconds;
} }
/**
* Set the maximum number of command bytes to read from the client.
*
* @param limit
* command limit in bytes; if 0 there is no limit.
* @since 4.7
*/
public void setMaxCommandBytes(long limit) {
maxCommandBytes = limit;
}
/**
* Set the maximum number of command bytes to discard from the client.
* <p>
* Discarding remaining bytes allows this instance to consume the rest of
* the command block and send a human readable over-limit error via the
* side-band channel. If the client sends an excessive number of bytes this
* limit kicks in and the instance disconnects, resulting in a non-specific
* 'pipe closed', 'end of stream', or similar generic error at the client.
* <p>
* When the limit is set to {@code -1} the implementation will default to
* the larger of {@code 3 * maxCommandBytes} or {@code 3 MiB}.
*
* @param limit
* discard limit in bytes; if 0 there is no limit; if -1 the
* implementation tries to set a reasonable default.
* @since 4.7
*/
public void setMaxCommandDiscardBytes(long limit) {
maxDiscardBytes = limit;
}
/** /**
* Set the maximum allowed Git object size. * Set the maximum allowed Git object size.
* <p> * <p>
@ -741,7 +785,6 @@ public abstract class BaseReceivePack {
maxObjectSizeLimit = limit; maxObjectSizeLimit = limit;
} }
/** /**
* Set the maximum allowed pack size. * Set the maximum allowed pack size.
* <p> * <p>
@ -1134,13 +1177,16 @@ public abstract class BaseReceivePack {
* @throws IOException * @throws IOException
*/ */
protected void recvCommands() throws IOException { protected void recvCommands() throws IOException {
PacketLineIn pck = maxCommandBytes > 0
? new PacketLineIn(rawIn, maxCommandBytes)
: pckIn;
PushCertificateParser certParser = getPushCertificateParser(); PushCertificateParser certParser = getPushCertificateParser();
boolean firstPkt = true; boolean firstPkt = true;
try { try {
for (;;) { for (;;) {
String line; String line;
try { try {
line = pckIn.readString(); line = pck.readString();
} catch (EOFException eof) { } catch (EOFException eof) {
if (commands.isEmpty()) if (commands.isEmpty())
return; return;
@ -1163,13 +1209,13 @@ public abstract class BaseReceivePack {
enableCapabilities(); enableCapabilities();
if (line.equals(GitProtocolConstants.OPTION_PUSH_CERT)) { if (line.equals(GitProtocolConstants.OPTION_PUSH_CERT)) {
certParser.receiveHeader(pckIn, !isBiDirectionalPipe()); certParser.receiveHeader(pck, !isBiDirectionalPipe());
continue; continue;
} }
} }
if (line.equals(PushCertificateParser.BEGIN_SIGNATURE)) { if (line.equals(PushCertificateParser.BEGIN_SIGNATURE)) {
certParser.receiveSignature(pckIn); certParser.receiveSignature(pck);
continue; continue;
} }
@ -1186,19 +1232,32 @@ public abstract class BaseReceivePack {
} }
pushCert = certParser.build(); pushCert = certParser.build();
if (hasCommands()) { if (hasCommands()) {
readPostCommands(pckIn); readPostCommands(pck);
} }
} catch (PackProtocolException e) { } catch (PackProtocolException e) {
discardCommands();
fatalError(e.getMessage());
throw e;
} catch (InputOverLimitIOException e) {
String msg = JGitText.get().tooManyCommands;
discardCommands();
fatalError(msg);
throw new PackProtocolException(msg);
}
}
private void discardCommands() {
if (sideBand) { if (sideBand) {
long max = maxDiscardBytes;
if (max < 0) {
max = Math.max(3 * maxCommandBytes, 3L << 20);
}
try { try {
pckIn.discardUntilEnd(); new PacketLineIn(rawIn, max).discardUntilEnd();
} catch (IOException e2) { } catch (IOException e) {
// Ignore read failures attempting to discard. // Ignore read failures attempting to discard.
} }
} }
fatalError(e.getMessage());
throw e;
}
} }
private void parseShallow(String idStr) throws PackProtocolException { private void parseShallow(String idStr) throws PackProtocolException {

66
org.eclipse.jgit/src/org/eclipse/jgit/transport/PacketLineIn.java

@ -87,19 +87,32 @@ public class PacketLineIn {
ACK_READY; ACK_READY;
} }
private final byte[] lineBuffer = new byte[SideBandOutputStream.SMALL_BUF];
private final InputStream in; private final InputStream in;
private long limit;
private final byte[] lineBuffer; /**
* Create a new packet line reader.
*
* @param in
* the input stream to consume.
*/
public PacketLineIn(InputStream in) {
this(in, 0);
}
/** /**
* Create a new packet line reader. * Create a new packet line reader.
* *
* @param i * @param in
* the input stream to consume. * the input stream to consume.
* @param limit
* bytes to read from the input; unlimited if set to 0.
* @since 4.7
*/ */
public PacketLineIn(final InputStream i) { public PacketLineIn(InputStream in, long limit) {
in = i; this.in = in;
lineBuffer = new byte[SideBandOutputStream.SMALL_BUF]; this.limit = limit;
} }
AckNackResult readACK(final MutableObjectId returnedId) throws IOException { AckNackResult readACK(final MutableObjectId returnedId) throws IOException {
@ -210,15 +223,48 @@ public class PacketLineIn {
int readLength() throws IOException { int readLength() throws IOException {
IO.readFully(in, lineBuffer, 0, 4); IO.readFully(in, lineBuffer, 0, 4);
int len;
try { try {
final int len = RawParseUtils.parseHexInt16(lineBuffer, 0); len = RawParseUtils.parseHexInt16(lineBuffer, 0);
if (len != 0 && len < 4)
throw new ArrayIndexOutOfBoundsException();
return len;
} catch (ArrayIndexOutOfBoundsException err) { } catch (ArrayIndexOutOfBoundsException err) {
throw new IOException(MessageFormat.format(JGitText.get().invalidPacketLineHeader, throw invalidHeader();
}
if (len == 0) {
return 0;
} else if (len < 4) {
throw invalidHeader();
}
if (limit != 0) {
int n = len - 4;
if (limit < n) {
limit = -1;
try {
IO.skipFully(in, n);
} catch (IOException e) {
// Ignore failure discarding packet over limit.
}
throw new InputOverLimitIOException();
}
// if set limit must not be 0 (means unlimited).
limit = n < limit ? limit - n : -1;
}
return len;
}
private IOException invalidHeader() {
return new IOException(MessageFormat.format(JGitText.get().invalidPacketLineHeader,
"" + (char) lineBuffer[0] + (char) lineBuffer[1] //$NON-NLS-1$ "" + (char) lineBuffer[0] + (char) lineBuffer[1] //$NON-NLS-1$
+ (char) lineBuffer[2] + (char) lineBuffer[3])); + (char) lineBuffer[2] + (char) lineBuffer[3]));
} }
/**
* IOException thrown by read when the configured input limit is exceeded.
*
* @since 4.7
*/
public static class InputOverLimitIOException extends IOException {
private static final long serialVersionUID = 1L;
} }
} }

Loading…
Cancel
Save