Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

minoc: filename override mechanism #551

Merged
merged 9 commits into from
Jan 26, 2024

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@
import java.net.URI;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.Random;
Expand All @@ -99,10 +100,16 @@ public void testPrioritizePullFromSites() throws Exception {
for (int i = 0; i < 10; i++) {
sites.add(new StorageSite(URI.create("ivo://site" + i), "site1" + i, true, rd.nextBoolean()));
}
ProtocolsGenerator.prioritizePullFromSites(sites);
for (StorageSite s : sites) {
log.info("found: " + s.getID() + " aka " + s.getResourceID());
}
List<StorageSite> result1 = ProtocolsGenerator.prioritizePullFromSites(sites);
Assert.assertEquals(sites.size(), result1.size());
Assert.assertTrue(result1.containsAll(sites));

List<StorageSite> result2 = ProtocolsGenerator.prioritizePullFromSites(sites);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this testing?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is just testing that pull from prioritization is random by those two sets not being the same order

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. Does it mean that it is possible to give a false positive when random randomly returns the same order?

Assert.assertEquals(sites.size(), result2.size());
Assert.assertTrue(result2.containsAll(sites));

// test random order
Assert.assertNotEquals(result1, result2);
}

@Test
Expand Down
112 changes: 112 additions & 0 deletions minoc/src/intTest/java/org/opencadc/minoc/BasicOpsTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -146,10 +146,12 @@ public void testPutGetUpdateHeadDelete() {
long contentLength = get.getContentLength();
String contentType = get.getContentType();
String contentEncoding = get.getContentEncoding();
String contentDisposition = get.getResponseHeader("content-disposition");
Assert.assertEquals(computeChecksumURI(data), checksumURI);
Assert.assertEquals(data.length, contentLength);
Assert.assertEquals(type, contentType);
Assert.assertEquals(encoding, contentEncoding);
Assert.assertTrue(contentDisposition.contains("filename=") && contentDisposition.contains("file.txt"));
Date lastModified = get.getLastModified();
Assert.assertNotNull(lastModified);

Expand Down Expand Up @@ -181,10 +183,12 @@ public void testPutGetUpdateHeadDelete() {
contentLength = head.getContentLength();
contentType = head.getContentType();
contentEncoding = head.getContentEncoding();
contentDisposition = head.getResponseHeader("content-disposition");
Assert.assertEquals(computeChecksumURI(data), checksumURI);
Assert.assertEquals(data.length, contentLength);
Assert.assertEquals(newType, contentType);
Assert.assertEquals(newEncoding, contentEncoding);
Assert.assertTrue(contentDisposition.contains("filename=") && contentDisposition.contains("file.txt"));
lastModified = head.getLastModified();
Assert.assertNotNull(lastModified);

Expand Down Expand Up @@ -331,6 +335,114 @@ public void testGetRanges() {
}
}

@Test
public void testFilenameOverride() {
try {
URI artifactURI = URI.create("cadc:TEST/testFilenameOverride.txt");
URL artifactURL = new URL(filesURL + "/" + artifactURI.toString());

String content = "abcdefghijklmnopqrstuvwxyz";
String encoding = "test-encoding";
String type = "text/plain";
byte[] data = content.getBytes();
URI expectedChecksum = computeChecksumURI(data);

// put: no length or checksum
InputStream in = new ByteArrayInputStream(data);
HttpUpload put = new HttpUpload(in, artifactURL);
put.setRequestProperty(HttpTransfer.CONTENT_TYPE, type);
put.setRequestProperty(HttpTransfer.CONTENT_ENCODING, encoding);
put.setDigest(expectedChecksum);

Subject.doAs(userSubject, new RunnableAction(put));
log.info("put: " + put.getResponseCode() + " " + put.getThrowable());
log.info("headers: " + put.getResponseHeader("content-length") + " " + put.getResponseHeader("digest"));
Assert.assertNull(put.getThrowable());
Assert.assertEquals("Created", 201, put.getResponseCode());

// head
ByteArrayOutputStream bos = new ByteArrayOutputStream();
HttpGet head = new HttpGet(artifactURL, bos);
head.setHeadOnly(true);
log.info("head: " + artifactURL.toExternalForm());
Subject.doAs(userSubject, new RunnableAction(head));
log.info("head: " + head.getResponseCode() + " " + head.getThrowable());
log.info("headers: " + head.getResponseHeader("content-length") + " " + head.getResponseHeader("digest"));
log.warn("head output: " + bos.toString());
Assert.assertNull(head.getThrowable());
URI checksumURI = head.getDigest();
long contentLength = head.getContentLength();
String contentType = head.getContentType();
String contentEncoding = head.getContentEncoding();
String contentDisposition = head.getResponseHeader("content-disposition");
Assert.assertEquals(computeChecksumURI(data), checksumURI);
Assert.assertEquals(data.length, contentLength);
Assert.assertEquals(type, contentType);
Assert.assertEquals(encoding, contentEncoding);
log.info("content-disposition: " + contentDisposition);
Assert.assertTrue(contentDisposition.contains("filename=") && contentDisposition.contains("testFilenameOverride.txt"));
Date lastModified = head.getLastModified();
Assert.assertNotNull(lastModified);

URL foURL = new URL(artifactURL.toExternalForm() + ":fo/alternate.txt");
head = new HttpGet(foURL, bos);
head.setHeadOnly(true);
log.info("head: " + foURL.toExternalForm());
Subject.doAs(userSubject, new RunnableAction(head));
log.info("head: " + head.getResponseCode() + " " + head.getThrowable());
log.info("headers: " + head.getResponseHeader("content-length") + " " + head.getResponseHeader("digest"));
log.warn("head output: " + bos.toString());
Assert.assertNull(head.getThrowable());
checksumURI = head.getDigest();
contentLength = head.getContentLength();
contentType = head.getContentType();
contentEncoding = head.getContentEncoding();
contentDisposition = head.getResponseHeader("content-disposition");
Assert.assertEquals(computeChecksumURI(data), checksumURI);
Assert.assertEquals(data.length, contentLength);
Assert.assertEquals(type, contentType);
Assert.assertEquals(encoding, contentEncoding);
log.info("content-disposition: " + contentDisposition);
Assert.assertTrue(contentDisposition.contains("filename=") && contentDisposition.contains("alternate.txt"));
Date lastModified2 = head.getLastModified();
Assert.assertEquals(lastModified, lastModified2);

// get
bos = new ByteArrayOutputStream();
log.info("get: " + foURL.toExternalForm());
HttpGet get = new HttpGet(foURL, bos);
Subject.doAs(userSubject, new RunnableAction(get));
log.info("get: " + get.getResponseCode() + " " + get.getThrowable());
log.info("headers: " + get.getResponseHeader("content-length") + " " + get.getResponseHeader("digest"));
log.warn("get output: " + bos.toString());
Assert.assertNull(get.getThrowable());
checksumURI = get.getDigest();
contentLength = get.getContentLength();
contentType = get.getContentType();
contentEncoding = get.getContentEncoding();
contentDisposition = get.getResponseHeader("content-disposition");
Assert.assertEquals(computeChecksumURI(data), checksumURI);
Assert.assertEquals(data.length, contentLength);
Assert.assertEquals(type, contentType);
Assert.assertEquals(encoding, contentEncoding);
log.info("content-disposition: " + contentDisposition);
Assert.assertTrue(contentDisposition.contains("filename=") && contentDisposition.contains("alternate.txt"));
Date lastModified3 = get.getLastModified();
Assert.assertEquals(lastModified, lastModified3);

// delete
HttpDelete delete = new HttpDelete(artifactURL, false);
Subject.doAs(userSubject, new RunnableAction(delete));
log.info("delete: " + delete.getResponseCode() + " " + delete.getThrowable());
Assert.assertNull(delete.getThrowable());
Assert.assertEquals("no content", 204, delete.getResponseCode());

} catch (Exception t) {
log.error("unexpected throwable", t);
Assert.fail("unexpected throwable: " + t);
}
}

@Test
public void testGetNotFound() {
try {
Expand Down
62 changes: 33 additions & 29 deletions minoc/src/main/java/org/opencadc/minoc/ArtifactAction.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
******************* CANADIAN ASTRONOMY DATA CENTRE *******************
************** CENTRE CANADIEN DE DONNÉES ASTRONOMIQUES **************
*
* (c) 2023. (c) 2023.
* (c) 2024. (c) 2024.
* Government of Canada Gouvernement du Canada
* National Research Council Conseil national de recherches
* Ottawa, Canada, K1A 0R6 Ottawa, Canada, K1A 0R6
Expand Down Expand Up @@ -116,6 +116,11 @@ public abstract class ArtifactAction extends RestAction {

// The target artifact
URI artifactURI;
String errMsg;

// alternmate filename for content-disposition header, usually null
boolean extractFilenameOverride = false;
String filenameOverride;

// The (possibly null) authentication token.
String authToken;
Expand Down Expand Up @@ -260,6 +265,10 @@ protected void initAndAuthorize(Class<? extends Grant> grantClass, boolean allow

void init() {
if (this.artifactURI == null) {
if (errMsg != null) {
throw new IllegalArgumentException(errMsg);
}
// generic
throw new IllegalArgumentException("missing or invalid artifact URI");
}
}
Expand All @@ -285,16 +294,29 @@ void parsePath() {
String path = this.syncInput.getPath();
log.debug("path: " + path);
if (path != null) {
int colonIndex = path.indexOf(":");
int firstSlashIndex = path.indexOf("/");
if (colonIndex != -1) {
if (firstSlashIndex < 0 || firstSlashIndex > colonIndex) {
// no auth token--artifact URI is complete path
this.artifactURI = createArtifactURI(path);
} else {
this.artifactURI = createArtifactURI(path.substring(firstSlashIndex + 1));
this.authToken = path.substring(0, firstSlashIndex);
log.debug("authToken: " + this.authToken);
int colon1 = path.indexOf(":");
int slash1 = path.indexOf("/");
if (colon1 != -1) {
if (slash1 >= 0 && slash1 < colon1) {
// auth token in front
this.authToken = path.substring(0, slash1);
path = path.substring(slash1 + 1);
}
try {
int foi = path.indexOf(":fo/");
if (foi > 0 && extractFilenameOverride) {
// filename override appended
this.filenameOverride = path.substring(foi + 4);
path = path.substring(0, foi);
} else if (foi > 0) {
throw new IllegalArgumentException("detected misuse of :fo/ filename override");
}
URI auri = new URI(path);
InventoryUtil.validateArtifactURI(ArtifactAction.class, auri);
this.artifactURI = auri;
} catch (URISyntaxException | IllegalArgumentException e) {
this.errMsg = "illegal artifact URI: " + path + " reason: " + e.getMessage();
log.debug(errMsg, e);
}
}
}
Expand All @@ -307,22 +329,4 @@ Artifact getArtifact(URI artifactURI) throws ResourceNotFoundException {
}
return artifact;
}

/**
* Create a valid artifact uri.
* @param uri The input string.
* @return The artifact uri object.
*/
private URI createArtifactURI(String uri) {
log.debug("artifact URI: " + uri);
URI ret;
try {
ret = new URI(uri);
InventoryUtil.validateArtifactURI(ArtifactAction.class, ret);
} catch (URISyntaxException | IllegalArgumentException e) {
ret = null;
log.debug("illegal artifact URI: " + uri, e);
}
return ret;
}
}
6 changes: 4 additions & 2 deletions minoc/src/main/java/org/opencadc/minoc/GetAction.java
Original file line number Diff line number Diff line change
Expand Up @@ -124,13 +124,15 @@ public class GetAction extends ArtifactAction {
// constructor for unit tests with no config/init
GetAction(boolean init) {
super(init);
this.extractFilenameOverride = true;
}

/**
* Default, no-arg constructor.
*/
public GetAction() {
super();
this.extractFilenameOverride = true;
}

/**
Expand Down Expand Up @@ -194,7 +196,7 @@ public void doAction() throws Exception {
}

// default: complete download
HeadAction.setHeaders(artifact, syncOutput);
HeadAction.setHeaders(artifact, filenameOverride, syncOutput);
bcos = new ByteCountOutputStream(syncOutput.getOutputStream());

// create tmp StorageLocation with expected checksum so adapter can potentially
Expand Down Expand Up @@ -238,7 +240,7 @@ public void doAction() throws Exception {
private ByteCountOutputStream doByteRangeRequest(Artifact artifact, ByteRange byteRange)
throws InterruptedException, IOException, ResourceNotFoundException,
ReadException, WriteException, StorageEngageException, TransientException {
HeadAction.setHeaders(artifact, syncOutput);
HeadAction.setHeaders(artifact, filenameOverride, syncOutput);
syncOutput.setCode(206);
long lastByte = byteRange.getOffset() + byteRange.getLength() - 1;
syncOutput.setHeader(CONTENT_RANGE, "bytes " + byteRange.getOffset() + "-"
Expand Down
10 changes: 7 additions & 3 deletions minoc/src/main/java/org/opencadc/minoc/HeadAction.java
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ public class HeadAction extends ArtifactAction {
*/
public HeadAction() {
super();
this.extractFilenameOverride = true;
}

/**
Expand Down Expand Up @@ -131,7 +132,7 @@ public void doAction() throws Exception {
artifact = getArtifact(artifactURI);
}
if (artifact != null) {
setHeaders(artifact, syncOutput);
setHeaders(artifact, filenameOverride, syncOutput);
}
}

Expand All @@ -140,7 +141,7 @@ public void doAction() throws Exception {
* @param artifact The artifact with metadata
* @param syncOutput The target response
*/
static void setHeaders(Artifact artifact, SyncOutput syncOutput) {
static void setHeaders(Artifact artifact, String filenameOverride, SyncOutput syncOutput) {
syncOutput.setHeader(ARTIFACT_ID_HDR, artifact.getID().toString());
syncOutput.setDigest(artifact.getContentChecksum());
syncOutput.setLastModified(artifact.getContentLastModified());
Expand All @@ -149,7 +150,10 @@ static void setHeaders(Artifact artifact, SyncOutput syncOutput) {
DateFormat df = DateUtil.getDateFormat(DateUtil.HTTP_DATE_FORMAT, DateUtil.GMT);
syncOutput.setHeader("Last-Modified", df.format(artifact.getContentLastModified()));

String filename = InventoryUtil.computeArtifactFilename(artifact.getURI());
String filename = filenameOverride;
if (filename == null) {
filename = InventoryUtil.computeArtifactFilename(artifact.getURI());
}
syncOutput.setHeader("Content-Disposition", "attachment; filename=\"" + filename + "\"");

if (artifact.contentEncoding != null) {
Expand Down
8 changes: 6 additions & 2 deletions minoc/src/main/java/org/opencadc/minoc/MinocInitAction.java
Original file line number Diff line number Diff line change
Expand Up @@ -196,9 +196,13 @@ private void initStorageSite() {
if (name.charAt(0) == '/') {
name = name.substring(1);
}

// possibly temporary hack: advertise readable and writable if this service
// is configured to accept preauth tokens
boolean trustPreauth = !config.getTrustedServices().isEmpty();

boolean allowRead = !config.getReadGrantServices().isEmpty();
boolean allowWrite = !config.getWriteGrantServices().isEmpty();
boolean allowRead = trustPreauth || !config.getReadGrantServices().isEmpty();
boolean allowWrite = trustPreauth || !config.getWriteGrantServices().isEmpty();

StorageSite self = null;
if (curlist.isEmpty()) {
Expand Down
2 changes: 1 addition & 1 deletion minoc/src/main/java/org/opencadc/minoc/PostAction.java
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ public void doAction() throws Exception {
log.debug("commit txn: OK");

syncOutput.setCode(202); // Accepted
HeadAction.setHeaders(existing, syncOutput);
HeadAction.setHeaders(existing, null, syncOutput);
syncOutput.setHeader("content-length", 0);
} catch (Exception e) {
log.error("failed to persist " + artifactURI, e);
Expand Down
Loading
Loading