Merge changes I5381e110,I5534b560
* changes: Discard request HTTP bodies for status code <400 Ensure all smart HTTP errors are sent to clients
This commit is contained in:
commit
cd958ba93c
|
@ -49,11 +49,11 @@
|
||||||
|
|
||||||
import java.io.ByteArrayOutputStream;
|
import java.io.ByteArrayOutputStream;
|
||||||
import java.io.IOException;
|
import java.io.IOException;
|
||||||
|
import java.io.OutputStream;
|
||||||
import java.util.Arrays;
|
import java.util.Arrays;
|
||||||
import java.util.Collections;
|
import java.util.Collections;
|
||||||
import java.util.List;
|
import java.util.List;
|
||||||
|
|
||||||
import javax.servlet.ServletOutputStream;
|
|
||||||
import javax.servlet.http.HttpServletRequest;
|
import javax.servlet.http.HttpServletRequest;
|
||||||
import javax.servlet.http.HttpServletResponse;
|
import javax.servlet.http.HttpServletResponse;
|
||||||
|
|
||||||
|
@ -184,24 +184,27 @@ public static void sendError(HttpServletRequest req,
|
||||||
pck.writeString("# service=" + svc + "\n");
|
pck.writeString("# service=" + svc + "\n");
|
||||||
pck.end();
|
pck.end();
|
||||||
pck.writeString("ERR " + textForGit);
|
pck.writeString("ERR " + textForGit);
|
||||||
send(res, infoRefsResultType(svc), buf.toByteArray());
|
send(req, res, infoRefsResultType(svc), buf.toByteArray());
|
||||||
} else if (isUploadPack(req)) {
|
} else if (isUploadPack(req)) {
|
||||||
pck.writeString("ERR " + textForGit);
|
pck.writeString("ERR " + textForGit);
|
||||||
send(res, UPLOAD_PACK_RESULT_TYPE, buf.toByteArray());
|
send(req, res, UPLOAD_PACK_RESULT_TYPE, buf.toByteArray());
|
||||||
} else if (isReceivePack(req)) {
|
} else if (isReceivePack(req)) {
|
||||||
pck.writeString("ERR " + textForGit);
|
pck.writeString("ERR " + textForGit);
|
||||||
send(res, RECEIVE_PACK_RESULT_TYPE, buf.toByteArray());
|
send(req, res, RECEIVE_PACK_RESULT_TYPE, buf.toByteArray());
|
||||||
} else {
|
} else {
|
||||||
|
if (httpStatus < 400)
|
||||||
|
ServletUtils.consumeRequestBody(req);
|
||||||
res.sendError(httpStatus);
|
res.sendError(httpStatus);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
private static void send(HttpServletResponse res, String type, byte[] buf)
|
private static void send(HttpServletRequest req, HttpServletResponse res,
|
||||||
throws IOException {
|
String type, byte[] buf) throws IOException {
|
||||||
|
ServletUtils.consumeRequestBody(req);
|
||||||
res.setStatus(HttpServletResponse.SC_OK);
|
res.setStatus(HttpServletResponse.SC_OK);
|
||||||
res.setContentType(type);
|
res.setContentType(type);
|
||||||
res.setContentLength(buf.length);
|
res.setContentLength(buf.length);
|
||||||
ServletOutputStream os = res.getOutputStream();
|
OutputStream os = res.getOutputStream();
|
||||||
try {
|
try {
|
||||||
os.write(buf);
|
os.write(buf);
|
||||||
} finally {
|
} finally {
|
||||||
|
|
|
@ -52,6 +52,7 @@
|
||||||
import static org.eclipse.jgit.http.server.GitSmartHttpTools.RECEIVE_PACK_RESULT_TYPE;
|
import static org.eclipse.jgit.http.server.GitSmartHttpTools.RECEIVE_PACK_RESULT_TYPE;
|
||||||
import static org.eclipse.jgit.http.server.GitSmartHttpTools.sendError;
|
import static org.eclipse.jgit.http.server.GitSmartHttpTools.sendError;
|
||||||
import static org.eclipse.jgit.http.server.ServletUtils.ATTRIBUTE_HANDLER;
|
import static org.eclipse.jgit.http.server.ServletUtils.ATTRIBUTE_HANDLER;
|
||||||
|
import static org.eclipse.jgit.http.server.ServletUtils.consumeRequestBody;
|
||||||
import static org.eclipse.jgit.http.server.ServletUtils.getInputStream;
|
import static org.eclipse.jgit.http.server.ServletUtils.getInputStream;
|
||||||
import static org.eclipse.jgit.http.server.ServletUtils.getRepository;
|
import static org.eclipse.jgit.http.server.ServletUtils.getRepository;
|
||||||
|
|
||||||
|
@ -158,17 +159,18 @@ public void doPost(final HttpServletRequest req,
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
SmartOutputStream out = new SmartOutputStream(req, rsp) {
|
||||||
|
@Override
|
||||||
|
public void flush() throws IOException {
|
||||||
|
doFlush();
|
||||||
|
}
|
||||||
|
};
|
||||||
|
|
||||||
ReceivePack rp = (ReceivePack) req.getAttribute(ATTRIBUTE_HANDLER);
|
ReceivePack rp = (ReceivePack) req.getAttribute(ATTRIBUTE_HANDLER);
|
||||||
try {
|
try {
|
||||||
rp.setBiDirectionalPipe(false);
|
rp.setBiDirectionalPipe(false);
|
||||||
rsp.setContentType(RECEIVE_PACK_RESULT_TYPE);
|
rsp.setContentType(RECEIVE_PACK_RESULT_TYPE);
|
||||||
|
|
||||||
final SmartOutputStream out = new SmartOutputStream(req, rsp) {
|
|
||||||
@Override
|
|
||||||
public void flush() throws IOException {
|
|
||||||
doFlush();
|
|
||||||
}
|
|
||||||
};
|
|
||||||
rp.receive(getInputStream(req), out, null);
|
rp.receive(getInputStream(req), out, null);
|
||||||
out.close();
|
out.close();
|
||||||
} catch (UnpackException e) {
|
} catch (UnpackException e) {
|
||||||
|
@ -176,8 +178,10 @@ public void flush() throws IOException {
|
||||||
getServletContext().log(
|
getServletContext().log(
|
||||||
HttpServerText.get().internalErrorDuringReceivePack,
|
HttpServerText.get().internalErrorDuringReceivePack,
|
||||||
e.getCause());
|
e.getCause());
|
||||||
|
consumeRequestBody(req);
|
||||||
|
out.close();
|
||||||
|
|
||||||
} catch (IOException e) {
|
} catch (Throwable e) {
|
||||||
getServletContext().log(HttpServerText.get().internalErrorDuringReceivePack, e);
|
getServletContext().log(HttpServerText.get().internalErrorDuringReceivePack, e);
|
||||||
if (!rsp.isCommitted()) {
|
if (!rsp.isCommitted()) {
|
||||||
rsp.reset();
|
rsp.reset();
|
||||||
|
|
|
@ -118,6 +118,50 @@ else if (enc != null)
|
||||||
return in;
|
return in;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Consume the entire request body, if one was supplied.
|
||||||
|
*
|
||||||
|
* @param req
|
||||||
|
* the request whose body must be consumed.
|
||||||
|
*/
|
||||||
|
public static void consumeRequestBody(HttpServletRequest req) {
|
||||||
|
if (0 < req.getContentLength() || isChunked(req)) {
|
||||||
|
try {
|
||||||
|
consumeRequestBody(req.getInputStream());
|
||||||
|
} catch (IOException e) {
|
||||||
|
// Ignore any errors obtaining the input stream.
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
private static boolean isChunked(HttpServletRequest req) {
|
||||||
|
return "chunked".equals(req.getHeader("Transfer-Encoding"));
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Consume the rest of the input stream and discard it.
|
||||||
|
*
|
||||||
|
* @param in
|
||||||
|
* the stream to discard, closed if not null.
|
||||||
|
*/
|
||||||
|
public static void consumeRequestBody(InputStream in) {
|
||||||
|
if (in == null)
|
||||||
|
return;
|
||||||
|
try {
|
||||||
|
while (0 < in.skip(2048) || 0 <= in.read()) {
|
||||||
|
// Discard until EOF.
|
||||||
|
}
|
||||||
|
} catch (IOException err) {
|
||||||
|
// Discard IOException during read or skip.
|
||||||
|
} finally {
|
||||||
|
try {
|
||||||
|
in.close();
|
||||||
|
} catch (IOException err) {
|
||||||
|
// Discard IOException during close of input stream.
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Send a plain text response to a {@code GET} or {@code HEAD} HTTP request.
|
* Send a plain text response to a {@code GET} or {@code HEAD} HTTP request.
|
||||||
* <p>
|
* <p>
|
||||||
|
|
|
@ -52,6 +52,7 @@
|
||||||
import static org.eclipse.jgit.http.server.GitSmartHttpTools.UPLOAD_PACK_RESULT_TYPE;
|
import static org.eclipse.jgit.http.server.GitSmartHttpTools.UPLOAD_PACK_RESULT_TYPE;
|
||||||
import static org.eclipse.jgit.http.server.GitSmartHttpTools.sendError;
|
import static org.eclipse.jgit.http.server.GitSmartHttpTools.sendError;
|
||||||
import static org.eclipse.jgit.http.server.ServletUtils.ATTRIBUTE_HANDLER;
|
import static org.eclipse.jgit.http.server.ServletUtils.ATTRIBUTE_HANDLER;
|
||||||
|
import static org.eclipse.jgit.http.server.ServletUtils.consumeRequestBody;
|
||||||
import static org.eclipse.jgit.http.server.ServletUtils.getInputStream;
|
import static org.eclipse.jgit.http.server.ServletUtils.getInputStream;
|
||||||
import static org.eclipse.jgit.http.server.ServletUtils.getRepository;
|
import static org.eclipse.jgit.http.server.ServletUtils.getRepository;
|
||||||
|
|
||||||
|
@ -160,22 +161,26 @@ public void doPost(final HttpServletRequest req,
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
SmartOutputStream out = new SmartOutputStream(req, rsp) {
|
||||||
|
@Override
|
||||||
|
public void flush() throws IOException {
|
||||||
|
doFlush();
|
||||||
|
}
|
||||||
|
};
|
||||||
|
|
||||||
UploadPack up = (UploadPack) req.getAttribute(ATTRIBUTE_HANDLER);
|
UploadPack up = (UploadPack) req.getAttribute(ATTRIBUTE_HANDLER);
|
||||||
try {
|
try {
|
||||||
up.setBiDirectionalPipe(false);
|
up.setBiDirectionalPipe(false);
|
||||||
rsp.setContentType(UPLOAD_PACK_RESULT_TYPE);
|
rsp.setContentType(UPLOAD_PACK_RESULT_TYPE);
|
||||||
|
|
||||||
final SmartOutputStream out = new SmartOutputStream(req, rsp) {
|
|
||||||
@Override
|
|
||||||
public void flush() throws IOException {
|
|
||||||
doFlush();
|
|
||||||
}
|
|
||||||
};
|
|
||||||
up.upload(getInputStream(req), out, null);
|
up.upload(getInputStream(req), out, null);
|
||||||
out.close();
|
out.close();
|
||||||
|
|
||||||
} catch (UploadPackMayNotContinueException e) {
|
} catch (UploadPackMayNotContinueException e) {
|
||||||
if (!e.isOutput() && !rsp.isCommitted()) {
|
if (e.isOutput()) {
|
||||||
|
consumeRequestBody(req);
|
||||||
|
out.close();
|
||||||
|
} else if (!rsp.isCommitted()) {
|
||||||
rsp.reset();
|
rsp.reset();
|
||||||
sendError(req, rsp, SC_FORBIDDEN, e.getMessage());
|
sendError(req, rsp, SC_FORBIDDEN, e.getMessage());
|
||||||
}
|
}
|
||||||
|
@ -186,8 +191,10 @@ public void flush() throws IOException {
|
||||||
getServletContext().log(
|
getServletContext().log(
|
||||||
HttpServerText.get().internalErrorDuringUploadPack,
|
HttpServerText.get().internalErrorDuringUploadPack,
|
||||||
e.getCause());
|
e.getCause());
|
||||||
|
consumeRequestBody(req);
|
||||||
|
out.close();
|
||||||
|
|
||||||
} catch (IOException e) {
|
} catch (Throwable e) {
|
||||||
getServletContext().log(HttpServerText.get().internalErrorDuringUploadPack, e);
|
getServletContext().log(HttpServerText.get().internalErrorDuringUploadPack, e);
|
||||||
if (!rsp.isCommitted()) {
|
if (!rsp.isCommitted()) {
|
||||||
rsp.reset();
|
rsp.reset();
|
||||||
|
|
|
@ -0,0 +1,188 @@
|
||||||
|
/*
|
||||||
|
* Copyright (C) 2010, Google Inc.
|
||||||
|
* and other copyright owners as documented in the project's IP log.
|
||||||
|
*
|
||||||
|
* This program and the accompanying materials are made available
|
||||||
|
* under the terms of the Eclipse Distribution License v1.0 which
|
||||||
|
* accompanies this distribution, is reproduced below, and is
|
||||||
|
* available at http://www.eclipse.org/org/documents/edl-v10.php
|
||||||
|
*
|
||||||
|
* All rights reserved.
|
||||||
|
*
|
||||||
|
* Redistribution and use in source and binary forms, with or
|
||||||
|
* without modification, are permitted provided that the following
|
||||||
|
* conditions are met:
|
||||||
|
*
|
||||||
|
* - Redistributions of source code must retain the above copyright
|
||||||
|
* notice, this list of conditions and the following disclaimer.
|
||||||
|
*
|
||||||
|
* - Redistributions in binary form must reproduce the above
|
||||||
|
* copyright notice, this list of conditions and the following
|
||||||
|
* disclaimer in the documentation and/or other materials provided
|
||||||
|
* with the distribution.
|
||||||
|
*
|
||||||
|
* - Neither the name of the Eclipse Foundation, Inc. nor the
|
||||||
|
* names of its contributors may be used to endorse or promote
|
||||||
|
* products derived from this software without specific prior
|
||||||
|
* written permission.
|
||||||
|
*
|
||||||
|
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
|
||||||
|
* CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
|
||||||
|
* INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
|
||||||
|
* OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
|
||||||
|
* ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
|
||||||
|
* CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
|
||||||
|
* SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
|
||||||
|
* NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
|
||||||
|
* LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
|
||||||
|
* CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
|
||||||
|
* STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
|
||||||
|
* ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
|
||||||
|
* ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
|
||||||
|
*/
|
||||||
|
|
||||||
|
package org.eclipse.jgit.http.test;
|
||||||
|
|
||||||
|
import static org.junit.Assert.assertEquals;
|
||||||
|
import static org.junit.Assert.assertSame;
|
||||||
|
|
||||||
|
import java.io.ByteArrayOutputStream;
|
||||||
|
import java.io.IOException;
|
||||||
|
import java.io.InputStream;
|
||||||
|
import java.io.OutputStream;
|
||||||
|
import java.net.HttpURLConnection;
|
||||||
|
import java.net.URL;
|
||||||
|
|
||||||
|
import javax.servlet.http.HttpServletRequest;
|
||||||
|
|
||||||
|
import org.eclipse.jetty.servlet.ServletContextHandler;
|
||||||
|
import org.eclipse.jetty.servlet.ServletHolder;
|
||||||
|
import org.eclipse.jgit.JGitText;
|
||||||
|
import org.eclipse.jgit.errors.RepositoryNotFoundException;
|
||||||
|
import org.eclipse.jgit.http.server.GitServlet;
|
||||||
|
import org.eclipse.jgit.http.server.GitSmartHttpTools;
|
||||||
|
import org.eclipse.jgit.junit.TestRepository;
|
||||||
|
import org.eclipse.jgit.junit.http.HttpTestCase;
|
||||||
|
import org.eclipse.jgit.lib.Constants;
|
||||||
|
import org.eclipse.jgit.lib.ObjectId;
|
||||||
|
import org.eclipse.jgit.lib.Repository;
|
||||||
|
import org.eclipse.jgit.revwalk.RevBlob;
|
||||||
|
import org.eclipse.jgit.storage.file.FileBasedConfig;
|
||||||
|
import org.eclipse.jgit.storage.file.FileRepository;
|
||||||
|
import org.eclipse.jgit.transport.PacketLineIn;
|
||||||
|
import org.eclipse.jgit.transport.PacketLineOut;
|
||||||
|
import org.eclipse.jgit.transport.URIish;
|
||||||
|
import org.eclipse.jgit.transport.resolver.RepositoryResolver;
|
||||||
|
import org.eclipse.jgit.transport.resolver.ServiceNotEnabledException;
|
||||||
|
import org.eclipse.jgit.util.NB;
|
||||||
|
import org.junit.Before;
|
||||||
|
import org.junit.Test;
|
||||||
|
|
||||||
|
public class ProtocolErrorTest extends HttpTestCase {
|
||||||
|
private FileRepository remoteRepository;
|
||||||
|
|
||||||
|
private URIish remoteURI;
|
||||||
|
|
||||||
|
private RevBlob a_blob;
|
||||||
|
|
||||||
|
@Before
|
||||||
|
public void setUp() throws Exception {
|
||||||
|
super.setUp();
|
||||||
|
|
||||||
|
final TestRepository<FileRepository> src = createTestRepository();
|
||||||
|
final String srcName = src.getRepository().getDirectory().getName();
|
||||||
|
|
||||||
|
ServletContextHandler app = server.addContext("/git");
|
||||||
|
GitServlet gs = new GitServlet();
|
||||||
|
gs.setRepositoryResolver(new RepositoryResolver<HttpServletRequest>() {
|
||||||
|
public Repository open(HttpServletRequest req, String name)
|
||||||
|
throws RepositoryNotFoundException,
|
||||||
|
ServiceNotEnabledException {
|
||||||
|
if (!name.equals(srcName))
|
||||||
|
throw new RepositoryNotFoundException(name);
|
||||||
|
|
||||||
|
final Repository db = src.getRepository();
|
||||||
|
db.incrementOpen();
|
||||||
|
return db;
|
||||||
|
}
|
||||||
|
});
|
||||||
|
app.addServlet(new ServletHolder(gs), "/*");
|
||||||
|
|
||||||
|
server.setUp();
|
||||||
|
|
||||||
|
remoteRepository = src.getRepository();
|
||||||
|
remoteURI = toURIish(app, srcName);
|
||||||
|
|
||||||
|
FileBasedConfig cfg = remoteRepository.getConfig();
|
||||||
|
cfg.setBoolean("http", null, "receivepack", true);
|
||||||
|
cfg.save();
|
||||||
|
|
||||||
|
a_blob = src.blob("a");
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void testPush_UnpackError_TruncatedPack() throws Exception {
|
||||||
|
StringBuilder sb = new StringBuilder();
|
||||||
|
sb.append(ObjectId.zeroId().name());
|
||||||
|
sb.append(' ');
|
||||||
|
sb.append(a_blob.name());
|
||||||
|
sb.append(' ');
|
||||||
|
sb.append("refs/objects/A");
|
||||||
|
sb.append('\0');
|
||||||
|
sb.append("report-status");
|
||||||
|
|
||||||
|
ByteArrayOutputStream reqbuf = new ByteArrayOutputStream();
|
||||||
|
PacketLineOut reqpck = new PacketLineOut(reqbuf);
|
||||||
|
reqpck.writeString(sb.toString());
|
||||||
|
reqpck.end();
|
||||||
|
|
||||||
|
packHeader(reqbuf, 1);
|
||||||
|
|
||||||
|
byte[] reqbin = reqbuf.toByteArray();
|
||||||
|
|
||||||
|
URL u = new URL(remoteURI.toString() + "/git-receive-pack");
|
||||||
|
HttpURLConnection c = (HttpURLConnection) u.openConnection();
|
||||||
|
try {
|
||||||
|
c.setRequestMethod("POST");
|
||||||
|
c.setDoOutput(true);
|
||||||
|
c.setRequestProperty("Content-Type",
|
||||||
|
GitSmartHttpTools.RECEIVE_PACK_REQUEST_TYPE);
|
||||||
|
c.setFixedLengthStreamingMode(reqbin.length);
|
||||||
|
OutputStream out = c.getOutputStream();
|
||||||
|
try {
|
||||||
|
out.write(reqbin);
|
||||||
|
} finally {
|
||||||
|
out.close();
|
||||||
|
}
|
||||||
|
|
||||||
|
assertEquals(200, c.getResponseCode());
|
||||||
|
assertEquals(GitSmartHttpTools.RECEIVE_PACK_RESULT_TYPE,
|
||||||
|
c.getContentType());
|
||||||
|
|
||||||
|
InputStream rawin = c.getInputStream();
|
||||||
|
try {
|
||||||
|
PacketLineIn pckin = new PacketLineIn(rawin);
|
||||||
|
assertEquals("unpack error "
|
||||||
|
+ JGitText.get().packfileIsTruncated,
|
||||||
|
pckin.readString());
|
||||||
|
assertEquals("ng refs/objects/A n/a (unpacker error)",
|
||||||
|
pckin.readString());
|
||||||
|
assertSame(PacketLineIn.END, pckin.readString());
|
||||||
|
} finally {
|
||||||
|
rawin.close();
|
||||||
|
}
|
||||||
|
} finally {
|
||||||
|
c.disconnect();
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
private void packHeader(ByteArrayOutputStream tinyPack, int cnt)
|
||||||
|
throws IOException {
|
||||||
|
final byte[] hdr = new byte[8];
|
||||||
|
NB.encodeInt32(hdr, 0, 2);
|
||||||
|
NB.encodeInt32(hdr, 4, cnt);
|
||||||
|
|
||||||
|
tinyPack.write(Constants.PACK_SIGNATURE);
|
||||||
|
tinyPack.write(hdr, 0, 8);
|
||||||
|
}
|
||||||
|
}
|
Loading…
Reference in New Issue