motiejus
/
jgit
1
Fork 0
Code Releases Activity

UploadPack: Introduce ErrorWriter 

ErrorWriter writes an error message to the user. The implementation is
swapped once it detects that the client supports sideband. By default it
uses the protocol level ERR packet, which was introduced recently.

In total the error output is done in two different places;
UploadPack#upload and UploadPack#sendPack. These will be consolidated in
the next change.

Change-Id: Ia8d72e31170bbeafc8ffa8ddb92702196af8a587
Signed-off-by: Masaya Suzuki <masayasuzuki@google.com>
This commit is contained in:
Masaya Suzuki 2019-07-23 11:10:24 -07:00
parent c9a8d3d040
commit 63bd24cf35
1 changed files with 35 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

52
org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java
View File

@ -44,6 +44,7 @@
package org.eclipse.jgit.transport; package org.eclipse.jgit.transport;
import static java.util.Collections.unmodifiableMap; import static java.util.Collections.unmodifiableMap;
import static java.util.Objects.requireNonNull;
import static org.eclipse.jgit.lib.Constants.R_TAGS; import static org.eclipse.jgit.lib.Constants.R_TAGS;
import static org.eclipse.jgit.transport.GitProtocolConstants.CAPABILITY_REF_IN_WANT; import static org.eclipse.jgit.transport.GitProtocolConstants.CAPABILITY_REF_IN_WANT;
import static org.eclipse.jgit.transport.GitProtocolConstants.CAPABILITY_SERVER_OPTION; import static org.eclipse.jgit.transport.GitProtocolConstants.CAPABILITY_SERVER_OPTION;
@ -282,6 +283,8 @@ private static interface IOConsumer<R> {
private OutputStream msgOut = NullOutputStream.INSTANCE; private OutputStream msgOut = NullOutputStream.INSTANCE;
private ErrorWriter errOut = new PackProtocolErrorWriter();
/** /**
* Refs eligible for advertising to the client, set using * Refs eligible for advertising to the client, set using
* {@link #setAdvertisedRefs}. * {@link #setAdvertisedRefs}.
@ -812,9 +815,9 @@ public void upload(InputStream input, OutputStream output,
// nothing. // nothing.
throw err; throw err;
} catch (ServiceMayNotContinueException err) { } catch (ServiceMayNotContinueException err) {
if (!err.isOutput() && err.getMessage() != null && pckOut != null) { if (!err.isOutput() && err.getMessage() != null) {
try { try {
pckOut.writeString("ERR " + err.getMessage() + "\n"); //$NON-NLS-1$ //$NON-NLS-2$ errOut.writeError(err.getMessage());
} catch (IOException e) { } catch (IOException e) {
err.addSuppressed(e); err.addSuppressed(e);
throw err; throw err;
@ -823,12 +826,12 @@ public void upload(InputStream input, OutputStream output,
} }
throw err; throw err;
} catch (IOException | RuntimeException | Error err) { } catch (IOException | RuntimeException | Error err) {
if (pckOut != null) { if (rawOut != null) {
String msg = err instanceof PackProtocolException String msg = err instanceof PackProtocolException
? err.getMessage() ? err.getMessage()
: JGitText.get().internalServerError; : JGitText.get().internalServerError;
try { try {
pckOut.writeString("ERR " + msg + "\n"); //$NON-NLS-1$ //$NON-NLS-2$ errOut.writeError(msg);
} catch (IOException e) { } catch (IOException e) {
err.addSuppressed(e); err.addSuppressed(e);
throw err; throw err;
@ -2115,6 +2118,8 @@ private void sendPack(PackStatistics.Accumulator accumulator,
boolean sideband = caps.contains(OPTION_SIDE_BAND) boolean sideband = caps.contains(OPTION_SIDE_BAND)
|| caps.contains(OPTION_SIDE_BAND_64K); || caps.contains(OPTION_SIDE_BAND_64K);
if (sideband) { if (sideband) {
errOut = new SideBandErrorWriter();
try { try {
sendPack(true, req, accumulator, allTags, unshallowCommits, sendPack(true, req, accumulator, allTags, unshallowCommits,
deepenNots, pckOut); deepenNots, pckOut);
@ -2124,7 +2129,7 @@ private void sendPack(PackStatistics.Accumulator accumulator,
message = JGitText.get().internalServerError; message = JGitText.get().internalServerError;
} }
try { try {
reportInternalServerErrorOverSideband(message); errOut.writeError(message);
} catch (IOException e) { } catch (IOException e) {
err.addSuppressed(e); err.addSuppressed(e);
throw err; throw err;
@ -2132,8 +2137,7 @@ private void sendPack(PackStatistics.Accumulator accumulator,
throw new UploadPackInternalServerErrorException(err); throw new UploadPackInternalServerErrorException(err);
} catch (IOException | RuntimeException | Error err) { } catch (IOException | RuntimeException | Error err) {
try { try {
reportInternalServerErrorOverSideband( errOut.writeError(JGitText.get().internalServerError);
JGitText.get().internalServerError);
} catch (IOException e) { } catch (IOException e) {
err.addSuppressed(e); err.addSuppressed(e);
throw err; throw err;
@ -2146,16 +2150,6 @@ private void sendPack(PackStatistics.Accumulator accumulator,
} }
} }
private void reportInternalServerErrorOverSideband(String message)
throws IOException {
@SuppressWarnings("resource" /* java 7 */)
SideBandOutputStream err = new SideBandOutputStream(
SideBandOutputStream.CH_ERROR, SideBandOutputStream.SMALL_BUF,
rawOut);
err.write(Constants.encode(message));
err.flush();
}
/** /**
* Send the requested objects to the client. * Send the requested objects to the client.
* *
@ -2411,4 +2405,28 @@ void stopBuffering() throws IOException {
} }
} }
} }
private interface ErrorWriter {
void writeError(String message) throws IOException;
}
private class SideBandErrorWriter implements ErrorWriter {
@Override
public void writeError(String message) throws IOException {
@SuppressWarnings("resource" /* java 7 */)
SideBandOutputStream err = new SideBandOutputStream(
SideBandOutputStream.CH_ERROR,
SideBandOutputStream.SMALL_BUF, requireNonNull(rawOut));
err.write(Constants.encode(message));
err.flush();
}
}
private class PackProtocolErrorWriter implements ErrorWriter {
@Override
public void writeError(String message) throws IOException {
new PacketLineOut(requireNonNull(rawOut))
.writeString("ERR " + message + '\n'); //$NON-NLS-1$
}
}
} }