From e27eab26e2027baba7e96a972b90b17f18843467 Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Fri, 22 Jul 2016 14:35:18 +0900 Subject: FileLfsServlet: Include error message in response body According to the specification [1], the error response body must include the error message in json format. [1] https://github.com/github/git-lfs/blob/master/docs/api/v1/http-v1-batch.md#response-errors Change-Id: I79e7a841d230fdedefa53b9c6d2d477e81e1f9e6 Signed-off-by: David Pursehouse --- .../eclipse/jgit/lfs/server/fs/DownloadTest.java | 27 +++++++++++------- .../eclipse/jgit/lfs/server/fs/LfsServerTest.java | 18 ++++++++++-- .../eclipse/jgit/lfs/server/fs/FileLfsServlet.java | 32 ++++++++++++++++++++-- 3 files changed, 62 insertions(+), 15 deletions(-) diff --git a/org.eclipse.jgit.lfs.server.test/tst/org/eclipse/jgit/lfs/server/fs/DownloadTest.java b/org.eclipse.jgit.lfs.server.test/tst/org/eclipse/jgit/lfs/server/fs/DownloadTest.java index ac707eacf0..f15edca01a 100644 --- a/org.eclipse.jgit.lfs.server.test/tst/org/eclipse/jgit/lfs/server/fs/DownloadTest.java +++ b/org.eclipse.jgit.lfs.server.test/tst/org/eclipse/jgit/lfs/server/fs/DownloadTest.java @@ -72,14 +72,15 @@ public class DownloadTest extends LfsServerTest { public void testDownloadInvalidPathInfo() throws ClientProtocolException, IOException { String TEXT = "test"; - AnyLongObjectId id = putContent(TEXT); + String id = putContent(TEXT).name().substring(0, 60); Path f = Paths.get(getTempDirectory().toString(), "download"); try { - getContent(id.name().substring(0, 60), f); + getContent(id, f); fail("expected RuntimeException"); } catch (RuntimeException e) { - assertEquals("Status: 422 Unprocessable Entity", - e.getMessage()); + String error = String.format( + "Invalid pathInfo '/%s' does not match '/{SHA-256}'", id); + assertEquals(formatErrorMessage(422, error), e.getMessage()); } } @@ -87,14 +88,14 @@ public class DownloadTest extends LfsServerTest { public void testDownloadInvalidId() throws ClientProtocolException, IOException { String TEXT = "test"; - AnyLongObjectId id = putContent(TEXT); + String id = putContent(TEXT).name().replace('f', 'z'); Path f = Paths.get(getTempDirectory().toString(), "download"); try { - getContent(id.name().replace('f', 'z'), f); + getContent(id, f); fail("expected RuntimeException"); } catch (RuntimeException e) { - assertEquals("Status: 422 Unprocessable Entity", - e.getMessage()); + String error = String.format("Invalid id: : %s", id); + assertEquals(formatErrorMessage(422, error), e.getMessage()); } } @@ -108,8 +109,8 @@ public class DownloadTest extends LfsServerTest { getContent(id, f); fail("expected RuntimeException"); } catch (RuntimeException e) { - assertEquals("Status: 404 Not Found", - e.getMessage()); + String error = String.format("Object '%s' not found", id.getName()); + assertEquals(formatErrorMessage(404, error), e.getMessage()); } } @@ -129,4 +130,10 @@ public class DownloadTest extends LfsServerTest { FileUtils.delete(f.toFile(), FileUtils.RETRY); } + + @SuppressWarnings("boxing") + private String formatErrorMessage(int status, String message) { + return String.format("Status: %d {\n \"message\": \"%s\"\n}", status, + message); + } } diff --git a/org.eclipse.jgit.lfs.server.test/tst/org/eclipse/jgit/lfs/server/fs/LfsServerTest.java b/org.eclipse.jgit.lfs.server.test/tst/org/eclipse/jgit/lfs/server/fs/LfsServerTest.java index 8c266d4830..b4a4228901 100644 --- a/org.eclipse.jgit.lfs.server.test/tst/org/eclipse/jgit/lfs/server/fs/LfsServerTest.java +++ b/org.eclipse.jgit.lfs.server.test/tst/org/eclipse/jgit/lfs/server/fs/LfsServerTest.java @@ -45,6 +45,7 @@ package org.eclipse.jgit.lfs.server.fs; import static org.junit.Assert.assertEquals; import java.io.BufferedInputStream; +import java.io.ByteArrayOutputStream; import java.io.FileNotFoundException; import java.io.IOException; import java.io.InputStream; @@ -186,8 +187,21 @@ public abstract class LfsServerTest { StatusLine statusLine = response.getStatusLine(); int status = statusLine.getStatusCode(); if (statusLine.getStatusCode() >= 400) { - throw new RuntimeException("Status: " + status + " " - + statusLine.getReasonPhrase()); + String error; + try { + BufferedInputStream bis = new BufferedInputStream( + response.getEntity().getContent()); + ByteArrayOutputStream buf = new ByteArrayOutputStream(); + int result = bis.read(); + while (result != -1) { + buf.write((byte) result); + result = bis.read(); + } + error = buf.toString(); + } catch (IOException e) { + error = statusLine.getReasonPhrase(); + } + throw new RuntimeException("Status: " + status + " " + error); } assertEquals(200, status); } diff --git a/org.eclipse.jgit.lfs.server/src/org/eclipse/jgit/lfs/server/fs/FileLfsServlet.java b/org.eclipse.jgit.lfs.server/src/org/eclipse/jgit/lfs/server/fs/FileLfsServlet.java index 925dea8de5..39aaa91d04 100644 --- a/org.eclipse.jgit.lfs.server/src/org/eclipse/jgit/lfs/server/fs/FileLfsServlet.java +++ b/org.eclipse.jgit.lfs.server/src/org/eclipse/jgit/lfs/server/fs/FileLfsServlet.java @@ -43,6 +43,7 @@ package org.eclipse.jgit.lfs.server.fs; import java.io.IOException; +import java.io.PrintWriter; import java.text.MessageFormat; import javax.servlet.AsyncContext; @@ -59,6 +60,10 @@ import org.eclipse.jgit.lfs.lib.Constants; import org.eclipse.jgit.lfs.lib.LongObjectId; import org.eclipse.jgit.lfs.server.internal.LfsServerText; +import com.google.gson.FieldNamingPolicy; +import com.google.gson.Gson; +import com.google.gson.GsonBuilder; + /** * Servlet supporting upload and download of large objects as defined by the * GitHub Large File Storage extension API extending git to allow separate @@ -76,6 +81,8 @@ public class FileLfsServlet extends HttpServlet { private final long timeout; + private static Gson gson = createGson(); + /** * @param repository * the repository storing the large objects @@ -106,7 +113,8 @@ public class FileLfsServlet extends HttpServlet { if (obj != null) { if (repository.getSize(obj) == -1) { sendError(rsp, HttpStatus.SC_NOT_FOUND, MessageFormat - .format(LfsServerText.get().objectNotFound, obj)); + .format(LfsServerText.get().objectNotFound, + obj.getName())); return; } AsyncContext context = req.startAsync(); @@ -157,11 +165,29 @@ public class FileLfsServlet extends HttpServlet { } } + static class Error { + String message; + + Error(String m) { + this.message = m; + } + } + static void sendError(HttpServletResponse rsp, int status, String message) throws IOException { rsp.setStatus(status); - // TODO return message in response body in json format as specified in - // https://github.com/github/git-lfs/blob/master/docs/api/v1/http-v1-batch.md + PrintWriter writer = rsp.getWriter(); + gson.toJson(new Error(message), writer); + writer.flush(); + writer.close(); rsp.flushBuffer(); } + + private static Gson createGson() { + GsonBuilder gb = new GsonBuilder() + .setFieldNamingPolicy( + FieldNamingPolicy.LOWER_CASE_WITH_UNDERSCORES) + .setPrettyPrinting().disableHtmlEscaping(); + return gb.create(); + } } -- cgit v1.2.3