diff --git a/docs/Config.md b/docs/Config.md index 43e9322..e0db539 100644 --- a/docs/Config.md +++ b/docs/Config.md @@ -37,6 +37,7 @@ Each target may have the following properties: * `access_key_path`: Path to the S3 access key (for private buckets) * `secret_key_path`: Path to the S3 secret key (for private buckets) * `config`: Botocore configuration options (see below) + * `proxy_etag`: If true, the backend's `ETag` is proxied through to clients (default: false). Most S3-compatible backends (e.g. VAST) return an `ETag` that is not a true content MD5, which causes the AWS CLI/SDK download integrity check to fail with "Unable to verify integrity of data download". Only set this to true for backends known to return a real content-MD5 `ETag` (e.g. real AWS S3). * *file*: Local filesystem targets. Options: * `path`: Path to the root * `buffer_size`: Size of chunks (in bytes) when streaming file content (default: 8192) diff --git a/tests/java/pom.xml b/tests/java/pom.xml index e38212f..51a131c 100644 --- a/tests/java/pom.xml +++ b/tests/java/pom.xml @@ -13,6 +13,8 @@ 17 UTF-8 2.25.60 + + 1.12.780 @@ -21,6 +23,16 @@ s3 ${aws.sdk.version} + + com.amazonaws + aws-java-sdk-s3 + ${aws.sdk.v1.version} + + + com.google.code.gson + gson + 2.11.0 + junit junit diff --git a/tests/java/src/test/java/org/janelia/x2s3/S3CompatTest.java b/tests/java/src/test/java/org/janelia/x2s3/S3CompatTest.java index 6bb72ed..63f1221 100644 --- a/tests/java/src/test/java/org/janelia/x2s3/S3CompatTest.java +++ b/tests/java/src/test/java/org/janelia/x2s3/S3CompatTest.java @@ -73,7 +73,9 @@ public void testListObjectsV2Basic() { for (int i = 0; i < awsObjects.size(); i++) { assertEquals("Key[" + i + "]", awsObjects.get(i).key(), proxyObjects.get(i).key()); assertEquals("Size[" + i + "]", awsObjects.get(i).size(), proxyObjects.get(i).size()); - assertEquals("ETag[" + i + "]", awsObjects.get(i).eTag(), proxyObjects.get(i).eTag()); + // ETag is not proxied by default (proxy_etag defaults to false), since many + // S3-compatible backends return an ETag that isn't a real content MD5. + assertNull("ETag[" + i + "] should not be proxied by default", proxyObjects.get(i).eTag()); assertEquals("StorageClass[" + i + "]", awsObjects.get(i).storageClassAsString(), proxyObjects.get(i).storageClassAsString()); @@ -212,9 +214,9 @@ public void testGetObject() { assertEquals("Content-Length", awsResp.response().contentLength(), proxyResp.response().contentLength()); - assertEquals("ETag", - awsResp.response().eTag(), - proxyResp.response().eTag()); + // ETag is not proxied by default (proxy_etag defaults to false), since many + // S3-compatible backends return an ETag that isn't a real content MD5. + assertNull("ETag should not be proxied by default", proxyResp.response().eTag()); // Compare body content try { @@ -287,8 +289,9 @@ public void testHeadObject() { awsResp.contentType(), proxyResp.contentType()); assertEquals("Content-Length", awsResp.contentLength(), proxyResp.contentLength()); - assertEquals("ETag", - awsResp.eTag(), proxyResp.eTag()); + // ETag is not proxied by default (proxy_etag defaults to false), since many + // S3-compatible backends return an ETag that isn't a real content MD5. + assertNull("ETag should not be proxied by default", proxyResp.eTag()); assertEquals("Last-Modified", awsResp.lastModified(), proxyResp.lastModified()); } diff --git a/tests/java/src/test/java/org/janelia/x2s3/S3v1IntegrityTest.java b/tests/java/src/test/java/org/janelia/x2s3/S3v1IntegrityTest.java new file mode 100644 index 0000000..bfde52f --- /dev/null +++ b/tests/java/src/test/java/org/janelia/x2s3/S3v1IntegrityTest.java @@ -0,0 +1,137 @@ +package org.janelia.x2s3; + +import org.junit.BeforeClass; +import org.junit.Test; +import static org.junit.Assert.*; + +import com.amazonaws.auth.AWSStaticCredentialsProvider; +import com.amazonaws.auth.AnonymousAWSCredentials; +import com.amazonaws.client.builder.AwsClientBuilder; +import com.amazonaws.services.s3.AmazonS3; +import com.amazonaws.services.s3.AmazonS3ClientBuilder; +import com.amazonaws.services.s3.model.ObjectMetadata; +import com.amazonaws.services.s3.model.S3Object; +import com.amazonaws.util.IOUtils; +import com.google.gson.JsonParser; + +import java.nio.charset.StandardCharsets; +import java.security.MessageDigest; + +/** + * Reproduces the "Unable to verify integrity of data download" error reported by + * N5/Zarr readers (org.janelia.saalfeldlab.n5.s3), which use the AWS SDK v1 S3 client. + * SDK v1 validates GetObject downloads by comparing a locally computed MD5 against the + * ETag header -- unless the ETag is absent, looks like a multipart ETag (has a "-N" + * suffix), or the object is SSE-C/SSE-KMS encrypted. That's a different code path than + * the SDK v2 client used in {@link S3CompatTest}, which does not do this validation. + * + * Point this at the same x2s3 target/key that failed in Fiji/BigDataViewer to confirm + * whether setting `proxy_etag: false` on that target (the default since this fix) + * resolves the integrity error, and separately whether the downloaded bytes are + * actually intact. + * + * Configure via env vars: + * PROXY_ENDPOINT (default http://localhost:8000) + * TEST_BUCKET (default janelia-data-examples) + * TEST_KEY (default jrc_mus_lung_covid.n5/attributes.json) + */ +public class S3v1IntegrityTest { + + private static AmazonS3 client; + private static String bucket; + private static String key; + + @BeforeClass + public static void setup() { + String endpoint = System.getenv().getOrDefault("PROXY_ENDPOINT", "http://localhost:8000"); + bucket = System.getenv().getOrDefault("TEST_BUCKET", "janelia-data-examples"); + key = System.getenv().getOrDefault("TEST_KEY", "jrc_mus_lung_covid.n5/attributes.json"); + + client = AmazonS3ClientBuilder.standard() + .withEndpointConfiguration(new AwsClientBuilder.EndpointConfiguration(endpoint, "us-east-1")) + .withCredentials(new AWSStaticCredentialsProvider(new AnonymousAWSCredentials())) + .withPathStyleAccessEnabled(true) + .build(); + } + + @Test + public void testGetObjectPassesIntegrityCheck() throws Exception { + // This is the exact path that threw SdkClientException in Fiji: reading the + // object body fully triggers DigestValidationInputStream's end-of-stream MD5 + // comparison against the ETag. + S3Object obj = client.getObject(bucket, key); + byte[] body; + try { + body = IOUtils.toByteArray(obj.getObjectContent()); + } finally { + obj.close(); + } + assertTrue("Body should not be empty", body.length > 0); + } + + @Test + public void testDownloadedJsonIsWellFormed() throws Exception { + // Independent of the ETag check: verify the bytes we actually got are intact + // and not truncated/corrupted, since that's a separate failure mode from the + // ETag mismatch (and was seen in the Fiji trace as a MalformedJsonException). + S3Object obj = client.getObject(bucket, key); + byte[] body; + try { + body = IOUtils.toByteArray(obj.getObjectContent()); + } finally { + obj.close(); + } + String text = new String(body, StandardCharsets.UTF_8).trim(); + assertTrue("Response should look like JSON, got: " + text, + text.startsWith("{") && text.endsWith("}")); + // Throws JsonSyntaxException if malformed, same as the Zarr reader. + JsonParser.parseString(text); + } + + @Test + public void testContentLengthMatchesActualBytes() throws Exception { + ObjectMetadata meta = client.getObjectMetadata(bucket, key); + S3Object obj = client.getObject(bucket, key); + byte[] body; + try { + body = IOUtils.toByteArray(obj.getObjectContent()); + } finally { + obj.close(); + } + assertEquals("Downloaded byte count should match Content-Length", + meta.getContentLength(), body.length); + } + + @Test + public void testReportedETagVsActualMd5() throws Exception { + // Purely diagnostic: disable v1 SDK's own validation so a mismatch here prints + // instead of throwing, to confirm whether the backend's ETag is a real content + // MD5 at all. + System.setProperty("com.amazonaws.services.s3.disableGetObjectMD5Validation", "true"); + try { + ObjectMetadata meta = client.getObjectMetadata(bucket, key); + String reportedETag = meta.getETag(); + + S3Object obj = client.getObject(bucket, key); + byte[] body; + try { + body = IOUtils.toByteArray(obj.getObjectContent()); + } finally { + obj.close(); + } + + MessageDigest md5 = MessageDigest.getInstance("MD5"); + StringBuilder sb = new StringBuilder(); + for (byte b : md5.digest(body)) { + sb.append(String.format("%02x", b & 0xff)); + } + String actualMd5 = sb.toString(); + + System.out.println("Reported ETag: " + reportedETag); + System.out.println("Actual MD5: " + actualMd5); + System.out.println("Bytes read: " + body.length); + } finally { + System.clearProperty("com.amazonaws.services.s3.disableGetObjectMD5Validation"); + } + } +} diff --git a/tests/test_boto.py b/tests/test_boto.py index 93f285c..be3f785 100644 --- a/tests/test_boto.py +++ b/tests/test_boto.py @@ -19,6 +19,10 @@ def get_settings(): Target( name='janelia-data-examples', options={'bucket':'janelia-data-examples'} + ), + Target( + name='janelia-data-examples-with-etag', + options={'bucket':'janelia-data-examples', 'proxy_etag': True} ) ] return settings @@ -108,6 +112,15 @@ def test_head_object(app, s3_client): def test_get_object(app, s3_client): response = s3_client.get_object(Bucket='janelia-data-examples', Key='jrc_mus_lung_covid.n5/attributes.json') assert response['ResponseMetadata']['HTTPStatusCode'] == 200 + assert 'ETag' not in response + json_obj = response['Body'].read().decode('utf-8') + assert 'n5' in json_obj + + +def test_get_object_with_etag(app, s3_client): + response = s3_client.get_object(Bucket='janelia-data-examples-with-etag', Key='jrc_mus_lung_covid.n5/attributes.json') + assert response['ResponseMetadata']['HTTPStatusCode'] == 200 + assert 'ETag' in response json_obj = response['Body'].read().decode('utf-8') assert 'n5' in json_obj diff --git a/x2s3/client_aioboto.py b/x2s3/client_aioboto.py index 71dab72..116a9b8 100644 --- a/x2s3/client_aioboto.py +++ b/x2s3/client_aioboto.py @@ -73,6 +73,11 @@ def __init__(self, proxy_kwargs, **kwargs): self.bucket_name = kwargs['bucket'] self.bucket_prefix = kwargs.get('prefix') + # Some S3-compatible backends (e.g. VAST) return ETags that are not a + # true content MD5, which breaks the AWS CLI/SDK download integrity + # check. Allow disabling ETag proxying per-target for those backends. + self.proxy_etag = kwargs.get('proxy_etag', False) + self.anonymous = True access_key,secret_key = '','' @@ -162,12 +167,14 @@ async def head_object(self, key: str): try: s3_res = await self.client.head_object(Bucket=self.bucket_name, Key=real_key) headers = { - "ETag": s3_res.get("ETag"), "Accept-Ranges": "bytes", "Content-Length": str(s3_res.get("ContentLength")), "Last-Modified": s3_res.get("LastModified").strftime("%a, %d %b %Y %H:%M:%S GMT"), } + if self.proxy_etag: + headers["ETag"] = s3_res.get("ETag") + content_type = guess_content_type(real_key) headers['Content-Type'] = content_type @@ -226,7 +233,7 @@ async def open_object(self, key: str, range_header: str = None): if "last-modified" in res_headers: headers["Last-Modified"] = res_headers["last-modified"] - if "etag" in res_headers: + if self.proxy_etag and "etag" in res_headers: headers["ETag"] = res_headers["etag"] return S3ObjectHandle( @@ -314,7 +321,7 @@ async def list_objects_v2(self, contents.append({ 'Key': remove_prefix(self.bucket_prefix, obj["Key"]), 'LastModified': obj["LastModified"].strftime("%Y-%m-%dT%H:%M:%S.000Z"), - 'ETag': obj.get("ETag"), + 'ETag': obj.get("ETag") if self.proxy_etag else None, 'Size': obj.get("Size"), 'StorageClass': obj.get("StorageClass") })