diff --git a/src/firefly/java/edu/caltech/ipac/firefly/server/util/QueryUtil.java b/src/firefly/java/edu/caltech/ipac/firefly/server/util/QueryUtil.java index 5bd130b6d9..4685649974 100644 --- a/src/firefly/java/edu/caltech/ipac/firefly/server/util/QueryUtil.java +++ b/src/firefly/java/edu/caltech/ipac/firefly/server/util/QueryUtil.java @@ -240,7 +240,8 @@ public static File resolveFileFromSource(String source, boolean checkForUpdates, public static File tmpFileForUrl(String urlStr, String prefix, File dir) { URL url = makeUrl(urlStr); String pExt = url == null ? null : FileUtil.getExtension(url.getPath().replaceFirst("^.*/", ""), true); - String ext = isEmpty(pExt) ? ".ul" : "." + pExt; + String pQuery = url == null ? null : url.getQuery(); + String ext = isEmpty(pExt) || !isEmpty(pQuery) ? ".ul" : "." + pExt; return Util.Try.it(() -> File.createTempFile(prefix, ext, dir)).get(); } diff --git a/src/firefly/java/edu/caltech/ipac/table/io/SpectrumMetaInspector.java b/src/firefly/java/edu/caltech/ipac/table/io/SpectrumMetaInspector.java index 7a8ddd8f45..aff4430346 100644 --- a/src/firefly/java/edu/caltech/ipac/table/io/SpectrumMetaInspector.java +++ b/src/firefly/java/edu/caltech/ipac/table/io/SpectrumMetaInspector.java @@ -179,6 +179,8 @@ else if (!spectrumHint) { // determine if we are to continue or just return // - guessing columns or checking utype // - checking utype only + fixAnyBadDataGroup(dg); + List dtAry= Arrays.asList(dg.getDataDefinitions()); List groupInfosList= new ArrayList<>(); var nonStandardSpecType= dg.getAttribute("SPEC_TYPE","").toLowerCase(); @@ -486,5 +488,24 @@ public static void insertSpectralData(DataGroup dg, InsertEntry wl, InsertEntry meta.addKeyword(TableMeta.UTYPE,SPEC_SPECTRUM); } + static void fixAnyBadDataGroup(DataGroup dg) { + var keyWords= dg.getTableMeta().getKeywords(); + var isHopsSpec= keyWords.stream().filter(s -> (s.getKey()==null && s.getValue().startsWith("HOPS"))).count()==1 && + keyWords.stream().filter(s -> (s.getKey()==null && s.getValue().startsWith("Spitzer"))).count()==1; + if (isHopsSpec) { // try to fix it + var dtAry= dg.getDataDefinitions(); + for (DataType dt : dtAry){ + if (dt.getDataType()==String.class) { + dt.setDataType(Double.class); + dt.setUnits(dt.getTypeDesc()); + dt.setTypeDesc("double"); + } + } + } + + // add others here + + } + public record InsertEntry(String column, String unit) {} } \ No newline at end of file diff --git a/src/firefly/java/edu/caltech/ipac/util/FormatUtil.java b/src/firefly/java/edu/caltech/ipac/util/FormatUtil.java index 43d718f926..98ffad5bb2 100644 --- a/src/firefly/java/edu/caltech/ipac/util/FormatUtil.java +++ b/src/firefly/java/edu/caltech/ipac/util/FormatUtil.java @@ -9,19 +9,19 @@ import edu.caltech.ipac.firefly.server.db.DuckDbReadable; import edu.caltech.ipac.firefly.server.util.Logger; -import nom.tam.fits.Fits; -import nom.tam.fits.FitsException; +import org.apache.commons.compress.compressors.bzip2.BZip2CompressorInputStream; import javax.annotation.Nonnull; -import java.nio.file.Files; -import java.nio.file.Path; -import java.util.EnumSet; -import java.util.Set; import java.io.BufferedReader; import java.io.CharArrayReader; import java.io.File; +import java.io.FileInputStream; import java.io.FileReader; import java.io.IOException; +import java.io.InputStream; +import java.util.EnumSet; +import java.util.Set; +import java.util.zip.GZIPInputStream; import static edu.caltech.ipac.firefly.core.Util.Opt.ifNotNull; import static edu.caltech.ipac.table.IpacTableUtil.isIpacTable; @@ -36,6 +36,7 @@ public class FormatUtil { private static final int SAMPLE_SIZE = (int) (8 * FileUtil.K); private static final Logger.LoggerImpl LOGGER = Logger.getLogger(); + private static final int[] FITS_MAGIC= {'S','I','M','P','L','E',' ',' ','='}; public record MimeDesc(String mime, String desc) {} @@ -114,7 +115,7 @@ public static Format fromMime(String mime) { /** * Detects the MIME type of file using magic-byte inspection of the file header, - * falling back to OS-level and extension-based guessing for unrecognized types. + * falling back to extension-based guessing for unrecognized types. * This is platform-independent for all binary formats handled here; text-based * formats (FITS text headers, VOTable, IPAC table, etc.) are handled downstream * by {@link FormatUtil#detect}. @@ -159,17 +160,21 @@ public static MimeDesc getMimeType(String inFile) { if (magic(hdr, 257, 'u','s','t','a','r')) return new MimeDesc(TAR.mime(), "POSIX tar archive"); // Binary FITS: header block starts with "SIMPLE =" - if (magic(hdr, 0, 'S','I','M','P','L','E',' ',' ','=')) + if (magic(hdr, 0, FITS_MAGIC)) return new MimeDesc(FITS.mime(), "FITS image data"); + var hdrStr= new String(hdr); + if ((hdrStr.startsWith(" { loc= url.getHost(); path= url.getFile(); + query= url.getQuery(); outLoc = FileUtil.makeShortHostName(loc); prefix= "URL-"; } @@ -134,7 +137,7 @@ public static String makeUniqueFileName(UriRef uri, String userLoginName, String retval = prefix + outLoc + "-" + originalHashCode + baseKey; //note: "=","," signs causes problem in download servlet. retval = retval.replaceAll("[ :\\[\\]/\\\\|*?<>=,]", "-"); - String ext= FileUtil.getExtension(fileStr,true); + String ext= isEmpty(query) ? FileUtil.getExtension(fileStr,true) : ""; var fileExt= EXT_LIST.contains(ext) ? ext : EXT_LIST.getFirst(); return retval + "." + fileExt; } diff --git a/src/firefly/java/edu/caltech/ipac/util/download/URLDownload.java b/src/firefly/java/edu/caltech/ipac/util/download/URLDownload.java index b1f73b03d3..6fb8d5207d 100644 --- a/src/firefly/java/edu/caltech/ipac/util/download/URLDownload.java +++ b/src/firefly/java/edu/caltech/ipac/util/download/URLDownload.java @@ -134,17 +134,12 @@ public static Map> getQueryParams(URL url) { var paramList= url.getQuery().split("&"); var queryParamMap = new HashMap>(); for(var p : paramList) { - var parts = p.split("="); - if (parts.length == 2) { - var key= parts[0].trim(); - var value= parts[1].trim(); - var valueList= queryParamMap.get(key); - if (valueList == null) { - valueList = new ArrayList<>(); - queryParamMap.put(key, valueList); - } - valueList.add(value); - } + var parts = p.split("=", 2); // split on the first "=" only, so values can contain "=" (e.g. base64 padding) + var key= parts[0].trim(); + if (key.isEmpty()) continue; + var value= parts.length == 2 ? parts[1].trim() : "true"; // a parameter with no value, e.g. ?flag + var valueList = queryParamMap.computeIfAbsent(key, k -> new ArrayList<>()); + valueList.add(value); } return queryParamMap; } @@ -161,7 +156,7 @@ public static String firstParamValUsingKeyList(Map> params, return foundKey!=null ? getFirstVal(params, foundKey) : null; } - private static int codeFromException(Exception e) { + static int codeFromException(Exception e) { // package-private for testing return switch (e) { case SSLException ignored -> 495; case SocketTimeoutException ignored -> HttpURLConnection.HTTP_CLIENT_TIMEOUT; diff --git a/src/firefly/test/edu/caltech/ipac/firefly/server/util/QueryUtilTest.java b/src/firefly/test/edu/caltech/ipac/firefly/server/util/QueryUtilTest.java new file mode 100644 index 0000000000..0455a11eb3 --- /dev/null +++ b/src/firefly/test/edu/caltech/ipac/firefly/server/util/QueryUtilTest.java @@ -0,0 +1,36 @@ +/* + * License information at https://github.com/Caltech-IPAC/firefly/blob/master/License.txt + */ + +package edu.caltech.ipac.firefly.server.util; + +import edu.caltech.ipac.firefly.ConfigTest; +import org.junit.Test; + +import java.io.File; +import java.io.IOException; +import java.nio.file.Files; + +import static org.junit.Assert.assertTrue; + +public class QueryUtilTest extends ConfigTest { + + @Test + public void tmpFileForUrl() throws IOException { + File dir = Files.createTempDirectory("queryUtilTest").toFile(); + dir.deleteOnExit(); + + assertTmpExt(dir, "http://h/a/x.fits.gz", ".fits.gz"); + assertTmpExt(dir, "http://h/a/x.csv", ".csv"); + // a query string makes the path extension unreliable + assertTmpExt(dir, "http://h/a/x.fits.gz?token=1", ".ul"); + assertTmpExt(dir, "http://h/cgi/get.py?file=a.csv", ".ul"); + assertTmpExt(dir, "http://h/a/noext", ".ul"); + } + + private static void assertTmpExt(File dir, String url, String ext) { + File f = QueryUtil.tmpFileForUrl(url, "req_", dir); + f.deleteOnExit(); + assertTrue(url + " -> " + f.getName(), f.getName().endsWith(ext)); + } +} diff --git a/src/firefly/test/edu/caltech/ipac/util/FormatUtilTest.java b/src/firefly/test/edu/caltech/ipac/util/FormatUtilTest.java index 4e995b80ce..2450f872ac 100644 --- a/src/firefly/test/edu/caltech/ipac/util/FormatUtilTest.java +++ b/src/firefly/test/edu/caltech/ipac/util/FormatUtilTest.java @@ -13,10 +13,25 @@ import java.io.File; import java.io.IOException; +import java.io.OutputStream; import java.nio.file.Files; import java.nio.file.Path; - -import static edu.caltech.ipac.util.FormatUtil.Format.*; +import java.nio.file.StandardCopyOption; +import java.util.zip.GZIPOutputStream; + +import static edu.caltech.ipac.util.FormatUtil.Format.CSV; +import static edu.caltech.ipac.util.FormatUtil.Format.FITS; +import static edu.caltech.ipac.util.FormatUtil.Format.GZIP; +import static edu.caltech.ipac.util.FormatUtil.Format.HTML; +import static edu.caltech.ipac.util.FormatUtil.Format.IPACTABLE; +import static edu.caltech.ipac.util.FormatUtil.Format.JSON; +import static edu.caltech.ipac.util.FormatUtil.Format.PARQUET; +import static edu.caltech.ipac.util.FormatUtil.Format.PNG; +import static edu.caltech.ipac.util.FormatUtil.Format.REGION; +import static edu.caltech.ipac.util.FormatUtil.Format.TEXT; +import static edu.caltech.ipac.util.FormatUtil.Format.TSV; +import static edu.caltech.ipac.util.FormatUtil.Format.UNKNOWN; +import static edu.caltech.ipac.util.FormatUtil.Format.VO_TABLE; import static org.junit.Assert.assertEquals; /** @@ -204,12 +219,12 @@ public void badMixedDelimiters() throws IOException { } @Test - public void mimeType() { + public void mimeType() throws IOException { File tfile = FileLoader.resolveFile("FileUpload-samples/VOTable/binary/binary_gaia.xml"); - assertEquals(tfile.getName(), "application/xml", FormatUtil.getMimeType(tfile).mime()); + assertEquals(tfile.getName(), VO_TABLE.mime(), FormatUtil.getMimeType(tfile).mime()); tfile = FileLoader.resolveFile("FileUpload-samples/VOTable/fits/starlinkVOfits.xml"); - assertEquals(tfile.getName(), "application/xml", FormatUtil.getMimeType(tfile).mime()); + assertEquals(tfile.getName(), VO_TABLE.mime(), FormatUtil.getMimeType(tfile).mime()); tfile = FileLoader.resolveFile("edu/caltech/ipac/firefly/server/query/ptf-lc.tbl"); assertEquals(tfile.getName(), UNKNOWN.mime(), FormatUtil.getMimeType(tfile).mime()); @@ -217,5 +232,44 @@ public void mimeType() { tfile = FileLoader.resolveFile("stripe82-testfits/calexp-i-0-366,0.fits.gz"); String mtype = FormatUtil.getMimeType(tfile).mime(); assertEquals(tfile.getName(), GZIP.mime(), mtype); + assertEquals(tfile.getName(), FITS, FormatUtil.detect(tfile)); + + // gzipped FITS must be detected by content, not by a "fit" in the file name (e.g. temp files for urls with a query string) + Path tmp = Files.createTempFile("req_", ".ul"); + tmp.toFile().deleteOnExit(); + Files.copy(tfile.toPath(), tmp, StandardCopyOption.REPLACE_EXISTING); + assertEquals("gzipped fits named .ul", FITS, FormatUtil.detect(tmp.toFile())); + } + + @Test + public void extensionDoesNotDecideFormat() throws IOException { + // a .fits name must not decide the format; on linux /etc/mime.types maps .fits to image/fits + Path tmp = Files.createTempFile("test", ".fits"); + tmp.toFile().deleteOnExit(); + Files.writeString(tmp, "a,b,c\n1,2,3\n4,5,6\n"); + assertEquals("csv named .fits", CSV, FormatUtil.detect(tmp.toFile())); + } + + @Test + public void voTableWithWrongExtension() throws IOException { + File tfile = FileLoader.resolveFile("FileUpload-samples/VOTable/binary/binary_gaia.xml"); + for (String ext : new String[] {".csv", ".json"}) { + Path tmp = Files.createTempFile("test", ext); + tmp.toFile().deleteOnExit(); + Files.copy(tfile.toPath(), tmp, StandardCopyOption.REPLACE_EXISTING); + assertEquals("votable named " + ext, VO_TABLE.mime(), FormatUtil.getMimeType(tmp.toFile()).mime()); + assertEquals("votable named " + ext, VO_TABLE, FormatUtil.detect(tmp.toFile())); + } + } + + @Test + public void gzipNotFitsNamedFits() throws IOException { + // the name says fits but the decompressed content does not + Path tmp = Files.createTempFile("test", ".fits.gz"); + tmp.toFile().deleteOnExit(); + try (OutputStream out = new GZIPOutputStream(Files.newOutputStream(tmp))) { + out.write("a,b,c\n1,2,3\n".getBytes()); + } + assertEquals("gzipped csv named .fits.gz", GZIP, FormatUtil.detect(tmp.toFile())); } } diff --git a/src/firefly/test/edu/caltech/ipac/util/download/RetrieveUtilTest.java b/src/firefly/test/edu/caltech/ipac/util/download/RetrieveUtilTest.java new file mode 100644 index 0000000000..e94aaa77bc --- /dev/null +++ b/src/firefly/test/edu/caltech/ipac/util/download/RetrieveUtilTest.java @@ -0,0 +1,100 @@ +/* + * License information at https://github.com/Caltech-IPAC/firefly/blob/master/License.txt + */ + +package edu.caltech.ipac.util.download; + +import org.junit.Test; + +import java.util.List; + +import static edu.caltech.ipac.util.download.UriRef.CloudEnvironment.AWS; +import static edu.caltech.ipac.util.download.UriRef.CloudEnvironment.GCP; +import static edu.caltech.ipac.util.download.UriRef.CloudEnvironment.ON_PREM; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertTrue; + +public class RetrieveUtilTest { + + private static final UriRef S3 = UriRef.make("s3://bucket/a/x.fits"); + private static final UriRef GCS = UriRef.make("https://storage.googleapis.com/bucket/a/x.fits"); + private static final UriRef URL = UriRef.make("https://abc.com/a/x.fits"); + private static final UriRef URL2 = UriRef.make("https://def.com/a/x.fits"); + + @Test + public void makeUniqueFileNameExtension() { + assertExt("https://abc.com/a/x.fits", ".fits"); + assertExt("https://abc.com/a/x.fits.gz", ".fits.gz"); + assertExt("https://abc.com/a/x.xyz", ".ul"); // not in the known extension list + assertExt("s3://bucket/a/x.fits", ".fits"); // s3 refs have no query + // a query string makes the path extension unreliable + assertExt("https://abc.com/a/x.fits?sig=abc", ".ul"); + assertExt("https://abc.com/cgi/get?file=a.fits", ".ul"); + } + + @Test + public void makeUniqueFileNameIsUnique() { + String a1 = name("https://abc.com/a/x.fits?a=1", "user1"); + assertEquals("same input, same name", a1, name("https://abc.com/a/x.fits?a=1", "user1")); + assertNotEquals("different query", a1, name("https://abc.com/a/x.fits?a=2", "user1")); + assertNotEquals("different user", a1, name("https://abc.com/a/x.fits?a=1", "user2")); + } + + @Test + public void makeUniqueFileNameChars() { + // "=" and "," cause problems in the download servlet + String n = name("https://abc.com/cgi/get?file=a.fits&pos=10,20", null); + assertFalse(n, n.matches(".*[=,?:/\\\\ ].*")); + } + + @Test + public void makeUniqueFileNameNull() { + assertEquals("no-network-resource.empty", RetrieveUtil.makeUniqueFileName(null, null, null)); + } + + @Test + public void optimalUriTrivial() { + assertNull("empty list", RetrieveUtil.getOptimalUri(List.of(), AWS)); + assertSame("single entry is used whatever the env", GCS, RetrieveUtil.getOptimalUri(List.of(GCS), AWS)); + assertSame("same type, first wins", URL, RetrieveUtil.getOptimalUri(List.of(URL, URL2), AWS)); + } + + @Test + public void optimalUriMatchesCloud() { + List all = List.of(URL, S3, GCS); + assertSame("aws prefers s3", S3, RetrieveUtil.getOptimalUri(all, AWS)); + assertSame("gcp prefers gcs", GCS, RetrieveUtil.getOptimalUri(all, GCP)); + assertSame("on prem prefers url", URL, RetrieveUtil.getOptimalUri(all, ON_PREM)); + assertSame("no env prefers url", URL, RetrieveUtil.getOptimalUri(all, null)); + } + + @Test + public void optimalUriFallsBackToUrl() { + // the env's cloud is not in the list, so use the url + assertSame("gcp, no gcs", URL, RetrieveUtil.getOptimalUri(List.of(S3, URL), GCP)); + assertSame("aws, no s3", URL, RetrieveUtil.getOptimalUri(List.of(GCS, URL), AWS)); + assertSame("first url of that type", URL, RetrieveUtil.getOptimalUri(List.of(S3, URL, URL2), ON_PREM)); + } + + @Test + public void optimalUriCloudsOnly() { + List clouds = List.of(GCS, S3); + assertSame("aws", S3, RetrieveUtil.getOptimalUri(clouds, AWS)); + assertSame("gcp", GCS, RetrieveUtil.getOptimalUri(clouds, GCP)); + // no match and no url: falls back to s3, then gcs, regardless of list order + assertSame("on prem guesses s3", S3, RetrieveUtil.getOptimalUri(clouds, ON_PREM)); + } + + private static String name(String uri, String user) { + return RetrieveUtil.makeUniqueFileName(UriRef.make(uri), user, null); + } + + private static void assertExt(String uri, String ext) { + String n = name(uri, null); + assertTrue(uri + " -> " + n, n.endsWith(ext)); + } +} diff --git a/src/firefly/test/edu/caltech/ipac/util/download/URLDownloadTest.java b/src/firefly/test/edu/caltech/ipac/util/download/URLDownloadTest.java index 706f65e99a..c162bdccf4 100644 --- a/src/firefly/test/edu/caltech/ipac/util/download/URLDownloadTest.java +++ b/src/firefly/test/edu/caltech/ipac/util/download/URLDownloadTest.java @@ -8,6 +8,20 @@ import org.junit.Assert; import org.junit.Test; +import javax.net.ssl.SSLException; +import java.io.IOException; +import java.net.HttpURLConnection; +import java.net.SocketTimeoutException; +import java.net.URI; +import java.net.URL; +import java.net.URLConnection; +import java.net.UnknownHostException; +import java.util.List; +import java.util.Map; + +import static edu.caltech.ipac.util.download.URLDownload.codeFromException; +import static edu.caltech.ipac.util.download.URLDownload.firstParamValUsingKeyList; +import static edu.caltech.ipac.util.download.URLDownload.getQueryParams; import static edu.caltech.ipac.util.download.URLDownload.getSuggestedFileName; import static edu.caltech.ipac.util.download.URLDownload.makeContentDisposition; @@ -45,4 +59,58 @@ public void roundTrip() { Assert.assertEquals(expected, getSuggestedFileName(makeContentDisposition(name))); } } + + @Test + public void queryParams() throws Exception { + Assert.assertTrue("null url", getQueryParams(null).isEmpty()); + Assert.assertTrue("no query", getQueryParams(url("https://abc.com/a/x.fits")).isEmpty()); + + var p = getQueryParams(url("https://abc.com/a?x=1&y=2&x=3&flag&&=5&eq=a=b&sig=abc==&empty=")); + Assert.assertEquals(List.of("1", "3"), p.get("x")); // repeated keys keep their order + Assert.assertEquals(List.of("2"), p.get("y")); + Assert.assertEquals("key with no value", List.of("true"), p.get("flag")); + Assert.assertEquals("value containing =", List.of("a=b"), p.get("eq")); + Assert.assertEquals("trailing = kept", List.of("abc=="), p.get("sig")); + Assert.assertEquals("explicit empty value", List.of(""), p.get("empty")); + Assert.assertEquals("empty segment and empty key are skipped", 6, p.size()); + } + + @Test + public void firstParamVal() { + Map> p = Map.of("b", List.of("b1", "b2"), "c", List.of("c1"), "empty", List.of()); + Assert.assertEquals("first key that has a value wins", "b1", firstParamValUsingKeyList(p, "a", "b", "c")); + Assert.assertEquals("key order decides, not map order", "c1", firstParamValUsingKeyList(p, "c", "b")); + Assert.assertEquals("empty value list is skipped", "c1", firstParamValUsingKeyList(p, "empty", "c")); + Assert.assertNull("no match", firstParamValUsingKeyList(p, "x", "y")); + Assert.assertNull("no keys", firstParamValUsingKeyList(p)); + Assert.assertNull("null params", firstParamValUsingKeyList(null, "b")); + } + + @Test + public void suggestedFileNameFromConnection() throws Exception { + Assert.assertNull("null connection", getSuggestedFileName((URLConnection) null)); + Assert.assertNull("no header", getSuggestedFileName(conn(null))); + Assert.assertEquals("table.csv", getSuggestedFileName(conn("attachment; filename=\"table.csv\""))); + } + + @Test + public void codeFromExceptionMapping() { + Assert.assertEquals(495, codeFromException(new SSLException("x"))); + Assert.assertEquals(HttpURLConnection.HTTP_CLIENT_TIMEOUT, codeFromException(new SocketTimeoutException())); + Assert.assertEquals(HttpURLConnection.HTTP_BAD_GATEWAY, codeFromException(new UnknownHostException())); + Assert.assertEquals(HttpURLConnection.HTTP_INTERNAL_ERROR, codeFromException(new IOException())); + Assert.assertEquals(HttpURLConnection.HTTP_INTERNAL_ERROR, codeFromException(new RuntimeException())); + } + + private static URL url(String s) throws Exception { return new URI(s).toURL(); } + + /** a connection that only answers the Content-Disposition header, no network */ + private static URLConnection conn(String disposition) throws Exception { + return new URLConnection(url("https://abc.com/a")) { + @Override public void connect() {} + @Override public String getHeaderField(String name) { + return "content-disposition".equalsIgnoreCase(name) ? disposition : null; + } + }; + } }