Skip to content

Commit bfd6f87

Browse files
robywwrobyww
authored
IRSA-8042: Merge PR #2041 from IRSA-8042-detection
IRSA-8042: Fixed: a votable with a .fits extension being recognized wrong on linux
2 parents 962e222 + 630c6b1 commit bfd6f87

9 files changed

Lines changed: 330 additions & 39 deletions

File tree

‎src/firefly/java/edu/caltech/ipac/firefly/server/util/QueryUtil.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -240,7 +240,8 @@ public static File resolveFileFromSource(String source, boolean checkForUpdates,
240240
public static File tmpFileForUrl(String urlStr, String prefix, File dir) {
241241
URL url = makeUrl(urlStr);
242242
String pExt = url == null ? null : FileUtil.getExtension(url.getPath().replaceFirst("^.*/", ""), true);
243-
String ext = isEmpty(pExt) ? ".ul" : "." + pExt;
243+
String pQuery = url == null ? null : url.getQuery();
244+
String ext = isEmpty(pExt) || !isEmpty(pQuery) ? ".ul" : "." + pExt;
244245
return Util.Try.it(() -> File.createTempFile(prefix, ext, dir)).get();
245246
}
246247

‎src/firefly/java/edu/caltech/ipac/table/io/SpectrumMetaInspector.java‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,8 @@ else if (!spectrumHint) { // determine if we are to continue or just return
179179
// - guessing columns or checking utype
180180
// - checking utype only
181181

182+
fixAnyBadDataGroup(dg);
183+
182184
List<DataType> dtAry= Arrays.asList(dg.getDataDefinitions());
183185
List<GroupInfo> groupInfosList= new ArrayList<>();
184186
var nonStandardSpecType= dg.getAttribute("SPEC_TYPE","").toLowerCase();
@@ -486,5 +488,24 @@ public static void insertSpectralData(DataGroup dg, InsertEntry wl, InsertEntry
486488
meta.addKeyword(TableMeta.UTYPE,SPEC_SPECTRUM);
487489
}
488490

491+
static void fixAnyBadDataGroup(DataGroup dg) {
492+
var keyWords= dg.getTableMeta().getKeywords();
493+
var isHopsSpec= keyWords.stream().filter(s -> (s.getKey()==null && s.getValue().startsWith("HOPS"))).count()==1 &&
494+
keyWords.stream().filter(s -> (s.getKey()==null && s.getValue().startsWith("Spitzer"))).count()==1;
495+
if (isHopsSpec) { // try to fix it
496+
var dtAry= dg.getDataDefinitions();
497+
for (DataType dt : dtAry){
498+
if (dt.getDataType()==String.class) {
499+
dt.setDataType(Double.class);
500+
dt.setUnits(dt.getTypeDesc());
501+
dt.setTypeDesc("double");
502+
}
503+
}
504+
}
505+
506+
// add others here
507+
508+
}
509+
489510
public record InsertEntry(String column, String unit) {}
490511
}

‎src/firefly/java/edu/caltech/ipac/util/FormatUtil.java‎

Lines changed: 33 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -9,19 +9,19 @@
99

1010
import edu.caltech.ipac.firefly.server.db.DuckDbReadable;
1111
import edu.caltech.ipac.firefly.server.util.Logger;
12-
import nom.tam.fits.Fits;
13-
import nom.tam.fits.FitsException;
12+
import org.apache.commons.compress.compressors.bzip2.BZip2CompressorInputStream;
1413

1514
import javax.annotation.Nonnull;
16-
import java.nio.file.Files;
17-
import java.nio.file.Path;
18-
import java.util.EnumSet;
19-
import java.util.Set;
2015
import java.io.BufferedReader;
2116
import java.io.CharArrayReader;
2217
import java.io.File;
18+
import java.io.FileInputStream;
2319
import java.io.FileReader;
2420
import java.io.IOException;
21+
import java.io.InputStream;
22+
import java.util.EnumSet;
23+
import java.util.Set;
24+
import java.util.zip.GZIPInputStream;
2525

2626
import static edu.caltech.ipac.firefly.core.Util.Opt.ifNotNull;
2727
import static edu.caltech.ipac.table.IpacTableUtil.isIpacTable;
@@ -36,6 +36,7 @@
3636
public class FormatUtil {
3737
private static final int SAMPLE_SIZE = (int) (8 * FileUtil.K);
3838
private static final Logger.LoggerImpl LOGGER = Logger.getLogger();
39+
private static final int[] FITS_MAGIC= {'S','I','M','P','L','E',' ',' ','='};
3940

4041
public record MimeDesc(String mime, String desc) {}
4142

@@ -114,7 +115,7 @@ public static Format fromMime(String mime) {
114115

115116
/**
116117
* Detects the MIME type of file using magic-byte inspection of the file header,
117-
* falling back to OS-level and extension-based guessing for unrecognized types.
118+
* falling back to extension-based guessing for unrecognized types.
118119
* This is platform-independent for all binary formats handled here; text-based
119120
* formats (FITS text headers, VOTable, IPAC table, etc.) are handled downstream
120121
* by {@link FormatUtil#detect}.
@@ -159,17 +160,21 @@ public static MimeDesc getMimeType(String inFile) {
159160
if (magic(hdr, 257, 'u','s','t','a','r'))
160161
return new MimeDesc(TAR.mime(), "POSIX tar archive");
161162
// Binary FITS: header block starts with "SIMPLE ="
162-
if (magic(hdr, 0, 'S','I','M','P','L','E',' ',' ','='))
163+
if (magic(hdr, 0, FITS_MAGIC))
163164
return new MimeDesc(FITS.mime(), "FITS image data");
164165

166+
var hdrStr= new String(hdr);
167+
if ((hdrStr.startsWith("<?xml") && hdrStr.contains("<VOTABLE")) || hdrStr.startsWith("<VOTABLE")) { // this is not a magic number but close and might fit in the 264 bytes
168+
return new MimeDesc(VO_TABLE.mime(), "VO Table");
169+
}
170+
165171
} catch (Exception ex) {
166172
Logger.getLogger().error(ex, "Failed to read header for mime detection: " + inFile);
167173
}
168174

169-
// Fall back to OS content probing and extension-based guess for text formats
175+
// Fall back to extension-based guess for text formats
170176
try {
171-
String mime = Files.probeContentType(Path.of(inFile));
172-
if (mime == null) mime = java.net.URLConnection.guessContentTypeFromName(inFile);
177+
var mime = java.net.URLConnection.guessContentTypeFromName(inFile);
173178
if (mime != null) return new MimeDesc(mime, mime);
174179
} catch (Exception ex) {
175180
Logger.getLogger().error(ex, "Failed to detect mime type for: " + inFile);
@@ -208,14 +213,8 @@ public static Format detect(File inFile) throws IOException {
208213
}
209214

210215

211-
if ((format == BZIP2 || format == GZIP) &&
212-
inFile.getName().toLowerCase().contains("fit")) {
213-
// special case and a very heavy operation: we can handle fits bz2 or gzip files but don't try unless we are pretty sure
214-
// in archives: legacy files are often named a.fits.gz or a.fits.bz2
215-
try (var ignored = new Fits(inFile)) {
216-
return FITS;
217-
}
218-
catch (FitsException ignore) {}
216+
if (isCompressedFits(inFile, format)) { // archives often serve a.fits.gz or a.fits.bz2; check the decompressed header
217+
return FITS;
219218
}
220219

221220
format = ifNotNull(guessBySamplingContent(inFile)).getOrElse(format);
@@ -307,7 +306,7 @@ private static Format guessBySamplingContent(File inf) throws IOException {
307306
//NAME-RESOLVER: xxx
308307
return FIXEDTARGETS;
309308
} else if (line.startsWith("<VOTABLE") ||
310-
(line.contains("<?xml") && line.contains("<VOTABLE "))) {
309+
(line.contains("<?xml") && line.contains("<VOTABLE"))) {
311310
return VO_TABLE;
312311
} else if (isUwsEl(line)) {
313312
return UWS;
@@ -325,6 +324,20 @@ private static boolean isUwsEl(String line) {
325324
return isUws && line.matches("<(.+:)?job .*");
326325
}
327326

327+
/**
328+
* Check the first bytes of a gzip or bzip2 file, after decompressing, for the FITS "SIMPLE =" keyword
329+
*/
330+
private static boolean isCompressedFits(File inFile, Format format) {
331+
if (format != BZIP2 && format != GZIP) return false;
332+
try (var fis = new FileInputStream(inFile);
333+
InputStream in = format == GZIP ? new GZIPInputStream(fis) : new BZip2CompressorInputStream(fis)) {
334+
byte[] hdr = in.readNBytes(9);
335+
return magic(hdr, 0, FITS_MAGIC);
336+
} catch (IOException e) {
337+
return false;
338+
}
339+
}
340+
328341
/** Returns true when {@code data[offset..]} starts with the given byte values. */
329342
private static boolean magic(byte[] data, int offset, int... expected) {
330343
if (data.length < offset + expected.length) return false;

‎src/firefly/java/edu/caltech/ipac/util/download/RetrieveUtil.java‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
import java.util.Objects;
1717
import java.util.stream.Stream;
1818

19+
import static edu.caltech.ipac.util.StringUtils.isEmpty;
1920
import static edu.caltech.ipac.util.download.UriRef.ResourceType.GcsCloud;
2021
import static edu.caltech.ipac.util.download.UriRef.ResourceType.OnPrimUrl;
2122
import static edu.caltech.ipac.util.download.UriRef.ResourceType.S3Cloud;
@@ -94,11 +95,13 @@ public static String makeUniqueFileName(UriRef uri, String userLoginName, String
9495
String path;
9596
String outLoc;
9697
String prefix;
98+
String query= null;
9799

98100
switch (uri.ref()) {
99101
case URL url -> {
100102
loc= url.getHost();
101103
path= url.getFile();
104+
query= url.getQuery();
102105
outLoc = FileUtil.makeShortHostName(loc);
103106
prefix= "URL-";
104107
}
@@ -134,7 +137,7 @@ public static String makeUniqueFileName(UriRef uri, String userLoginName, String
134137
retval = prefix + outLoc + "-" + originalHashCode + baseKey;
135138
//note: "=","," signs causes problem in download servlet.
136139
retval = retval.replaceAll("[ :\\[\\]/\\\\|*?<>=,]", "-");
137-
String ext= FileUtil.getExtension(fileStr,true);
140+
String ext= isEmpty(query) ? FileUtil.getExtension(fileStr,true) : "";
138141
var fileExt= EXT_LIST.contains(ext) ? ext : EXT_LIST.getFirst();
139142
return retval + "." + fileExt;
140143
}

‎src/firefly/java/edu/caltech/ipac/util/download/URLDownload.java‎

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -134,17 +134,12 @@ public static Map<String,List<String>> getQueryParams(URL url) {
134134
var paramList= url.getQuery().split("&");
135135
var queryParamMap = new HashMap<String, List<String>>();
136136
for(var p : paramList) {
137-
var parts = p.split("=");
138-
if (parts.length == 2) {
139-
var key= parts[0].trim();
140-
var value= parts[1].trim();
141-
var valueList= queryParamMap.get(key);
142-
if (valueList == null) {
143-
valueList = new ArrayList<>();
144-
queryParamMap.put(key, valueList);
145-
}
146-
valueList.add(value);
147-
}
137+
var parts = p.split("=", 2); // split on the first "=" only, so values can contain "=" (e.g. base64 padding)
138+
var key= parts[0].trim();
139+
if (key.isEmpty()) continue;
140+
var value= parts.length == 2 ? parts[1].trim() : "true"; // a parameter with no value, e.g. ?flag
141+
var valueList = queryParamMap.computeIfAbsent(key, k -> new ArrayList<>());
142+
valueList.add(value);
148143
}
149144
return queryParamMap;
150145
}
@@ -161,7 +156,7 @@ public static String firstParamValUsingKeyList(Map<String, List<String>> params,
161156
return foundKey!=null ? getFirstVal(params, foundKey) : null;
162157
}
163158

164-
private static int codeFromException(Exception e) {
159+
static int codeFromException(Exception e) { // package-private for testing
165160
return switch (e) {
166161
case SSLException ignored -> 495;
167162
case SocketTimeoutException ignored -> HttpURLConnection.HTTP_CLIENT_TIMEOUT;

‎src/firefly/test/edu/caltech/ipac/firefly/server/util/QueryUtilTest.java‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
/*
2+
* License information at https://github.com/Caltech-IPAC/firefly/blob/master/License.txt
3+
*/
4+
5+
package edu.caltech.ipac.firefly.server.util;
6+
7+
import edu.caltech.ipac.firefly.ConfigTest;
8+
import org.junit.Test;
9+
10+
import java.io.File;
11+
import java.io.IOException;
12+
import java.nio.file.Files;
13+
14+
import static org.junit.Assert.assertTrue;
15+
16+
public class QueryUtilTest extends ConfigTest {
17+
18+
@Test
19+
public void tmpFileForUrl() throws IOException {
20+
File dir = Files.createTempDirectory("queryUtilTest").toFile();
21+
dir.deleteOnExit();
22+
23+
assertTmpExt(dir, "http://h/a/x.fits.gz", ".fits.gz");
24+
assertTmpExt(dir, "http://h/a/x.csv", ".csv");
25+
// a query string makes the path extension unreliable
26+
assertTmpExt(dir, "http://h/a/x.fits.gz?token=1", ".ul");
27+
assertTmpExt(dir, "http://h/cgi/get.py?file=a.csv", ".ul");
28+
assertTmpExt(dir, "http://h/a/noext", ".ul");
29+
}
30+
31+
private static void assertTmpExt(File dir, String url, String ext) {
32+
File f = QueryUtil.tmpFileForUrl(url, "req_", dir);
33+
f.deleteOnExit();
34+
assertTrue(url + " -> " + f.getName(), f.getName().endsWith(ext));
35+
}
36+
}

‎src/firefly/test/edu/caltech/ipac/util/FormatUtilTest.java‎

Lines changed: 59 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,25 @@
1313

1414
import java.io.File;
1515
import java.io.IOException;
16+
import java.io.OutputStream;
1617
import java.nio.file.Files;
1718
import java.nio.file.Path;
18-
19-
import static edu.caltech.ipac.util.FormatUtil.Format.*;
19+
import java.nio.file.StandardCopyOption;
20+
import java.util.zip.GZIPOutputStream;
21+
22+
import static edu.caltech.ipac.util.FormatUtil.Format.CSV;
23+
import static edu.caltech.ipac.util.FormatUtil.Format.FITS;
24+
import static edu.caltech.ipac.util.FormatUtil.Format.GZIP;
25+
import static edu.caltech.ipac.util.FormatUtil.Format.HTML;
26+
import static edu.caltech.ipac.util.FormatUtil.Format.IPACTABLE;
27+
import static edu.caltech.ipac.util.FormatUtil.Format.JSON;
28+
import static edu.caltech.ipac.util.FormatUtil.Format.PARQUET;
29+
import static edu.caltech.ipac.util.FormatUtil.Format.PNG;
30+
import static edu.caltech.ipac.util.FormatUtil.Format.REGION;
31+
import static edu.caltech.ipac.util.FormatUtil.Format.TEXT;
32+
import static edu.caltech.ipac.util.FormatUtil.Format.TSV;
33+
import static edu.caltech.ipac.util.FormatUtil.Format.UNKNOWN;
34+
import static edu.caltech.ipac.util.FormatUtil.Format.VO_TABLE;
2035
import static org.junit.Assert.assertEquals;
2136

2237
/**
@@ -204,18 +219,57 @@ public void badMixedDelimiters() throws IOException {
204219
}
205220

206221
@Test
207-
public void mimeType() {
222+
public void mimeType() throws IOException {
208223
File tfile = FileLoader.resolveFile("FileUpload-samples/VOTable/binary/binary_gaia.xml");
209-
assertEquals(tfile.getName(), "application/xml", FormatUtil.getMimeType(tfile).mime());
224+
assertEquals(tfile.getName(), VO_TABLE.mime(), FormatUtil.getMimeType(tfile).mime());
210225

211226
tfile = FileLoader.resolveFile("FileUpload-samples/VOTable/fits/starlinkVOfits.xml");
212-
assertEquals(tfile.getName(), "application/xml", FormatUtil.getMimeType(tfile).mime());
227+
assertEquals(tfile.getName(), VO_TABLE.mime(), FormatUtil.getMimeType(tfile).mime());
213228

214229
tfile = FileLoader.resolveFile("edu/caltech/ipac/firefly/server/query/ptf-lc.tbl");
215230
assertEquals(tfile.getName(), UNKNOWN.mime(), FormatUtil.getMimeType(tfile).mime());
216231

217232
tfile = FileLoader.resolveFile("stripe82-testfits/calexp-i-0-366,0.fits.gz");
218233
String mtype = FormatUtil.getMimeType(tfile).mime();
219234
assertEquals(tfile.getName(), GZIP.mime(), mtype);
235+
assertEquals(tfile.getName(), FITS, FormatUtil.detect(tfile));
236+
237+
// 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)
238+
Path tmp = Files.createTempFile("req_", ".ul");
239+
tmp.toFile().deleteOnExit();
240+
Files.copy(tfile.toPath(), tmp, StandardCopyOption.REPLACE_EXISTING);
241+
assertEquals("gzipped fits named .ul", FITS, FormatUtil.detect(tmp.toFile()));
242+
}
243+
244+
@Test
245+
public void extensionDoesNotDecideFormat() throws IOException {
246+
// a .fits name must not decide the format; on linux /etc/mime.types maps .fits to image/fits
247+
Path tmp = Files.createTempFile("test", ".fits");
248+
tmp.toFile().deleteOnExit();
249+
Files.writeString(tmp, "a,b,c\n1,2,3\n4,5,6\n");
250+
assertEquals("csv named .fits", CSV, FormatUtil.detect(tmp.toFile()));
251+
}
252+
253+
@Test
254+
public void voTableWithWrongExtension() throws IOException {
255+
File tfile = FileLoader.resolveFile("FileUpload-samples/VOTable/binary/binary_gaia.xml");
256+
for (String ext : new String[] {".csv", ".json"}) {
257+
Path tmp = Files.createTempFile("test", ext);
258+
tmp.toFile().deleteOnExit();
259+
Files.copy(tfile.toPath(), tmp, StandardCopyOption.REPLACE_EXISTING);
260+
assertEquals("votable named " + ext, VO_TABLE.mime(), FormatUtil.getMimeType(tmp.toFile()).mime());
261+
assertEquals("votable named " + ext, VO_TABLE, FormatUtil.detect(tmp.toFile()));
262+
}
263+
}
264+
265+
@Test
266+
public void gzipNotFitsNamedFits() throws IOException {
267+
// the name says fits but the decompressed content does not
268+
Path tmp = Files.createTempFile("test", ".fits.gz");
269+
tmp.toFile().deleteOnExit();
270+
try (OutputStream out = new GZIPOutputStream(Files.newOutputStream(tmp))) {
271+
out.write("a,b,c\n1,2,3\n".getBytes());
272+
}
273+
assertEquals("gzipped csv named .fits.gz", GZIP, FormatUtil.detect(tmp.toFile()));
220274
}
221275
}

0 commit comments

Comments
 (0)