From e987da8f6cef57cc45418236268251c688ba5864 Mon Sep 17 00:00:00 2001 From: Patrick Dowler Date: Thu, 11 Apr 2024 17:52:00 -0700 Subject: [PATCH 1/4] minoc intTest fix --- .../opencadc/minoc/FitsOperationsTest.java | 22 ++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/minoc/src/intTest/java/org/opencadc/minoc/FitsOperationsTest.java b/minoc/src/intTest/java/org/opencadc/minoc/FitsOperationsTest.java index 2efd1888..bc462bce 100644 --- a/minoc/src/intTest/java/org/opencadc/minoc/FitsOperationsTest.java +++ b/minoc/src/intTest/java/org/opencadc/minoc/FitsOperationsTest.java @@ -134,7 +134,7 @@ public FitsOperationsTest() throws Exception { super(); final RegistryClient regClient = new RegistryClient(); - filesVaultURL = new URL(regClient.getServiceURL(VOSPACE_URI, Standards.VOSPACE_FILES_20, AuthMethod.ANON) + filesVaultURL = new URL(regClient.getServiceURL(VOSPACE_URI, Standards.VOSPACE_FILES, AuthMethod.ANON) + "/CADC/test-data/cutouts"); DEFAULT_DATA_PATH.toFile().mkdirs(); } @@ -149,6 +149,26 @@ public void testCompressed() throws Exception { }; uploadAndCompareCutout(artifactURI, SodaParamValidator.SUB, cutoutSpecs, testFilePrefix); + + LOGGER.info("unset content-type and try again: rely on filename extension only..."); + final URI noclArtifactURI = URI.create("cadc:TEST/" + testFilePrefix + "-nocl." + testFileExtension); + final URL noclArtifactURL = new URL(filesURL + "/" + noclArtifactURI.toString()); + LOGGER.info("no content-length: " + noclArtifactURL); + + try { + setContentType = false; + uploadAndCompareCutout(noclArtifactURI, SodaParamValidator.SUB, cutoutSpecs, testFilePrefix); + } finally { + setContentType = true; + } + + Subject.doAs(userSubject, (PrivilegedExceptionAction) () -> { + HttpGet head = new HttpGet(noclArtifactURL, false); + head.setHeadOnly(true); + head.prepare(); + Assert.assertNull("no content type", head.getResponseHeader("content-type")); + return null; + }); } @Test From b579fe0d66017cf21aadcbace98059e25964b0eb Mon Sep 17 00:00:00 2001 From: Patrick Dowler Date: Fri, 12 Apr 2024 11:07:57 -0700 Subject: [PATCH 2/4] minoc: work around for bad Artifact.contentType and fits --- minoc/VERSION | 2 +- .../java/org/opencadc/minoc/BasicOpsTest.java | 15 +++++- .../opencadc/minoc/FitsOperationsTest.java | 54 +++++++++++-------- .../minoc/IncorrectPutMetadataTest.java | 54 +++++++++++++++++++ .../org/opencadc/minoc/ArtifactAction.java | 15 ++++++ .../java/org/opencadc/minoc/GetAction.java | 27 ++++++++-- .../java/org/opencadc/minoc/PostAction.java | 12 ++--- .../java/org/opencadc/minoc/PutAction.java | 6 ++- 8 files changed, 145 insertions(+), 40 deletions(-) diff --git a/minoc/VERSION b/minoc/VERSION index c5fd5316..5ff92dd7 100644 --- a/minoc/VERSION +++ b/minoc/VERSION @@ -4,6 +4,6 @@ # tags with and without build number so operators use the versioned # tag but we always keep a timestamped tag in case a semantic tag gets # replaced accidentally -VER=1.0.3 +VER=1.0.4 TAGS="${VER} ${VER}-$(date --utc +"%Y%m%dT%H%M%S")" unset VER diff --git a/minoc/src/intTest/java/org/opencadc/minoc/BasicOpsTest.java b/minoc/src/intTest/java/org/opencadc/minoc/BasicOpsTest.java index 36a6552f..f5df1ff9 100644 --- a/minoc/src/intTest/java/org/opencadc/minoc/BasicOpsTest.java +++ b/minoc/src/intTest/java/org/opencadc/minoc/BasicOpsTest.java @@ -134,6 +134,7 @@ public void testPutGetUpdateHeadDelete() { log.info("headers: " + put.getResponseHeader("content-length") + " " + put.getResponseHeader("digest")); Assert.assertNull(put.getThrowable()); Assert.assertEquals("Created", 201, put.getResponseCode()); + Assert.assertEquals(0L, put.getContentLength()); // get OutputStream out = new ByteArrayOutputStream(); @@ -166,9 +167,21 @@ public void testPutGetUpdateHeadDelete() { post.setDigest(computeChecksumURI(data)); Subject.doAs(userSubject, new RunnableAction(post)); log.info("post: " + post.getResponseCode() + " " + post.getThrowable()); - log.info("headers: " + post.getResponseHeader("content-length") + " " + post.getResponseHeader("digest")); Assert.assertNull(post.getThrowable()); Assert.assertEquals("Accepted", 202, post.getResponseCode()); + Assert.assertEquals(newEncoding, post.getResponseHeader(HttpTransfer.CONTENT_ENCODING)); + Assert.assertEquals(newType, post.getResponseHeader(HttpTransfer.CONTENT_TYPE)); + + // invalid update + String invalidType = "bogus"; + params.clear(); + params.put("contentType", invalidType); + post = new HttpPost(artifactURL, params, false); + post.setDigest(computeChecksumURI(data)); + Subject.doAs(userSubject, new RunnableAction(post)); + log.info("post: " + post.getResponseCode() + " " + post.getThrowable()); + Assert.assertNotNull(post.getThrowable()); + Assert.assertEquals("Rejected", 400, post.getResponseCode()); // head ByteArrayOutputStream bos = new ByteArrayOutputStream(); diff --git a/minoc/src/intTest/java/org/opencadc/minoc/FitsOperationsTest.java b/minoc/src/intTest/java/org/opencadc/minoc/FitsOperationsTest.java index bc462bce..48f874a3 100644 --- a/minoc/src/intTest/java/org/opencadc/minoc/FitsOperationsTest.java +++ b/minoc/src/intTest/java/org/opencadc/minoc/FitsOperationsTest.java @@ -71,7 +71,6 @@ import ca.nrc.cadc.auth.AuthMethod; import ca.nrc.cadc.net.HttpDelete; import ca.nrc.cadc.net.HttpGet; -import ca.nrc.cadc.net.HttpPost; import ca.nrc.cadc.net.HttpTransfer; import ca.nrc.cadc.net.HttpUpload; import ca.nrc.cadc.net.NetUtil; @@ -80,16 +79,6 @@ import ca.nrc.cadc.reg.client.RegistryClient; import ca.nrc.cadc.util.Log4jInit; import ca.nrc.cadc.util.StringUtil; -import nom.tam.fits.Fits; -import nom.tam.util.RandomAccessFileIO; -import org.apache.log4j.Level; -import org.apache.log4j.Logger; -import org.junit.Assert; -import org.junit.Test; -import org.opencadc.fits.RandomAccessStorageObject; -import org.opencadc.soda.SodaParamValidator; - -import javax.security.auth.Subject; import java.io.File; import java.io.FileInputStream; import java.io.FileOutputStream; @@ -100,9 +89,15 @@ import java.nio.file.Path; import java.security.PrivilegedExceptionAction; import java.util.Arrays; -import java.util.List; -import java.util.Map; -import java.util.TreeMap; +import javax.security.auth.Subject; +import nom.tam.fits.Fits; +import nom.tam.util.RandomAccessFileIO; +import org.apache.log4j.Level; +import org.apache.log4j.Logger; +import org.junit.Assert; +import org.junit.Test; +import org.opencadc.fits.RandomAccessStorageObject; +import org.opencadc.soda.SodaParamValidator; /** * Integration test to pull existing test FITS files from VOSpace (Vault) into a local directory, then PUT them into @@ -122,8 +117,7 @@ public class FitsOperationsTest extends MinocTest { protected URL filesVaultURL; - // normally true except for one test - private boolean setContentType = true; + private String putContentType = "application/fits"; static { Log4jInit.setLevel("org.opencadc.minoc", Level.INFO); @@ -156,10 +150,10 @@ public void testCompressed() throws Exception { LOGGER.info("no content-length: " + noclArtifactURL); try { - setContentType = false; + putContentType = null; uploadAndCompareCutout(noclArtifactURI, SodaParamValidator.SUB, cutoutSpecs, testFilePrefix); } finally { - setContentType = true; + putContentType = "application/fits"; } Subject.doAs(userSubject, (PrivilegedExceptionAction) () -> { @@ -224,20 +218,36 @@ public void testSimple() throws Exception { LOGGER.info("no content-length: " + noclArtifactURL); try { - setContentType = false; + putContentType = null; uploadAndCompareCutout(noclArtifactURI, SodaParamValidator.SUB, cutoutSpecs, testFilePrefix); } finally { - setContentType = true; + putContentType = "application/fits"; } Subject.doAs(userSubject, (PrivilegedExceptionAction) () -> { HttpGet head = new HttpGet(noclArtifactURL, false); head.setHeadOnly(true); head.prepare(); + LOGGER.info("null content-type test: " + head.getContentType()); Assert.assertNull("no content type", head.getResponseHeader("content-type")); return null; }); + try { + putContentType = "application/octet-stream"; + uploadAndCompareCutout(noclArtifactURI, SodaParamValidator.SUB, cutoutSpecs, testFilePrefix); + } finally { + putContentType = "application/fits"; + } + + Subject.doAs(userSubject, (PrivilegedExceptionAction) () -> { + HttpGet head = new HttpGet(noclArtifactURL, false); + head.setHeadOnly(true); + head.prepare(); + LOGGER.info("useless content-type test: " + head.getContentType()); + Assert.assertEquals("content type", "application/octet-stream", head.getResponseHeader("content-type")); + return null; + }); } @Test @@ -510,8 +520,8 @@ private void ensureFile(final URI artifactURI) throws Exception { final HttpUpload upload = new HttpUpload(fileInputStream, artifactURL); upload.setRequestProperty("X-Test-Method", fileName); upload.setRequestProperty(HttpTransfer.CONTENT_LENGTH, Long.toString(localFile.length())); - if (setContentType) { - upload.setRequestProperty(HttpTransfer.CONTENT_TYPE, "application/fits"); + if (putContentType != null) { + upload.setRequestProperty(HttpTransfer.CONTENT_TYPE, putContentType); } upload.run(); LOGGER.info("response code: " + upload.getResponseCode() + " " + upload.getThrowable()); diff --git a/minoc/src/intTest/java/org/opencadc/minoc/IncorrectPutMetadataTest.java b/minoc/src/intTest/java/org/opencadc/minoc/IncorrectPutMetadataTest.java index 6a221c9b..2799b721 100644 --- a/minoc/src/intTest/java/org/opencadc/minoc/IncorrectPutMetadataTest.java +++ b/minoc/src/intTest/java/org/opencadc/minoc/IncorrectPutMetadataTest.java @@ -108,6 +108,60 @@ public IncorrectPutMetadataTest() { super(); } + @Test + public void testUploadInvalidContentType() { + try { + + Subject.doAs(userSubject, new PrivilegedExceptionAction() { + public Object run() throws Exception { + + byte[] bytes = randomData(16384); // 16k + final String incorrectData = "incorrect artifact"; + URI artifactURI = URI.create("cadc:TEST/testUploadInvalidContentType"); + URL artifactURL = new URL(filesURL + "/" + artifactURI.toString()); + + // cleanup + HttpDelete del = new HttpDelete(artifactURL, false); + del.run(); + // verify + HttpGet head = new HttpGet(artifactURL, false); + head.setHeadOnly(true); + try { + head.prepare(); + Assert.fail("cleanup failed -- file exists: " + artifactURI); + } catch (ResourceNotFoundException ok) { + log.info("verify not found: " + artifactURL); + } + + // put file + InputStream in = new ByteArrayInputStream(bytes); + HttpUpload put = new HttpUpload(in, artifactURL); + put.setDigest(computeChecksumURI(incorrectData.getBytes())); + put.setRequestProperty(HttpTransfer.CONTENT_TYPE, "None"); + put.run(); + log.info("response code: " + put.getResponseCode() + " " + put.getThrowable()); + Assert.assertEquals("should be 400", 400, put.getResponseCode()); + + // verify + head = new HttpGet(artifactURL, false); + head.setHeadOnly(true); + try { + head.prepare(); + Assert.fail("put succeeded -- file exists: " + artifactURI); + } catch (ResourceNotFoundException ok) { + log.info("verify not found: " + artifactURL); + } + + return null; + } + }); + + } catch (Exception t) { + log.error("unexpected throwable", t); + Assert.fail("unexpected throwable: " + t); + } + } + @Test public void testUploadContentMD5_Mismatch() { try { diff --git a/minoc/src/main/java/org/opencadc/minoc/ArtifactAction.java b/minoc/src/main/java/org/opencadc/minoc/ArtifactAction.java index 7e51264c..f5594a40 100644 --- a/minoc/src/main/java/org/opencadc/minoc/ArtifactAction.java +++ b/minoc/src/main/java/org/opencadc/minoc/ArtifactAction.java @@ -70,6 +70,7 @@ import ca.nrc.cadc.auth.AuthenticationUtil; import ca.nrc.cadc.auth.HttpPrincipal; import ca.nrc.cadc.log.WebServiceLogInfo; +import ca.nrc.cadc.net.ContentType; import ca.nrc.cadc.net.ResourceNotFoundException; import ca.nrc.cadc.net.TransientException; import ca.nrc.cadc.rest.InlineContentHandler; @@ -329,4 +330,18 @@ Artifact getArtifact(URI artifactURI) throws ResourceNotFoundException { } return artifact; } + + // validate and return canonical form + public static String validateContentType(String s) { + if (s == null) { + return null; + } + ContentType ct = new ContentType(s); + String base = ct.getBaseType(); + String[] mm = base.split("/"); + if (mm.length != 2) { + throw new IllegalArgumentException("invalid content-type: '" + s + "' reason: not of form {type}/{subtype}[;{params}]"); + } + return ct.getValue(); + } } diff --git a/minoc/src/main/java/org/opencadc/minoc/GetAction.java b/minoc/src/main/java/org/opencadc/minoc/GetAction.java index f58b687f..e774e779 100644 --- a/minoc/src/main/java/org/opencadc/minoc/GetAction.java +++ b/minoc/src/main/java/org/opencadc/minoc/GetAction.java @@ -74,6 +74,7 @@ import ca.nrc.cadc.io.ByteCountOutputStream; import ca.nrc.cadc.io.ReadException; import ca.nrc.cadc.io.WriteException; +import ca.nrc.cadc.net.HttpGet; import ca.nrc.cadc.net.HttpTransfer; import ca.nrc.cadc.net.RangeNotSatisfiableException; import ca.nrc.cadc.net.ResourceNotFoundException; @@ -114,16 +115,22 @@ public class GetAction extends ArtifactAction { private static final String RANGE = "range"; private static final String CONTENT_DISPOSITION = "content-disposition"; private static final String CONTENT_RANGE = "content-range"; - private static final String CONTENT_LENGTH = "content-length"; + private static final String CONTENT_LENGTH = HttpTransfer.CONTENT_LENGTH; private static final String FITS_CONTENT_TYPE = "application/fits"; private static final String[] FITS_CONTENT_TYPES = new String[] { FITS_CONTENT_TYPE, "image/fits" // alternative }; + private static final String[] EFFECTIVE_NULL_TYPES = new String[] { + "application/octet-stream", + "NULL", // existing bad content + "None" // existing bad content + }; private static final String[] FITS_EXTENSIONS = new String[] { - ".fits", - ".fz" + ".fits", + ".fits.fz", + ".fz" //?? }; private final SodaParamValidator sodaParamValidator = new SodaParamValidator(); @@ -388,12 +395,22 @@ private T assertSingleWCS(final String key, final List wcsValues) { } private boolean isFITS(final Artifact artifact) { - final String contentType = (artifact.contentType != null - ? artifact.contentType : getContentTypeFromFilename(artifact.getURI().getSchemeSpecificPart())); + String effectiveType = getEffectiveContentType(artifact.contentType); + final String contentType = (effectiveType != null + ? effectiveType : getContentTypeFromFilename(artifact.getURI().getSchemeSpecificPart())); return StringUtil.hasText(contentType) && Arrays.stream(FITS_CONTENT_TYPES).anyMatch(s -> s.equals(contentType)); } + private String getEffectiveContentType(String s) { + for (String en : EFFECTIVE_NULL_TYPES) { + if (en.equals(s)) { + return null; + } + } + return s; + } + private String getContentTypeFromFilename(String filename) { for (String ext : FITS_EXTENSIONS) { if (filename.endsWith(ext)) { diff --git a/minoc/src/main/java/org/opencadc/minoc/PostAction.java b/minoc/src/main/java/org/opencadc/minoc/PostAction.java index e6a6cb7e..e499c77b 100644 --- a/minoc/src/main/java/org/opencadc/minoc/PostAction.java +++ b/minoc/src/main/java/org/opencadc/minoc/PostAction.java @@ -68,12 +68,14 @@ package org.opencadc.minoc; import ca.nrc.cadc.db.TransactionManager; +import ca.nrc.cadc.net.ContentType; import ca.nrc.cadc.profiler.Profiler; import org.apache.log4j.Logger; import org.opencadc.inventory.Artifact; import org.opencadc.inventory.db.EntityNotFoundException; import org.opencadc.inventory.storage.PutTransaction; import org.opencadc.inventory.storage.StorageMetadata; +import static org.opencadc.minoc.ArtifactAction.validateContentType; import org.opencadc.permissions.WriteGrant; /** @@ -85,16 +87,10 @@ public class PostAction extends ArtifactAction { private static final Logger log = Logger.getLogger(PostAction.class); - /** - * Default, no-arg constructor. - */ public PostAction() { super(); } - /** - * Perform auth checks and initialize resources. - */ @Override public void initAction() throws Exception { super.initAction(); @@ -104,9 +100,6 @@ public void initAction() throws Exception { initStorageAdapter(); } - /** - * Update artifact metadata. - */ @Override public void doAction() throws Exception { @@ -116,6 +109,7 @@ public void doAction() throws Exception { log.debug("new uri: " + newURI); log.debug("new contentType: " + newContentType); log.debug("new contentEncoding: " + newContentEncoding); + newContentType = validateContentType(newContentType); String txnID = syncInput.getHeader(PUT_TXN_ID); String txnOP = syncInput.getHeader(PUT_TXN_OP); diff --git a/minoc/src/main/java/org/opencadc/minoc/PutAction.java b/minoc/src/main/java/org/opencadc/minoc/PutAction.java index 44c099dc..3dcf196d 100644 --- a/minoc/src/main/java/org/opencadc/minoc/PutAction.java +++ b/minoc/src/main/java/org/opencadc/minoc/PutAction.java @@ -72,6 +72,7 @@ import ca.nrc.cadc.io.ByteLimitExceededException; import ca.nrc.cadc.io.ReadException; import ca.nrc.cadc.io.WriteException; +import ca.nrc.cadc.net.ContentType; import ca.nrc.cadc.net.PreconditionFailedException; import ca.nrc.cadc.net.ResourceNotFoundException; import ca.nrc.cadc.net.TransientException; @@ -141,7 +142,8 @@ public void doAction() throws Exception { log.debug("Content-Length: " + lengthHeader); log.debug("Content-Encoding: " + encodingHeader); log.debug("Content-Type: " + typeHeader); - + final String contentType = validateContentType(typeHeader); + Long contentLength = null; if (lengthHeader != null) { try { @@ -258,7 +260,7 @@ public void doAction() throws Exception { artifactURI, artifactMetadata.getContentChecksum(), artifactMetadata.getContentLastModified(), artifactMetadata.getContentLength()); artifact.contentEncoding = encodingHeader; - artifact.contentType = typeHeader; + artifact.contentType = contentType; artifact.storageLocation = artifactMetadata.getStorageLocation(); if (txnID != null) { From 661543c0e1f55595f8303b931ffe14aa104d9155 Mon Sep 17 00:00:00 2001 From: Patrick Dowler Date: Fri, 12 Apr 2024 11:21:52 -0700 Subject: [PATCH 3/4] minoc: checkstyle fix --- minoc/src/main/java/org/opencadc/minoc/PostAction.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/minoc/src/main/java/org/opencadc/minoc/PostAction.java b/minoc/src/main/java/org/opencadc/minoc/PostAction.java index e499c77b..f0d29b6a 100644 --- a/minoc/src/main/java/org/opencadc/minoc/PostAction.java +++ b/minoc/src/main/java/org/opencadc/minoc/PostAction.java @@ -68,14 +68,11 @@ package org.opencadc.minoc; import ca.nrc.cadc.db.TransactionManager; -import ca.nrc.cadc.net.ContentType; import ca.nrc.cadc.profiler.Profiler; import org.apache.log4j.Logger; import org.opencadc.inventory.Artifact; -import org.opencadc.inventory.db.EntityNotFoundException; import org.opencadc.inventory.storage.PutTransaction; import org.opencadc.inventory.storage.StorageMetadata; -import static org.opencadc.minoc.ArtifactAction.validateContentType; import org.opencadc.permissions.WriteGrant; /** From e21dd840d428101003a4f87c240948a854299e39 Mon Sep 17 00:00:00 2001 From: Patrick Dowler Date: Fri, 12 Apr 2024 12:28:58 -0700 Subject: [PATCH 4/4] remove cruft from tests --- minoc/src/intTest/java/org/opencadc/minoc/BasicOpsTest.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/minoc/src/intTest/java/org/opencadc/minoc/BasicOpsTest.java b/minoc/src/intTest/java/org/opencadc/minoc/BasicOpsTest.java index f5df1ff9..a6ef8b05 100644 --- a/minoc/src/intTest/java/org/opencadc/minoc/BasicOpsTest.java +++ b/minoc/src/intTest/java/org/opencadc/minoc/BasicOpsTest.java @@ -164,7 +164,6 @@ public void testPutGetUpdateHeadDelete() { params.put("contentEncoding", newEncoding); params.put("contentType", newType); HttpPost post = new HttpPost(artifactURL, params, false); - post.setDigest(computeChecksumURI(data)); Subject.doAs(userSubject, new RunnableAction(post)); log.info("post: " + post.getResponseCode() + " " + post.getThrowable()); Assert.assertNull(post.getThrowable()); @@ -177,7 +176,6 @@ public void testPutGetUpdateHeadDelete() { params.clear(); params.put("contentType", invalidType); post = new HttpPost(artifactURL, params, false); - post.setDigest(computeChecksumURI(data)); Subject.doAs(userSubject, new RunnableAction(post)); log.info("post: " + post.getResponseCode() + " " + post.getThrowable()); Assert.assertNotNull(post.getThrowable());