Mercurial > hg > release > thermostat-0.11
changeset 1151:2b3cf603245d
Fix NPE in WebStorageEndPoint (causing server 500 errors)
reviewed-by: neugens
review-thread: http://icedtea.classpath.org/pipermail/thermostat/2013-July/007234.html
author | Jon VanAlten <jon.vanalten@redhat.com> |
---|---|
date | Wed, 03 Jul 2013 12:12:47 -0600 |
parents | d6f3b3eec520 |
children | 4ba43dcff78e |
files | integration-tests/src/test/java/com/redhat/thermostat/itest/WebAppTest.java web/client/pom.xml web/client/src/main/java/com/redhat/thermostat/web/client/internal/WebStorage.java web/client/src/test/java/com/redhat/thermostat/web/client/internal/WebStorageTest.java web/server/src/main/java/com/redhat/thermostat/web/server/WebStorageEndPoint.java |
diffstat | 5 files changed, 126 insertions(+), 74 deletions(-) [+] |
line wrap: on
line diff
--- a/integration-tests/src/test/java/com/redhat/thermostat/itest/WebAppTest.java Mon Jun 24 14:15:01 2013 -0600 +++ b/integration-tests/src/test/java/com/redhat/thermostat/itest/WebAppTest.java Wed Jul 03 12:12:47 2013 -0600 @@ -43,6 +43,7 @@ import static com.redhat.thermostat.itest.IntegrationTest.spawnThermostat; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; import java.io.ByteArrayInputStream; @@ -58,11 +59,7 @@ import java.util.Properties; import java.util.UUID; -import org.eclipse.jetty.security.DefaultUserIdentity; -import org.eclipse.jetty.security.MappedLoginService; import org.eclipse.jetty.server.Server; -import org.eclipse.jetty.server.UserIdentity; -import org.eclipse.jetty.util.security.Password; import org.eclipse.jetty.webapp.WebAppContext; import org.junit.After; import org.junit.AfterClass; @@ -599,11 +596,19 @@ byte[] data = "Hello World".getBytes(); webStorage.saveFile("test", new ByteArrayInputStream(data)); - // Note: Apparently, saving the file takes a bit. Without this - // waiting, we sometimes get problems on loadFile. There seems - // to be no way to synchronize on the operation in Mongo. - Thread.sleep(300); - InputStream loadStream = webStorage.loadFile("test"); + // Note: On the server side, the file is saved into mongodb + // via GridFS. The save operation returns before write is + // complete, and there is no callback mechanism to find out + // when the write is complete. So, we try a few times to + // load it before considering it a failure. + InputStream loadStream = null; + int loadAttempts = 0; + while (loadStream == null && loadAttempts < 3) { + Thread.sleep(300); + loadStream = webStorage.loadFile("test"); + loadAttempts++; + } + assertNotNull(loadStream); StringBuilder str = new StringBuilder(); int i = loadStream.read(); while (i != -1) { @@ -612,30 +617,4 @@ } assertEquals("Hello World", str.toString()); } - - private static class TestLoginService extends MappedLoginService { - - private final String[] roleNames; - private final String username; - private final String password; - - private TestLoginService(String username, String password, - String[] roleNames) { - this.username = username; - this.password = password; - this.roleNames = roleNames; - } - - @Override - protected void loadUsers() throws IOException { - putUser(username, new Password(password), - roleNames); - } - - @Override - protected UserIdentity loadUser(String username) { - return new DefaultUserIdentity(null, null, - roleNames); - } - } }
--- a/web/client/pom.xml Mon Jun 24 14:15:01 2013 -0600 +++ b/web/client/pom.xml Wed Jul 03 12:12:47 2013 -0600 @@ -86,6 +86,12 @@ <version>${jetty.version}</version> <scope>test</scope> </dependency> + <dependency> + <groupId>javax.servlet</groupId> + <artifactId>servlet-api</artifactId> + <version>${javax.servlet.version}</version> + <scope>provided</scope> + </dependency> <dependency> <groupId>com.google.code.gson</groupId>
--- a/web/client/src/main/java/com/redhat/thermostat/web/client/internal/WebStorage.java Mon Jun 24 14:15:01 2013 -0600 +++ b/web/client/src/main/java/com/redhat/thermostat/web/client/internal/WebStorage.java Wed Jul 03 12:12:47 2013 -0600 @@ -58,6 +58,7 @@ import java.util.logging.Logger; import javax.net.ssl.SSLContext; +import javax.servlet.http.HttpServletResponse; import org.apache.commons.beanutils.BeanUtils; import org.apache.commons.codec.binary.Base64; @@ -121,9 +122,11 @@ private static class CloseableHttpEntity implements Closeable, HttpEntity { private HttpEntity entity; + private int responseCode; - CloseableHttpEntity(HttpEntity entity) { + CloseableHttpEntity(HttpEntity entity, int responseCode) { this.entity = entity; + this.responseCode = responseCode; } @Override @@ -181,6 +184,9 @@ } } + int getResponseCode() { + return responseCode; + } } private final class WebConnection extends Connection { @@ -436,11 +442,19 @@ } HttpResponse response = httpClient.execute(httpPost); StatusLine status = response.getStatusLine(); - if (status.getStatusCode() != 200) { + int responseCode = status.getStatusCode(); + switch (responseCode) { + case (HttpServletResponse.SC_NO_CONTENT): + // Let calling code handle SC_NO_CONTENT + break; + case (HttpServletResponse.SC_OK): + // Let calling code handle SC_OK + break; + default: throw new IOException("Server returned status: " + status); } - return new CloseableHttpEntity(response.getEntity()); + return new CloseableHttpEntity(response.getEntity(), responseCode); } private static InputStream getContent(HttpEntity entity) { @@ -527,10 +541,31 @@ NameValuePair fileParam = new BasicNameValuePair("file", name); List<NameValuePair> formparams = Arrays.asList(fileParam); CloseableHttpEntity entity = post(endpoint + "/load-file", formparams); + if (entity.getResponseCode() == HttpServletResponse.SC_NO_CONTENT) { + return null; + } return new WebDataStream(entity); } @Override + public void saveFile(String name, InputStream in) throws StorageException { + InputStreamBody body = new InputStreamBody(in, name); + MultipartEntity mpEntity = new MultipartEntity(); + mpEntity.addPart("file", body); + // See IcedTea bug #1314. For safe-file we need to do this. However, + // doing this for other actions messes up authentication when using + // jetty (and possibly others). Hence, do this expect-continue thingy + // only for save-file. + httpClient.getParams().setParameter("http.protocol.expect-continue", Boolean.TRUE); + try { + post(endpoint + "/save-file", mpEntity).close(); + } finally { + // FIXME: Not sure if we need this :/ + httpClient.getParams().removeParameter("http.protocol.expect-continue"); + } + } + + @Override public void purge(String agentId) throws StorageException { NameValuePair agentIdParam = new BasicNameValuePair("agentId", agentId); List<NameValuePair> agentIdParams = Arrays.asList(agentIdParam); @@ -582,24 +617,6 @@ } @Override - public void saveFile(String name, InputStream in) throws StorageException { - InputStreamBody body = new InputStreamBody(in, name); - MultipartEntity mpEntity = new MultipartEntity(); - mpEntity.addPart("file", body); - // See IcedTea bug #1314. For safe-file we need to do this. However, - // doing this for other actions messes up authentication when using - // jetty (and possibly others). Hence, do this expect-continue thingy - // only for save-file. - httpClient.getParams().setParameter("http.protocol.expect-continue", Boolean.TRUE); - try { - post(endpoint + "/save-file", mpEntity).close(); - } finally { - // FIXME: Not sure if we need this :/ - httpClient.getParams().removeParameter("http.protocol.expect-continue"); - } - } - - @Override public void setAgentId(UUID agentId) { this.agentId = agentId; }
--- a/web/client/src/test/java/com/redhat/thermostat/web/client/internal/WebStorageTest.java Mon Jun 24 14:15:01 2013 -0600 +++ b/web/client/src/test/java/com/redhat/thermostat/web/client/internal/WebStorageTest.java Wed Jul 03 12:12:47 2013 -0600 @@ -119,13 +119,16 @@ private int port; - private String requestBody; + // Set these in prepareServer() to determine server behaviour + private int responseStatus = HttpServletResponse.SC_OK; + private String responseBody; - private String responseBody; + // These get set by test server handler (anonymous class in startServer()) + // Check them after WebStorage method call that should interact with server. + private String requestBody; private Map<String,String> headers; private String method; private String requestURI; - private int responseStatus; private static Category<TestObj> category; private static Key<String> key1; @@ -151,7 +154,7 @@ @Before public void setUp() throws Exception { - + port = FreePortFinder.findFreePort(new TryPort() { @Override public void tryPort(int port) throws Exception { @@ -170,9 +173,6 @@ storage.setEndpoint("http://localhost:" + port + "/"); storage.setAgentId(new UUID(123, 456)); headers = new HashMap<>(); - requestURI = null; - method = null; - responseStatus = HttpServletResponse.SC_OK; registerCategory(); factory = new ExpressionFactory(); } @@ -216,6 +216,32 @@ server.start(); } + // Specified status and response body. + private void prepareServer(int responseStatus, String responseBody) { + this.responseStatus = responseStatus; + this.responseBody = responseBody; + + requestBody = null; + requestURI = null; + method = null; + headers.clear(); + } + + // Specified status and null response body. + private void prepareServer(int responseStatus) { + prepareServer(responseStatus, null); + } + + // OK status and specified response body. + private void prepareServer(String responseBody) { + prepareServer(HttpServletResponse.SC_OK, responseBody); + } + + // OK status and null response body. + private void prepareServer() { + prepareServer(HttpServletResponse.SC_OK); + } + @After public void tearDown() throws Exception { @@ -246,7 +272,6 @@ obj2.setProperty1("fluffor2"); Gson gson = new GsonBuilder().registerTypeHierarchyAdapter(Expression.class, new ExpressionSerializer()) .registerTypeHierarchyAdapter(Operator.class, new OperatorSerializer()).create(); - responseBody = gson.toJson(Arrays.asList(obj1, obj2)); Key<String> key1 = new Key<>("property1", true); Query<TestObj> query = storage.createQuery(category); @@ -254,7 +279,9 @@ Expression expr = factory.equalTo(key1, "fluff"); query.where(expr); + prepareServer(gson.toJson(Arrays.asList(obj1, obj2))); Cursor<TestObj> results = query.execute(); + StringReader reader = new StringReader(requestBody); BufferedReader bufRead = new BufferedReader(reader); String line = URLDecoder.decode(bufRead.readLine(), "UTF-8"); @@ -291,6 +318,8 @@ Put replace = storage.createReplace(category); replace.setPojo(obj); + + prepareServer(); replace.apply(); Gson gson = new Gson(); @@ -332,6 +361,8 @@ public void testRemovePojo() throws UnsupportedEncodingException, IOException { Expression expr = factory.equalTo(key1, "test"); Remove remove = storage.createRemove().from(category).where(expr); + + prepareServer(); storage.removePojo(remove); Gson gson = new GsonBuilder() @@ -376,6 +407,8 @@ update.where(expr); update.set(key1, "fluff"); update.set(key2, 42); + + prepareServer(); update.apply(); Gson gson = new GsonBuilder() @@ -422,9 +455,7 @@ @Test public void testGetCount() throws UnsupportedEncodingException, IOException { - Gson gson = new Gson(); - responseBody = gson.toJson(12345); - + prepareServer(new Gson().toJson(12345)); long result = storage.getCount(category); StringReader reader = new StringReader(requestBody); @@ -440,7 +471,10 @@ public void testSaveFile() { String data = "Hello World"; ByteArrayInputStream in = new ByteArrayInputStream(data.getBytes()); + + prepareServer(); storage.saveFile("fluff", in); + assertEquals("chunked", headers.get("Transfer-Encoding")); String contentType = headers.get("Content-Type"); assertTrue(contentType.startsWith("multipart/form-data; boundary=")); @@ -458,8 +492,12 @@ @Test public void testLoadFile() throws IOException { - responseBody = "Hello World"; - InputStream in = storage.loadFile("fluff"); + prepareServer(HttpServletResponse.SC_NO_CONTENT); + InputStream in = storage.loadFile("no_file_here"); + assertNull(in); + + prepareServer("Hello World"); + in = storage.loadFile("fluff"); assertEquals("file=fluff", requestBody.trim()); byte[] data = new byte[11]; int totalRead = 0; @@ -476,7 +514,10 @@ @Test public void testPurge() throws UnsupportedEncodingException, IOException { + + prepareServer(); storage.purge("fluff"); + assertEquals("POST", method); assertTrue(requestURI.endsWith("/purge")); StringReader reader = new StringReader(requestBody); @@ -489,9 +530,10 @@ @Test public void testGenerateToken() throws UnsupportedEncodingException { - responseBody = "flufftoken"; final String actionName = "some action"; + + prepareServer("flufftoken"); AuthToken authToken = storage.generateToken(actionName); assertTrue(requestURI.endsWith("/generate-token")); @@ -509,6 +551,7 @@ assertTrue(Arrays.equals(clientToken, authToken.getClientToken())); // Send another request and verify that we send a different client-token every time. + prepareServer("flufftoken"); storage.generateToken(actionName); requestParts = requestBody.split("="); @@ -521,6 +564,7 @@ @Test public void canSSLEnableClient() { + // This test doesn't use the class-wide storage+server setup. StartupConfiguration config = new StartupConfiguration() { @Override @@ -528,7 +572,7 @@ return "https://onlyHttpsPrefixUsed.example.com"; } }; - storage = new WebStorage(config); + WebStorage storage = new WebStorage(config); HttpClient client = storage.httpClient; SchemeRegistry schemeReg = client.getConnectionManager().getSchemeRegistry(); Scheme scheme = schemeReg.getScheme("https"); @@ -545,8 +589,10 @@ AuthToken authToken = new AuthToken(token, tokenDigest); - responseStatus = 200; + prepareServer(); boolean ok = storage.verifyToken(authToken, someAction); + + assertTrue(ok); assertTrue(requestURI.endsWith("/verify-token")); assertEquals("POST", method); String[] requestParts = requestBody.split("&"); @@ -565,10 +611,9 @@ urlDecoded = URLDecoder.decode(authTokenParts[1], "UTF-8"); String base64decoded = new String(Base64.decodeBase64(urlDecoded)); assertEquals("stuff", base64decoded); - assertTrue(ok); // Try another one in which verification fails. - responseStatus = 401; + prepareServer(HttpServletResponse.SC_UNAUTHORIZED); ok = storage.verifyToken(authToken, someAction); assertFalse(ok);
--- a/web/server/src/main/java/com/redhat/thermostat/web/server/WebStorageEndPoint.java Mon Jun 24 14:15:01 2013 -0600 +++ b/web/server/src/main/java/com/redhat/thermostat/web/server/WebStorageEndPoint.java Wed Jul 03 12:12:47 2013 -0600 @@ -271,6 +271,10 @@ String name = req.getParameter("file"); try (InputStream data = storage.loadFile(name)) { + if (data == null) { + resp.setStatus(HttpServletResponse.SC_NO_CONTENT); + return; + } OutputStream out = resp.getOutputStream(); byte[] buffer = new byte[512]; int read = 0; @@ -280,6 +284,7 @@ out.write(buffer, 0, read); } } + resp.setStatus(HttpServletResponse.SC_OK); } }