If the client sent a well-formed enough request to see it wants to use side-band-64k for status reporting (meaning its a modern client), but any other command record was somehow invalid (e.g. corrupt SHA-1) report the parsing exception using channel 3. This allows clients to see the failure and know the server will not be continuing. git-core and JGit clients send all commands and then start a sideband demux before sending the pack. By consuming all commands first we get the client into a state where it can see and respond to the channel 3 server failure. This behavior is useful on HTTPS connections when the client is buggy and sent a corrupt command, but still managed to request side-band-64k in the first line. Change-Id: If385b91ceb9f024ccae2d1645caf15bc6b206130tags/v4.5.0.201609210915-r
@@ -43,13 +43,18 @@ | |||
package org.eclipse.jgit.transport; | |||
import static org.eclipse.jgit.transport.BasePackPushConnection.CAPABILITY_REPORT_STATUS; | |||
import static org.eclipse.jgit.transport.BasePackPushConnection.CAPABILITY_SIDE_BAND_64K; | |||
import static org.eclipse.jgit.transport.RemoteRefUpdate.Status.REJECTED_OTHER_REASON; | |||
import static org.junit.Assert.assertEquals; | |||
import static org.junit.Assert.fail; | |||
import java.io.IOException; | |||
import java.io.StringWriter; | |||
import java.util.HashMap; | |||
import java.util.Map; | |||
import org.eclipse.jgit.errors.TransportException; | |||
import org.eclipse.jgit.internal.storage.dfs.DfsRepositoryDescription; | |||
import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository; | |||
import org.eclipse.jgit.lib.Constants; | |||
@@ -133,4 +138,39 @@ public class PushConnectionTest { | |||
assertEquals(REJECTED_OTHER_REASON, rru.getStatus()); | |||
assertEquals("invalid old id sent", rru.getMessage()); | |||
} | |||
@Test | |||
public void invalidCommand() throws IOException { | |||
try (Transport tn = testProtocol.open(uri, client, "server"); | |||
InternalPushConnection c = (InternalPushConnection) tn.openPush()) { | |||
StringWriter msgs = new StringWriter(); | |||
PacketLineOut pckOut = c.pckOut; | |||
@SuppressWarnings("resource") | |||
SideBandInputStream in = new SideBandInputStream(c.in, | |||
NullProgressMonitor.INSTANCE, msgs, null); | |||
// Explicitly invalid command, but sane enough capabilities. | |||
StringBuilder buf = new StringBuilder(); | |||
buf.append("42"); | |||
buf.append(' '); | |||
buf.append(obj2.name()); | |||
buf.append(' '); | |||
buf.append("refs/heads/A" + obj2.name()); | |||
buf.append('\0').append(CAPABILITY_SIDE_BAND_64K); | |||
buf.append(' ').append(CAPABILITY_REPORT_STATUS); | |||
buf.append('\n'); | |||
pckOut.writeString(buf.toString()); | |||
pckOut.end(); | |||
try { | |||
in.read(); | |||
fail("expected TransportException"); | |||
} catch (TransportException e) { | |||
assertEquals( | |||
"remote: error: invalid protocol: wanted 'old new ref'", | |||
e.getMessage()); | |||
} | |||
} | |||
} | |||
} |
@@ -51,6 +51,7 @@ import static org.eclipse.jgit.transport.GitProtocolConstants.CAPABILITY_REPORT_ | |||
import static org.eclipse.jgit.transport.GitProtocolConstants.CAPABILITY_SIDE_BAND_64K; | |||
import static org.eclipse.jgit.transport.GitProtocolConstants.OPTION_AGENT; | |||
import static org.eclipse.jgit.transport.SideBandOutputStream.CH_DATA; | |||
import static org.eclipse.jgit.transport.SideBandOutputStream.CH_ERROR; | |||
import static org.eclipse.jgit.transport.SideBandOutputStream.CH_PROGRESS; | |||
import static org.eclipse.jgit.transport.SideBandOutputStream.MAX_BUF; | |||
@@ -215,6 +216,7 @@ public abstract class BaseReceivePack { | |||
/** Optional message output stream. */ | |||
protected OutputStream msgOut; | |||
private SideBandOutputStream errOut; | |||
/** Packet line input stream around {@link #rawIn}. */ | |||
protected PacketLineIn pckIn; | |||
@@ -879,6 +881,19 @@ public abstract class BaseReceivePack { | |||
} | |||
} | |||
private void fatalError(String msg) { | |||
if (errOut != null) { | |||
try { | |||
errOut.write(Constants.encode(msg)); | |||
errOut.flush(); | |||
} catch (IOException e) { | |||
// Ignore write failures | |||
} | |||
} else { | |||
sendError(msg); | |||
} | |||
} | |||
/** | |||
* Send a message to the client, if it supports receiving them. | |||
* <p> | |||
@@ -1128,7 +1143,14 @@ public abstract class BaseReceivePack { | |||
} | |||
pushCert = certParser.build(); | |||
} catch (PackProtocolException e) { | |||
sendError(e.getMessage()); | |||
if (sideBand) { | |||
try { | |||
pckIn.discardUntilEnd(); | |||
} catch (IOException e2) { | |||
// Ignore read failures attempting to discard. | |||
} | |||
} | |||
fatalError(e.getMessage()); | |||
throw e; | |||
} | |||
} | |||
@@ -1175,6 +1197,7 @@ public abstract class BaseReceivePack { | |||
rawOut = new SideBandOutputStream(CH_DATA, MAX_BUF, out); | |||
msgOut = new SideBandOutputStream(CH_PROGRESS, MAX_BUF, out); | |||
errOut = new SideBandOutputStream(CH_ERROR, MAX_BUF, out); | |||
pckOut = new PacketLineOut(rawOut); | |||
pckOut.setFlushOnEnd(false); |
@@ -182,6 +182,16 @@ public class PacketLineIn { | |||
return RawParseUtils.decode(Constants.CHARSET, raw, 0, len); | |||
} | |||
void discardUntilEnd() throws IOException { | |||
for (;;) { | |||
int n = readLength(); | |||
if (n == 0) { | |||
break; | |||
} | |||
IO.skipFully(in, n - 4); | |||
} | |||
} | |||
int readLength() throws IOException { | |||
IO.readFully(in, lineBuffer, 0, 4); | |||
try { |