Skip to content

Commit

Permalink
Merge pull request #578 from pdowler/master
Browse files Browse the repository at this point in the history 
minoc: work around for bad Artifact.contentType and fits
  • Loading branch information
@pdowler
pdowler authored Apr 12, 2024
2 parents 51456da + e21dd84 commit caf3e87
Show file tree
Hide file tree
Showing 8 changed files with 161 additions and 41 deletions.
2 changes: 1 addition & 1 deletion minoc/VERSION
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
15 changes: 13 additions & 2 deletions minoc/src/intTest/java/org/opencadc/minoc/BasicOpsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down
72 changes: 51 additions & 21 deletions minoc/src/intTest/java/org/opencadc/minoc/FitsOperationsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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
Expand All @@ -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);
Expand All @@ -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();
}
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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());
Expand Down
54 changes: 54 additions & 0 deletions minoc/src/intTest/java/org/opencadc/minoc/IncorrectPutMetadataTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
15 changes: 15 additions & 0 deletions minoc/src/main/java/org/opencadc/minoc/ArtifactAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
}
}
27 changes: 22 additions & 5 deletions minoc/src/main/java/org/opencadc/minoc/GetAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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)) {
Expand Down
11 changes: 1 addition & 10 deletions minoc/src/main/java/org/opencadc/minoc/PostAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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();
Expand All @@ -104,9 +97,6 @@ public void initAction() throws Exception {
initStorageAdapter();
}

/**
* Update artifact metadata.
*/
@Override
public void doAction() throws Exception {

Expand All @@ -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);
Expand Down
6 changes: 4 additions & 2 deletions minoc/src/main/java/org/opencadc/minoc/PutAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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) {
Expand Down

0 comments on commit caf3e87

Please sign in to comment.