opencadc · pdowler · Apr 12, 2024 · Apr 12, 2024 · Apr 12, 2024 · Apr 12, 2024
diff --git 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
@@ -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();
@@ -163,12 +164,22 @@ 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());
-            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);
+            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
@@ -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);
@@ -134,7 +128,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 +143,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 {
+            putContentType = null;
+            uploadAndCompareCutout(noclArtifactURI, SodaParamValidator.SUB, cutoutSpecs, testFilePrefix);
+        } finally {
+            putContentType = "application/fits";
+        }
+
+        Subject.doAs(userSubject, (PrivilegedExceptionAction<Object>) () -> {
+            HttpGet head = new HttpGet(noclArtifactURL, false);
+            head.setHeadOnly(true);
+            head.prepare();
+            Assert.assertNull("no content type", head.getResponseHeader("content-type"));
+            return null;
+        });
     }
 
     @Test
@@ -204,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<Object>) () -> {
             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<Object>) () -> {
+            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
@@ -490,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
@@ -108,6 +108,60 @@ public IncorrectPutMetadataTest() {
         super();
     }
 
+    @Test
+    public void testUploadInvalidContentType() {
+        try {
+
+            Subject.doAs(userSubject, new PrivilegedExceptionAction<Object>() {
+                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
@@ -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
@@ -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> T assertSingleWCS(final String key, final List<T> 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
@@ -71,7 +71,6 @@
 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 org.opencadc.permissions.WriteGrant;
@@ -85,16 +84,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 +97,6 @@ public void initAction() throws Exception {
         initStorageAdapter();
     }
 
-    /**
-     * Update artifact metadata.
-     */
     @Override
     public void doAction() throws Exception {
 
@@ -116,6 +106,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
@@ -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) {