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 <david.pursehouse@gmail.com>tags/v4.5.0.201609210915-r
@@ -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); | |||
} | |||
} |
@@ -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); | |||
} |
@@ -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(); | |||
} | |||
} |