Skip to content

Commit 0f31eaa

Browse files
authored
Fail MD5 validation if server MD5 is missing (#299)
Resolves #298 * Throw a checksum exception if checksum validation was requested but is impossible * Require entity to exist when validating checksum * StandardHttpHelperTest for validating response code and the fact that checksums are _required_ * Improve missing request entity error message * 100% test coverage of StandardHttpHelper#validateChecksum * Remove mistakenly-included template and extra newlines
1 parent 04ebb25 commit 0f31eaa

File tree

3 files changed

+206
-5
lines changed

3 files changed

+206
-5
lines changed

java-manta-client/src/main/java/com/joyent/manta/client/MantaClient.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1057,6 +1057,7 @@ public MantaObjectResponse put(final String rawPath,
10571057
final String string,
10581058
final MantaHttpHeaders headers,
10591059
final MantaMetadata metadata) throws IOException {
1060+
Validate.notNull(string, "String content must not be null");
10601061
Validate.notNull(rawPath, "Path must not be null");
10611062

10621063
String path = formatPath(rawPath);

java-manta-client/src/main/java/com/joyent/manta/http/StandardHttpHelper.java

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -351,13 +351,11 @@ protected static void validateChecksum(final DigestedEntity entity,
351351
final HttpRequest request,
352352
final HttpResponse response)
353353
throws MantaChecksumFailedException {
354-
if (entity == null) {
355-
return;
356-
}
354+
Validate.notNull(entity, "Request body required");
357355

358356
if (serverMd5 == null || serverMd5.length == 0) {
359-
LOGGER.warn("No cryptographic check performed by the server");
360-
return;
357+
final String msg = "Server calculated MD5 is missing";
358+
throw new MantaChecksumFailedException(msg, request, response);
361359
}
362360

363361
final byte[] clientMd5 = entity.getDigest();
Lines changed: 202 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,202 @@
1+
package com.joyent.manta.http;
2+
3+
import com.joyent.manta.client.MantaObjectResponse;
4+
import com.joyent.manta.config.BaseChainedConfigContext;
5+
import com.joyent.manta.config.StandardConfigContext;
6+
import com.joyent.manta.exception.MantaChecksumFailedException;
7+
import com.joyent.manta.exception.MantaClientHttpResponseException;
8+
import com.joyent.manta.http.entity.NoContentEntity;
9+
import com.twmacinta.util.FastMD5Digest;
10+
import org.apache.commons.io.output.NullOutputStream;
11+
import org.apache.commons.lang3.RandomUtils;
12+
import org.apache.commons.lang3.StringUtils;
13+
import org.apache.http.Header;
14+
import org.apache.http.HttpStatus;
15+
import org.apache.http.StatusLine;
16+
import org.apache.http.client.methods.CloseableHttpResponse;
17+
import org.apache.http.client.methods.HttpPut;
18+
import org.apache.http.entity.ByteArrayEntity;
19+
import org.apache.http.impl.client.CloseableHttpClient;
20+
import org.apache.http.message.BasicHeader;
21+
import org.bouncycastle.crypto.Digest;
22+
import org.mockito.Mock;
23+
import org.mockito.Mockito;
24+
import org.mockito.MockitoAnnotations;
25+
import org.mockito.internal.util.reflection.Whitebox;
26+
import org.testng.Assert;
27+
import org.testng.annotations.BeforeMethod;
28+
import org.testng.annotations.Test;
29+
30+
import java.util.Base64;
31+
32+
import static org.mockito.Matchers.any;
33+
import static org.mockito.Matchers.anyString;
34+
import static org.mockito.Mockito.mock;
35+
import static org.mockito.Mockito.reset;
36+
import static org.mockito.Mockito.when;
37+
38+
public class StandardHttpHelperTest {
39+
40+
@Mock
41+
private final MantaConnectionFactory connectionFactory = mock(MantaConnectionFactory.class);
42+
@Mock
43+
private final CloseableHttpClient client = mock(CloseableHttpClient.class);
44+
@Mock
45+
private final CloseableHttpResponse response = mock(CloseableHttpResponse.class);
46+
@Mock
47+
private final MantaConnectionContext connCtx = mock(MantaConnectionContext.class);
48+
@Mock
49+
private final StatusLine statusLine = mock(StatusLine.class);
50+
51+
private BaseChainedConfigContext config;
52+
53+
@BeforeMethod
54+
public void setup() throws Exception {
55+
MockitoAnnotations.initMocks(this);
56+
config = new StandardConfigContext().setMantaURL("");
57+
58+
Whitebox.setInternalState(connectionFactory, "config", config);
59+
60+
when(connCtx.getHttpClient())
61+
.thenReturn(client);
62+
63+
when(client.execute(any()))
64+
.thenReturn(response);
65+
66+
when(response.getStatusLine())
67+
.thenReturn(statusLine);
68+
69+
when(response.getAllHeaders())
70+
.thenReturn(new Header[]{});
71+
72+
when(connectionFactory.uriForPath(anyString()))
73+
.thenCallRealMethod();
74+
75+
when(connectionFactory.put(Mockito.anyString()))
76+
.thenCallRealMethod();
77+
}
78+
79+
@Test
80+
public void testHttpPutValidatesResponseCodeSuccessfully() throws Exception {
81+
when(statusLine.getStatusCode())
82+
.thenReturn(HttpStatus.SC_NO_CONTENT);
83+
84+
when(statusLine.getReasonPhrase())
85+
.thenReturn("No Content");
86+
87+
config.setVerifyUploads(false);
88+
final StandardHttpHelper helper = new StandardHttpHelper(connCtx, connectionFactory, config);
89+
90+
final MantaObjectResponse put =
91+
helper.httpPut("/path", null, NoContentEntity.INSTANCE, null);
92+
93+
Assert.assertNotNull(put);
94+
}
95+
96+
@Test
97+
public void testHttpPutValidatesResponseCodeAndThrowsWhenInvalid() throws Exception {
98+
when(statusLine.getStatusCode())
99+
.thenReturn(HttpStatus.SC_OK);
100+
101+
when(statusLine.getReasonPhrase())
102+
.thenReturn("OK");
103+
104+
final StandardHttpHelper helper = new StandardHttpHelper(connCtx, connectionFactory, config);
105+
106+
Assert.assertThrows(MantaClientHttpResponseException.class, () ->
107+
helper.httpPut("/path", null, NoContentEntity.INSTANCE, null));
108+
}
109+
110+
@Test
111+
public void testHttpPutChecksumsSuccessfully() throws Exception {
112+
when(statusLine.getStatusCode())
113+
.thenReturn(HttpStatus.SC_NO_CONTENT);
114+
115+
when(statusLine.getReasonPhrase())
116+
.thenReturn("No Content");
117+
118+
reset(client);
119+
when(client.execute(any()))
120+
.then((invocationOnMock) -> {
121+
// checksum is calculated as entity is written to network
122+
final HttpPut request = invocationOnMock.getArgumentAt(0, HttpPut.class);
123+
request.getEntity().writeTo(NullOutputStream.NULL_OUTPUT_STREAM);
124+
return response;
125+
});
126+
127+
final byte[] contentBytes = RandomUtils.nextBytes(100);
128+
final Digest digest = new FastMD5Digest();
129+
final byte[] checksumBytes = new byte[digest.getDigestSize()];
130+
digest.update(contentBytes, 0, contentBytes.length);
131+
digest.doFinal(checksumBytes, 0);
132+
133+
when(response.getAllHeaders())
134+
.thenReturn(new Header[]{
135+
new BasicHeader(
136+
MantaHttpHeaders.COMPUTED_MD5,
137+
Base64.getEncoder().encodeToString(checksumBytes))
138+
});
139+
140+
final StandardHttpHelper helper = new StandardHttpHelper(connCtx, connectionFactory, config);
141+
142+
// it's the default but let's just be explicit
143+
config.setVerifyUploads(true);
144+
145+
final MantaObjectResponse put =
146+
helper.httpPut("/path", null, new ByteArrayEntity(contentBytes), null);
147+
148+
Assert.assertNotNull(put);
149+
}
150+
151+
@Test
152+
public void testHttpPutChecksumsCompareDifferentlyFails() throws Exception {
153+
when(statusLine.getStatusCode())
154+
.thenReturn(HttpStatus.SC_NO_CONTENT);
155+
156+
when(statusLine.getReasonPhrase())
157+
.thenReturn("No Content");
158+
159+
reset(client);
160+
when(client.execute(any()))
161+
.then((invocationOnMock) -> {
162+
// checksum is calculated as entity is written to network
163+
final HttpPut request = invocationOnMock.getArgumentAt(0, HttpPut.class);
164+
request.getEntity().writeTo(NullOutputStream.NULL_OUTPUT_STREAM);
165+
return response;
166+
});
167+
168+
final byte[] contentBytes = StringUtils.repeat('a', 100).getBytes();
169+
170+
when(response.getAllHeaders())
171+
.thenReturn(new Header[]{
172+
new BasicHeader(
173+
MantaHttpHeaders.COMPUTED_MD5,
174+
"YmFzZTY0Cg==") // "base64" encoded in base64
175+
});
176+
177+
final StandardHttpHelper helper = new StandardHttpHelper(connCtx, connectionFactory, config);
178+
179+
// it's the default but let's just be explicit
180+
config.setVerifyUploads(true);
181+
182+
Assert.assertThrows(MantaChecksumFailedException.class, () ->
183+
helper.httpPut("/path", null, new ByteArrayEntity(contentBytes), null));
184+
}
185+
186+
@Test
187+
public void testHttpPutThrowsWhenChecksumRequestedButNotReturned() throws Exception {
188+
when(statusLine.getStatusCode())
189+
.thenReturn(HttpStatus.SC_NO_CONTENT);
190+
191+
when(statusLine.getReasonPhrase())
192+
.thenReturn("No Content");
193+
194+
final StandardHttpHelper helper = new StandardHttpHelper(connCtx, connectionFactory, config);
195+
196+
// it's the default but let's just be explicit
197+
config.setVerifyUploads(true);
198+
199+
Assert.assertThrows(MantaChecksumFailedException.class, () ->
200+
helper.httpPut("/path", null, NoContentEntity.INSTANCE, null));
201+
}
202+
}

0 commit comments

Comments
 (0)