From 50a64b94dfb8a4f4f576bfb3c61376d606913972 Mon Sep 17 00:00:00 2001 From: Gintautas Miselis Date: Thu, 11 May 2017 11:52:24 +0100 Subject: [PATCH 1/2] Rest::responseBodyBytes returns response body when status code is not 2XX --- .../java/com/bettercloud/vault/rest/Rest.java | 17 +++++++++++++++-- .../com/bettercloud/vault/rest/GetTests.java | 19 +++++++++++++++++++ 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/bettercloud/vault/rest/Rest.java b/src/main/java/com/bettercloud/vault/rest/Rest.java index 73a1bf6f..cc73c5e1 100644 --- a/src/main/java/com/bettercloud/vault/rest/Rest.java +++ b/src/main/java/com/bettercloud/vault/rest/Rest.java @@ -512,10 +512,23 @@ private String parametersToQueryString() { * * @param connection An active HTTP(S) connection * @return The body payload, downloaded from the HTTP connection response + * @throws RestException */ - private byte[] responseBodyBytes(final URLConnection connection) { + private byte[] responseBodyBytes(final URLConnection connection) throws RestException { try { - final InputStream inputStream = connection.getInputStream(); + final InputStream inputStream; + final int responseCode = this.connectionStatus(connection); + if (200 <= responseCode && responseCode <= 299) { + inputStream = connection.getInputStream(); + } else { + if (connection instanceof HttpsURLConnection) { + final HttpsURLConnection httpsURLConnection = (HttpsURLConnection) connection; + inputStream = httpsURLConnection.getErrorStream(); + } else { + final HttpURLConnection httpURLConnection = (HttpURLConnection) connection; + inputStream = httpURLConnection.getErrorStream(); + } + } final ByteArrayOutputStream byteArrayOutputStream = new ByteArrayOutputStream(); int bytesRead; final byte[] bytes = new byte[16384]; diff --git a/src/test/java/com/bettercloud/vault/rest/GetTests.java b/src/test/java/com/bettercloud/vault/rest/GetTests.java index a010b214..157fb27a 100644 --- a/src/test/java/com/bettercloud/vault/rest/GetTests.java +++ b/src/test/java/com/bettercloud/vault/rest/GetTests.java @@ -7,6 +7,7 @@ import java.io.UnsupportedEncodingException; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; /** * Unit tests relating the REST client processing of GET requests. @@ -131,4 +132,22 @@ public void testGet_WithHeaders() throws RestException, UnsupportedEncodingExcep assertEquals("Note+that+headers+are+send+in+url-encoded+format", headers.getString("Two-Part", null)); } + /** + *

Verify that response body is retrieved when http status is error code

+ * + * @throws RestException + * @throws UnsupportedEncodingException If there's a problem parsing the response JSON as UTF-8 + */ + @Test + public void testGet_RetrievesResponseBodyWhenStatusIs418() throws RestException, UnsupportedEncodingException { + final RestResponse restResponse = new Rest() + .url("http://httpbin.org/status/418") + .get(); + assertEquals(418, restResponse.getStatus()); + + final String responseBody = new String(restResponse.getBody(), "UTF-8"); + assertTrue("Response body is empty", responseBody.length() > 0); + assertTrue("Response body doesn't contain word teapot", responseBody.contains("teapot")); + } + } From d02b85292e1409fc614b751a0d12658f64c5a14a Mon Sep 17 00:00:00 2001 From: Gintautas Miselis Date: Thu, 11 May 2017 12:08:05 +0100 Subject: [PATCH 2/2] Logical methods include response body to exception message It eases debugging, Example of exception message: com.bettercloud.vault.VaultException: Expecting HTTP status 204 or 200, but instead receiving 403 Response body: {"errors":["permission denied"]} --- .../com/bettercloud/vault/api/Logical.java | 14 ++-- .../bettercloud/vault/api/LogicalTests.java | 81 +++++++++++++++++++ 2 files changed, 90 insertions(+), 5 deletions(-) diff --git a/src/main/java/com/bettercloud/vault/api/Logical.java b/src/main/java/com/bettercloud/vault/api/Logical.java index 302858c4..45268f1d 100644 --- a/src/main/java/com/bettercloud/vault/api/Logical.java +++ b/src/main/java/com/bettercloud/vault/api/Logical.java @@ -1,5 +1,6 @@ package com.bettercloud.vault.api; +import java.io.UnsupportedEncodingException; import java.util.ArrayList; import java.util.List; import java.util.Map; @@ -61,11 +62,12 @@ public LogicalResponse read(final String path) throws VaultException { // Validate response if (restResponse.getStatus() != 200) { - throw new VaultException("Vault responded with HTTP status code: " + restResponse.getStatus(), restResponse.getStatus()); + throw new VaultException("Vault responded with HTTP status code: " + restResponse.getStatus() + + "\nResponse body: " + new String(restResponse.getBody(), "UTF-8"), restResponse.getStatus()); } return new LogicalResponse(restResponse, retryCount); - } catch (RuntimeException | VaultException | RestException e) { + } catch (RuntimeException | VaultException | RestException | UnsupportedEncodingException e) { // If there are retries to perform, then pause for the configured interval and then execute the loop again... if (retryCount < config.getMaxRetries()) { retryCount++; @@ -130,7 +132,8 @@ public LogicalResponse write(final String path, final Map nameVa if (restStatus == 200 || restStatus == 204) { return new LogicalResponse(restResponse, retryCount); } else { - throw new VaultException("Expecting HTTP status 204 or 200, but instead receiving " + restStatus, restStatus); + throw new VaultException("Expecting HTTP status 204 or 200, but instead receiving " + restStatus + + "\nResponse body: " + new String(restResponse.getBody(), "UTF-8"), restStatus); } } catch (Exception e) { // If there are retries to perform, then pause for the configured interval and then execute the loop again... @@ -218,10 +221,11 @@ public LogicalResponse delete(final String path) throws VaultException { // Validate response if (restResponse.getStatus() != 204) { - throw new VaultException("Vault responded with HTTP status code: " + restResponse.getStatus(), restResponse.getStatus()); + throw new VaultException("Vault responded with HTTP status code: " + restResponse.getStatus() + + "\nResponse body: " + new String(restResponse.getBody(), "UTF-8"), restResponse.getStatus()); } return new LogicalResponse(restResponse, retryCount); - } catch (RuntimeException | VaultException | RestException e) { + } catch (RuntimeException | VaultException | RestException | UnsupportedEncodingException e) { // If there are retries to perform, then pause for the configured interval and then execute the loop again... if (retryCount < config.getMaxRetries()) { retryCount++; diff --git a/src/test-integration/java/com/bettercloud/vault/api/LogicalTests.java b/src/test-integration/java/com/bettercloud/vault/api/LogicalTests.java index 1c46b6d8..ed077e5b 100644 --- a/src/test-integration/java/com/bettercloud/vault/api/LogicalTests.java +++ b/src/test-integration/java/com/bettercloud/vault/api/LogicalTests.java @@ -6,11 +6,13 @@ import org.junit.Before; import org.junit.BeforeClass; +import org.junit.Rule; import org.junit.Test; import com.bettercloud.vault.Vault; import com.bettercloud.vault.VaultConfig; import com.bettercloud.vault.VaultException; +import org.junit.rules.ExpectedException; import static junit.framework.TestCase.*; @@ -136,4 +138,83 @@ public void testDelete() throws VaultException { assertFalse(vault.logical().list("secret").contains("hello")); } + @Rule + public ExpectedException expectedEx = ExpectedException.none(); + + /** + * Tests that exception message includes errors returned by Vault + * + * @throws VaultException + */ + @Test + public void testReadExceptionMessageIncludesErrorsReturnedByVault() throws VaultException { + expectedEx.expect(VaultException.class); + expectedEx.expectMessage("permission denied"); + + final VaultConfig config = new VaultConfig(address, "invalid-token"); + final Vault vault = new Vault(config); + vault.logical().read("secret/null"); + } + + /** + * Tests that exception message includes errors returned by Vault + * + * @throws VaultException + */ + @Test + public void testWriteExceptionMessageIncludesErrorsReturnedByVault() throws VaultException { + expectedEx.expect(VaultException.class); + expectedEx.expectMessage("permission denied"); + + final VaultConfig config = new VaultConfig(address, "invalid-token"); + final Vault vault = new Vault(config); + vault.logical().write("secret/null", new HashMap() {{ put("value", null); }}); + } + + /** + * Tests that exception message includes errors returned by Vault + * + * @throws VaultException + */ + @Test + public void testDeleteExceptionMessageIncludesErrorsReturnedByVault() throws VaultException { + expectedEx.expect(VaultException.class); + expectedEx.expectMessage("permission denied"); + + final VaultConfig config = new VaultConfig(address, "invalid-token"); + final Vault vault = new Vault(config); + vault.logical().delete("secret/null"); + } + + /** + * Tests that exception message includes errors returned by Vault + * + * @throws VaultException + */ + @Test + public void testListExceptionMessageIncludesErrorsReturnedByVault() throws VaultException { + expectedEx.expect(VaultException.class); + expectedEx.expectMessage("permission denied"); + + final VaultConfig config = new VaultConfig(address, "invalid-token"); + final Vault vault = new Vault(config); + vault.logical().list("secret/null"); + } + + /** + * Write a secret and verify that it can be read containing a null value. + * + * @throws VaultException + */ + @Test + public void testReadExceptionMessageIncludesErrorsReturnedByVaultOn404() throws VaultException { + expectedEx.expect(VaultException.class); + expectedEx.expectMessage("{\"errors\":[]}"); + + final VaultConfig config = new VaultConfig(address, token); + final Vault vault = new Vault(config); + + vault.logical().read("secret/null"); + } + }