Skip to content

Commit d841804

Browse files
zakharovsergey1000msohn
authored andcommitted
push: Report fatal server errors during pack writing
If the push client has requested side-band support the server can signal a fatal error parsing the pack using the error channel (3) and then hang up. This may cause the PackWriter to fail to write data onto the network socket, which throws a misleading error back up to the application and the user. During a write failure poll the input to see if the side band system can parse out an error message off channel 3. This should be fast as there will either be an error present in the buffer, or the remote will also have hung-up on the side band channel. In the case of a hang-up just rethrow the original IOException as it's a network error. Enable ignored PushConnectionTests this change is fixing. Change-Id: Ie6ca43b39f3ef5727aba9251019e587006144376
1 parent d54cb2f commit d841804

2 files changed

Lines changed: 37 additions & 21 deletions

File tree

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

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@
3939
import org.eclipse.jgit.lib.Repository;
4040
import org.junit.After;
4141
import org.junit.Before;
42-
import org.junit.Ignore;
4342
import org.junit.Test;
4443

4544
public class PushConnectionTest {
@@ -155,7 +154,6 @@ public void invalidCommand() throws IOException {
155154
}
156155

157156
@Test
158-
@Ignore
159157
public void limitCommandBytes() throws IOException {
160158
Map<String, RemoteRefUpdate> updates = new HashMap<>();
161159
for (int i = 0; i < 4; i++) {
@@ -181,7 +179,6 @@ public void limitCommandBytes() throws IOException {
181179
}
182180

183181
@Test
184-
@Ignore
185182
public void limitPackSize() throws IOException {
186183
// this maxPackSize leads to an unPackError
187184
maxPackSize = 1;

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

Lines changed: 37 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -172,38 +172,57 @@ protected TransportException noRepository(Throwable cause) {
172172
* if any exception occurs.
173173
* @since 3.0
174174
*/
175+
@SuppressWarnings("Finally")
175176
protected void doPush(final ProgressMonitor monitor,
176177
final Map<String, RemoteRefUpdate> refUpdates,
177178
OutputStream outputStream) throws TransportException {
179+
Throwable suppressedThrowable = null;
178180
try {
179181
writeCommands(refUpdates.values(), monitor, outputStream);
180182

181183
if (pushOptions != null && capablePushOptions)
182184
transmitOptions();
183185
if (writePack)
184186
writePack(refUpdates, monitor);
185-
if (sentCommand) {
186-
if (capableReport)
187-
readStatusReport(refUpdates);
188-
if (capableSideBand) {
189-
// Ensure the data channel is at EOF, so we know we have
190-
// read all side-band data from all channels and have a
191-
// complete copy of the messages (if any) buffered from
192-
// the other data channels.
193-
//
194-
int b = in.read();
195-
if (0 <= b) {
196-
throw new TransportException(uri, MessageFormat.format(
197-
JGitText.get().expectedEOFReceived,
198-
Character.valueOf((char) b)));
199-
}
200-
}
201-
}
202187
} catch (TransportException e) {
188+
suppressedThrowable = e;
203189
throw e;
204190
} catch (Exception e) {
205-
throw new TransportException(uri, e.getMessage(), e);
191+
TransportException transportException = new TransportException(uri, e.getMessage(), e);
192+
suppressedThrowable = transportException;
193+
throw transportException;
206194
} finally {
195+
try {
196+
if (sentCommand) {
197+
if (capableReport) {
198+
readStatusReport(refUpdates);
199+
}
200+
if (capableSideBand) {
201+
// Ensure the data channel is at EOF, so we know we have
202+
// read all side-band data from all channels and have a
203+
// complete copy of the messages (if any) buffered from
204+
// the other data channels.
205+
//
206+
int b = in.read();
207+
if (0 <= b) {
208+
throw new TransportException(uri, MessageFormat.format(
209+
JGitText.get().expectedEOFReceived,
210+
Character.valueOf((char) b)));
211+
}
212+
}
213+
}
214+
} catch (TransportException e) {
215+
if (suppressedThrowable != null) {
216+
e.addSuppressed(suppressedThrowable);
217+
}
218+
throw e;
219+
} catch (Exception e) {
220+
TransportException transportException = new TransportException(uri, e.getMessage(), e);
221+
if (suppressedThrowable != null) {
222+
transportException.addSuppressed(suppressedThrowable);
223+
}
224+
throw transportException;
225+
}
207226
if (in instanceof SideBandInputStream) {
208227
((SideBandInputStream) in).drainMessages();
209228
}

0 commit comments

Comments
 (0)