From 32be35ed392a0d5e890f4df773500f4e85e9d432 Mon Sep 17 00:00:00 2001 From: loi Date: Thu, 24 Sep 2026 11:45:29 -0700 Subject: [PATCH 1/2] FIREFLY-2064: Failed to save large downloads - switch response.blob() to iframe form post - fix suggested name to support additional standards --- .../ipac/firefly/server/RequestAgent.java | 25 ++++ .../server/servlets/AnyFileDownload.java | 16 ++- .../server/servlets/CommandService.java | 11 ++ .../server/servlets/HttpServCommands.java | 4 +- .../ipac/util/download/URLDownload.java | 41 +++++- src/firefly/js/core/background/JobMonitor.jsx | 6 +- src/firefly/js/tables/ui/TableSave.jsx | 51 ++++--- src/firefly/js/util/fetch.js | 131 +++++++++++++++--- .../ipac/util/download/URLDownloadTest.java | 48 +++++++ 9 files changed, 282 insertions(+), 51 deletions(-) create mode 100644 src/firefly/test/edu/caltech/ipac/util/download/URLDownloadTest.java diff --git a/src/firefly/java/edu/caltech/ipac/firefly/server/RequestAgent.java b/src/firefly/java/edu/caltech/ipac/firefly/server/RequestAgent.java index 04f2b8a045..9b9b8f4481 100644 --- a/src/firefly/java/edu/caltech/ipac/firefly/server/RequestAgent.java +++ b/src/firefly/java/edu/caltech/ipac/firefly/server/RequestAgent.java @@ -12,9 +12,12 @@ import jakarta.servlet.http.HttpServletResponse; import java.io.File; import java.io.IOException; +import java.net.URLDecoder; +import java.nio.charset.StandardCharsets; import java.util.Arrays; import java.util.Collections; import java.util.HashMap; +import java.util.List; import java.util.Map; import static edu.caltech.ipac.util.StringUtils.applyIfNotEmpty; @@ -158,6 +161,8 @@ protected Map extractCookies() { public static final class HTTP extends RequestAgent { private static final String AUTH_KEY = "JOSSO_SESSIONID"; + // headers that may be passed as query parameters instead, e.g. when downloading via form submit, which cannot set headers + private static final List QUERY_PARAM_HEADERS = List.of("FF-connID", "FF-channel"); private static final Logger.LoggerImpl LOG = Logger.getLogger(); private final HashMap headers = new HashMap<>(); // key stored as lowercase; private final HashMap cookies = new HashMap<>(); @@ -172,6 +177,11 @@ public HTTP(HttpServletRequest request, HttpServletResponse response) { Collections.list(request.getHeaderNames()).forEach(h -> { headers.put(h.toLowerCase(), request.getHeader(h)); }); + QUERY_PARAM_HEADERS.forEach(h -> { + if (StringUtils.isEmpty(headers.get(h.toLowerCase()))) { + applyIfNotEmpty(getQueryParam(request, h), v -> headers.put(h.toLowerCase(), v)); + } + }); applyIfNotEmpty(request.getCookies(), v -> { Arrays.stream(v).forEach(c -> cookies.put(c.getName(), c)); }); @@ -242,6 +252,21 @@ public void sendCookie(Cookie cookie) { } } + /** + * Read a parameter from the query string only. Unlike request.getParameter, this does not consume the body of a POST request. + */ + private static String getQueryParam(HttpServletRequest request, String name) { + String qs = request.getQueryString(); + if (StringUtils.isEmpty(qs)) return null; + for (String kv : qs.split("&")) { + String[] parts = kv.split("=", 2); + if (URLDecoder.decode(parts[0], StandardCharsets.UTF_8).equals(name)) { + return parts.length > 1 ? URLDecoder.decode(parts[1], StandardCharsets.UTF_8) : ""; + } + } + return null; + } + @Override public String getHeader(String name, String def) { String retval = name == null ? null : headers.get(name.toLowerCase()); diff --git a/src/firefly/java/edu/caltech/ipac/firefly/server/servlets/AnyFileDownload.java b/src/firefly/java/edu/caltech/ipac/firefly/server/servlets/AnyFileDownload.java index 1c0e7e9946..1206ca2997 100644 --- a/src/firefly/java/edu/caltech/ipac/firefly/server/servlets/AnyFileDownload.java +++ b/src/firefly/java/edu/caltech/ipac/firefly/server/servlets/AnyFileDownload.java @@ -13,9 +13,11 @@ import edu.caltech.ipac.util.cache.Cache; import edu.caltech.ipac.util.cache.CacheManager; import edu.caltech.ipac.util.download.FailedRequestException; +import edu.caltech.ipac.util.download.URLDownload; import edu.caltech.ipac.util.download.UriRef; import edu.caltech.ipac.util.download.UriRefParams; import edu.caltech.ipac.visualize.net.URLParms; +import jakarta.servlet.http.Cookie; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; @@ -43,6 +45,7 @@ public class AnyFileDownload extends BaseHttpServlet { public static final String FILE_PARAM = "file"; // required for other request public static final String RETURN_PARAM= "return"; // a name for the file, if empty or USE_SERVER_NAME the use file name on server public static final String LOG_PARAM = "log"; // if true, log status + public static final String DOWNLOAD_TOKEN = "ffDownloadToken"; // parameter sent by the client to be notified when the download starts public static final String USE_SERVER_NAME = "USE_SERVER_NAME"; @@ -52,6 +55,7 @@ public class AnyFileDownload extends BaseHttpServlet { private static final String BASE_URL = "servlet/Download?"+ LOG_PARAM +"=true&" + FILE_PARAM +"="; private static final String RET_FILE = "&"+RETURN_PARAM+"="; + private static final String DOWNLOAD_COOKIE_PREFIX = "ffdl_"; public static String getDownloadURL(File file, String suggestedFileName) { return getDownloadURL(file, suggestedFileName, ServerContext.getRequestOwner().getBaseUrl()); @@ -206,6 +210,15 @@ private void handleExternalURLRequest(HttpServletRequest req, HttpServletRespons } + /** Let the client know the download has started by setting a short-lived cookie named after its download token. */ + public static void sendDownloadStartedCookie(String token, HttpServletResponse res) { + if (isEmpty(token) || !token.matches("[A-Za-z0-9_-]{1,64}")) return; + Cookie cookie = new Cookie(DOWNLOAD_COOKIE_PREFIX + token, "1"); + cookie.setPath("/"); + cookie.setMaxAge(60); + res.addCookie(cookie); + } + private void sendFileToClient(HttpServletRequest req, HttpServletResponse res, File f, String local) throws IOException { SrvParam sp= new SrvParam(req.getParameterMap()); boolean log= sp.getOptionalBoolean(LOG_PARAM,false); @@ -218,8 +231,9 @@ private void sendFileToClient(HttpServletRequest req, HttpServletResponse res, F String retFileStr= (local!=null) ? local : f.getName(); if (retFileStr.equals(USE_SERVER_NAME)) retFileStr= f.getName(); if (!isEmpty(retFileStr)) { - res.addHeader("Content-Disposition", "attachment; filename=" + retFileStr); + res.addHeader("Content-Disposition", URLDownload.makeContentDisposition(retFileStr)); } + sendDownloadStartedCookie(sp.getOptional(DOWNLOAD_TOKEN), res); diff --git a/src/firefly/java/edu/caltech/ipac/firefly/server/servlets/CommandService.java b/src/firefly/java/edu/caltech/ipac/firefly/server/servlets/CommandService.java index c5d373ffce..3796e7c47d 100644 --- a/src/firefly/java/edu/caltech/ipac/firefly/server/servlets/CommandService.java +++ b/src/firefly/java/edu/caltech/ipac/firefly/server/servlets/CommandService.java @@ -8,6 +8,7 @@ import static edu.caltech.ipac.util.FormatUtil.Format.*; import edu.caltech.ipac.firefly.server.ServerContext; import edu.caltech.ipac.firefly.server.SrvParam; +import edu.caltech.ipac.firefly.server.filters.CorsFilter; import edu.caltech.ipac.firefly.server.util.Logger; import org.json.simple.JSONObject; @@ -42,6 +43,7 @@ public static void sendError(HttpServletRequest req, HttpServletResponse res, Ma rval.put("success", false); rval.put("error", error); String msg = rval.toJSONString(); + resetForError(req, res); res.setStatus(statusCode); res.setContentLength(msg.length()); res.setContentType(JSON.mime()); @@ -54,6 +56,15 @@ public static void sendError(HttpServletRequest req, HttpServletResponse res, Ma } + /** + * If a download fails before sending data, reset the response to prevent downloading the error. + */ + private static void resetForError(HttpServletRequest req, HttpServletResponse res) { + if (res.isCommitted() || !res.containsHeader("Content-Disposition")) return; + res.reset(); + CorsFilter.enableCors(req, res); + } + protected void processRequest(HttpServletRequest req, HttpServletResponse res) throws Exception { try { long start = System.currentTimeMillis(); diff --git a/src/firefly/java/edu/caltech/ipac/firefly/server/servlets/HttpServCommands.java b/src/firefly/java/edu/caltech/ipac/firefly/server/servlets/HttpServCommands.java index c89405b9d7..dda344dfee 100644 --- a/src/firefly/java/edu/caltech/ipac/firefly/server/servlets/HttpServCommands.java +++ b/src/firefly/java/edu/caltech/ipac/firefly/server/servlets/HttpServCommands.java @@ -17,6 +17,7 @@ import edu.caltech.ipac.util.FormatUtil; import edu.caltech.ipac.util.StringUtils; import edu.caltech.ipac.table.TableUtil; +import edu.caltech.ipac.util.download.URLDownload; import org.json.simple.JSONObject; import jakarta.servlet.http.HttpServletRequest; @@ -85,7 +86,8 @@ public void processRequest(HttpServletRequest req, HttpServletResponse res, SrvP return; } - res.setHeader("Content-Disposition", "attachment; filename=" + fileName + fileNameExt); + res.setHeader("Content-Disposition", URLDownload.makeContentDisposition(fileName + fileNameExt)); + AnyFileDownload.sendDownloadStartedCookie(sp.getOptional(AnyFileDownload.DOWNLOAD_TOKEN), res); FileInfo fi = am.save(res.getOutputStream(), request, tblFormat, mode); if (fi != null) { long length = fi.getSizeInBytes(); // if written from the db, the length is 0 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 ac0be581cb..b1f73b03d3 100644 --- a/src/firefly/java/edu/caltech/ipac/util/download/URLDownload.java +++ b/src/firefly/java/edu/caltech/ipac/util/download/URLDownload.java @@ -36,8 +36,12 @@ import java.net.URI; import java.net.URL; import java.net.URLConnection; +import java.net.URLDecoder; +import java.net.URLEncoder; import java.net.UnknownHostException; import java.nio.ByteBuffer; +import java.nio.charset.Charset; +import java.nio.charset.StandardCharsets; import java.security.KeyManagementException; import java.security.NoSuchAlgorithmException; import java.security.cert.X509Certificate; @@ -49,6 +53,8 @@ import java.util.List; import java.util.Map; import java.util.Set; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import java.util.zip.GZIPInputStream; import java.util.zip.InflaterInputStream; @@ -174,17 +180,42 @@ private static FileInfo exceptionToFileInfo(Exception e) { return new FileInfo(codeFromException(e),ResponseMessage.getNetworkCallFailureMessage(e)); } + private static final Pattern DISP_EXT_FILENAME = Pattern.compile("filename\\*\\s*=\\s*([^']*)'[^']*'([^;\\s]+)", Pattern.CASE_INSENSITIVE); + private static final Pattern DISP_FILENAME = Pattern.compile("filename\\s*=\\s*(?:\"([^\"]*)\"|([^;]+))", Pattern.CASE_INSENSITIVE); + + /** + * Parse the file name from a Content-Disposition header value. + * Handles quoted and unquoted filename, and the RFC 6266 filename* extended form, which takes precedence. + * @param disposition the Content-Disposition header value + * @return a sanitized file name, or null if none is found + */ public static String getSuggestedFileName(String disposition) { if (disposition == null) return null; - String[] strs = disposition.split(";"); - if (strs.length != 2) return null; - String[] fname = strs[1].split("="); - if (fname[0].toLowerCase().contains("filename")) { - return sanitizeFilename(fname[1]); + Matcher m = DISP_EXT_FILENAME.matcher(disposition); + if (m.find()) { + try { + Charset cs = isEmpty(m.group(1)) ? StandardCharsets.UTF_8 : Charset.forName(m.group(1).trim()); + return sanitizeFilename(URLDecoder.decode(m.group(2).replace("+", "%2B"), cs)); + } catch (Exception ignored) {} // bad encoding; fall back to filename } + m = DISP_FILENAME.matcher(disposition); + if (m.find()) return sanitizeFilename(m.group(1) != null ? m.group(1) : m.group(2)); return null; } + /** + * Create a Content-Disposition header value for sending the given file name as an attachment. + * Includes a quoted ASCII-only filename for older clients, and filename* (RFC 6266) to preserve the full name. + * @param fileName the file name the client should save as + * @return the header value + */ + public static String makeContentDisposition(String fileName) { + String name = fileName == null ? "" : fileName.replaceAll("[\\p{Cntrl}/\\\\]", "_"); // no control chars or path separators + String asciiName = name.replaceAll("[^\\x20-\\x7E]|\"", "_"); + String encoded = URLEncoder.encode(name, StandardCharsets.UTF_8).replace("+", "%20").replace("*", "%2A"); + return "attachment; filename=\"" + asciiName + "\"; filename*=UTF-8''" + encoded; + } + public static String getFileNameFromUrl(URL url) { if (url == null) return null; String urlPath = url.getPath(); diff --git a/src/firefly/js/core/background/JobMonitor.jsx b/src/firefly/js/core/background/JobMonitor.jsx index eaf5d86ec8..adc8522709 100644 --- a/src/firefly/js/core/background/JobMonitor.jsx +++ b/src/firefly/js/core/background/JobMonitor.jsx @@ -523,10 +523,8 @@ function doDownload(job, index) { return; } dispatchBgJobInfo( updateSet(job, ['downloadState',index], 'WORKING') ); - download(url).then( () => { - dispatchBgJobInfo( updateSet(job, ['downloadState',index], 'DONE') ); - }).catch(() => { - dispatchBgJobInfo( updateSet(job, ['downloadState',index], 'FAIL') ); + download(url).then( (started) => { + dispatchBgJobInfo( updateSet(job, ['downloadState',index], started ? 'DONE' : 'FAIL') ); }); } diff --git a/src/firefly/js/tables/ui/TableSave.jsx b/src/firefly/js/tables/ui/TableSave.jsx index 2f29b95d88..ba4a51607c 100644 --- a/src/firefly/js/tables/ui/TableSave.jsx +++ b/src/firefly/js/tables/ui/TableSave.jsx @@ -20,17 +20,19 @@ import {DownloadOptionsDialog, fileNameValidator, getTypeData, import {WS_SERVER_PARAM, isWsFolder, isValidWSFolder, getWorkspacePath, dispatchWorkspaceUpdate} from '../../visualize/WorkspaceCntlr.js'; import {ServerParams} from '../../data/ServerParams.js'; -import {INFO_POPUP} from '../../ui/PopupUtil.jsx'; +import {INFO_POPUP, showInfoPopup} from '../../ui/PopupUtil.jsx'; import FieldGroupCntlr from '../../fieldGroup/FieldGroupCntlr.js'; import {getFieldVal} from '../../fieldGroup/FieldGroupUtils.js'; import {getWorkspaceConfig} from '../../visualize/WorkspaceCntlr.js'; import {ListBoxInputField} from '../../ui/ListBoxInputField.jsx'; import {download, makeDefaultDownloadFileName} from '../../util/fetch'; -import {getCmdSrvSyncURL} from '../../util/WebUtil'; +import {callWhileAwaiting, getCmdSrvSyncURL} from '../../util/WebUtil'; import {RadioGroupInputField} from 'firefly/ui/RadioGroupInputField.jsx'; import {useStoreConnector} from 'firefly/ui/SimpleComponent.jsx'; import {findTableCenterColumns} from 'firefly/voAnalyzer/TableAnalysis'; import {Stacker} from 'firefly/ui/Stacker.jsx'; +import {IfWorkingMaskById} from 'firefly/ui/panel/MaskPanel.jsx'; +import {dispatchAddWorkingTask} from 'firefly/core/AppDataCntlr.js'; const fKeyDef = { fileName: {fKey: 'fileName', label: 'File name'}, @@ -79,6 +81,8 @@ const defValues = { }; const tblDownloadGroupKey = 'TABLE_DOWNLOAD_FORM'; +const TBL_SAVE_MASK_ID = 'TableSavePanel'; +const WORKING_DELAY = 1000; // ms before showing the working mask export function showTableDownloadDialog({tbl_id, tbl_ui_id}) { return () => { @@ -136,7 +140,7 @@ function TableSavePanel({tbl_id, tbl_ui_id, onComplete}) { const sizing = wsSelected ? {height:'60vh', minHeight:'28em', resize:'both'} : isWs ? {height:'20em'} : {height:'19em'}; return ( - + @@ -162,6 +166,7 @@ function TableSavePanel({tbl_id, tbl_ui_id, onComplete}) { text={'Save'}/> + ); } @@ -257,32 +262,40 @@ function resultSuccess(tbl_id, tbl_ui_id, onComplete, cenCols) { return Object.assign(params, {file_format : fileFormat, mode}); }; + // resolves to true when the file is on its way; false if it failed and the error was shown const downloadFile = (urlOrOp) => { if (isWorkspace()) { doDownloadWorkspace(getCmdSrvSyncURL(), {params: urlOrOp}); + return true; } else { - download(urlOrOp); + return download(urlOrOp); } - - onComplete?.(); }; - const {origTableModel} = getTblById(tbl_id) || {}; - if (origTableModel) { - getAsyncTableSourceUrl(tbl_ui_id, getOtherParams(fileName)).then((urlOrOp) => { - downloadFile(urlOrOp); - }); - } else { - let urlOrOp; - if (tbl_ui_id) { - urlOrOp= getTableSourceUrl(tbl_ui_id, getOtherParams(fileName)); - } - else { + const getUrlOrOp = async () => { + const {origTableModel} = getTblById(tbl_id) || {}; + if (origTableModel) { + return getAsyncTableSourceUrl(tbl_ui_id, getOtherParams(fileName)); + } else if (tbl_ui_id) { + return getTableSourceUrl(tbl_ui_id, getOtherParams(fileName)); + } else { const table= getTblById(tbl_id); - urlOrOp = makeTableSourceUrl(table.tableData.columns, table.request, getOtherParams(fileName)); + return makeTableSourceUrl(table.tableData.columns, table.request, getOtherParams(fileName)); } - downloadFile(urlOrOp); + }; + + const saving = getUrlOrOp() + .then(downloadFile) + .catch((e) => { + showInfoPopup(e?.message || 'Unable to save the table', 'Save table'); + return false; + }); + + // if the download has not started within WORKING_DELAY, mask the dialog until it does, then close it. on error, keep it open. + if (!isWorkspace()) { + callWhileAwaiting(saving, (p) => dispatchAddWorkingTask(TBL_SAVE_MASK_ID, p, 'Preparing file for download...'), WORKING_DELAY); } + saving.then((ok) => ok && onComplete?.()); }; } diff --git a/src/firefly/js/util/fetch.js b/src/firefly/js/util/fetch.js index e91f9467c8..fe1dd20500 100644 --- a/src/firefly/js/util/fetch.js +++ b/src/firefly/js/util/fetch.js @@ -7,7 +7,7 @@ import {getOrCreateWsConn} from '../core/messaging/WebSocketClient.js'; import {ServerParams} from '../data/ServerParams.js'; import {showInfoPopup} from '../ui/PopupUtil.jsx'; import {logger} from './Logger.js'; -import {parseUrl, AJAX_REQUEST, lowLevelDoFetch, REQUEST_WITH, WS_CHANNEL_HD, WS_CONNID_HD} from './WebUtil.js'; +import {encodeParams, parseUrl, toNameValuePairs, AJAX_REQUEST, lowLevelDoFetch, REQUEST_WITH, WS_CHANNEL_HD, WS_CONNID_HD} from './WebUtil.js'; /** @@ -60,42 +60,131 @@ export function downloadBlob(blob, filename) { a.download = filename; document.body.appendChild(a); a.click(); - window.URL.revokeObjectURL(a.href); document.body.removeChild(a); + setTimeout(() => window.URL.revokeObjectURL(a.href), 10000); // revoking right away may cancel the download in some browsers } +const DOWNLOAD_ERROR_MSG = 'Download failed. The server did not return a file.'; +const DOWNLOAD_TOKEN = 'ffDownloadToken'; // must match AnyFileDownload.DOWNLOAD_TOKEN +const DOWNLOAD_COOKIE_PREFIX = 'ffdl_'; // must match AnyFileDownload.sendDownloadStartedCookie +const POLL_INTERVAL = 500; +const MAX_WAIT = 30 * 60 * 1000; // stop waiting for the download to start after this long + /** - * return the filename from the Content-Disposition header or undefined - * @param resp - fetch response - * @return {string|undefined} + * Downloads a file via a hidden form/iframe, allowing the browser to stream it directly to disk. + * Use a token cookie to detect when the download starts. + * @param {string} url the url to download. Its query parameters are sent as a POST body. + * @return {Promise} true when started, false on error; never rejects. */ -function resolveFileName(resp) { - if (resp && resp.headers) { - const cd = resp.headers.get('Content-Disposition') || ''; - const parts = cd.match(/.*filename=(.*)/); - const possibleName= (parts && parts.length > 1) ? parts[1] : undefined; - return possibleName?.indexOf('?')>4 ? possibleName.split('?')[0] : possibleName; - } -} +export async function download(url) { + if (!url) return false; + const {origin, protocol, host, path, hash, searchObject = {}} = parseUrl(url); + const {connId, channel} = await getOrCreateWsConn(); + const token = Date.now() + '-' + Math.random().toString(36).slice(2); -export async function download(url, filename) { - const {protocol, host, path, hash, searchObject = {}} = parseUrl(url); + // cmd and websocket info go on the query string; the server reads them before the body is parsed const cmd = searchObject[ServerParams.COMMAND]; - url = cmd ? `${protocol}//${host}${path}?${ServerParams.COMMAND}=${cmd}` + (hash ? '#' + hash : '') : url;// add cmd into the url as a workaround for server-side code not supporting it + const query = encodeParams({...(cmd && {[ServerParams.COMMAND]: cmd}), [WS_CONNID_HD]: connId, [WS_CHANNEL_HD]: channel, [DOWNLOAD_TOKEN]: token}); + const action = `${protocol}//${host}${path}?${query}` + (hash ? '#' + hash : ''); const params = Object.fromEntries( Object.entries(searchObject) + .filter(([k]) => k !== ServerParams.COMMAND) .map(([k, v]) => [k, (isPlainObject(v) ? JSON.stringify(v) : v)])); // convert object back into JSON if needed. + const iframe = document.createElement('iframe'); + iframe.name = 'ff-download-' + token; + iframe.style.display = 'none'; + document.body.appendChild(iframe); + + const failed = waitForErrorPage(iframe).then((msg) => { + showInfoPopup(truncate(msg, {length: 200}), 'Unexpected error'); + iframe.remove(); + }); + + submitForm(action, params, iframe.name); + + if (origin !== window.location.origin) return true; // cannot tell when it starts + + let stopped = false; try { - const resp= await fetchUrl(url, {method: 'post', params}); - filename = filename || resolveFileName(resp); - downloadBlob(await resp.blob(), filename); - } catch ({message}) { - showInfoPopup(truncate(message, {length: 200}), 'Unexpected error'); + return await Promise.race([ + waitForCookie(DOWNLOAD_COOKIE_PREFIX + token, () => stopped).then(() => true), + failed.then(() => false) + ]); + } finally { + stopped = true; // stop polling for the cookie if the error came first } } +/** + * The iframe's load event only fires when the server returned a page instead of a file, i.e. an error. + * @param {HTMLIFrameElement} iframe + * @return {Promise} resolves to the page's text when readable (same origin), otherwise a generic message. + */ +function waitForErrorPage(iframe) { + return new Promise((resolve) => { + iframe.onload = () => { + let msg; + try { + if (iframe.contentWindow.location.href === 'about:blank') return; // initial load of the empty iframe + const doc = iframe.contentDocument; + const text = doc?.body?.innerText?.trim(); + msg = doc?.contentType === 'application/json' ? getJsonErrorMsg(text) : text; + } catch {} + resolve(msg || DOWNLOAD_ERROR_MSG); + }; + }); +} + +/** + * Extracts the root-cause message from a server JSON error. + * @return {string|undefined} the error message, or undefined if invalid. + */ +function getJsonErrorMsg(text) { + try { + const causes = Object.values(JSON.parse(text)?.error ?? {}); + return causes.at(-1)?.replace(/^[\w.$]+: /, ''); // the last one is most specific; remove ClassName info + } catch { + return undefined; // not JSON, e.g. the browser rendered it with a JSON viewer + } +} + +/** + * Wait for the cookie to appear, then remove it. + * @param {string} name cookie name + * @param {function(): boolean} isStopped stop waiting when it returns true + * @return {Promise} resolves when the cookie appears, when stopped, or after MAX_WAIT + */ +async function waitForCookie(name, isStopped) { + const startTime = Date.now(); + while (!isStopped() && Date.now() - startTime < MAX_WAIT) { + if (document.cookie.split(';').some((c) => c.trim().startsWith(name + '='))) { + document.cookie = `${name}=; max-age=0; path=/`; + return; + } + await new Promise((resolve) => setTimeout(resolve, POLL_INTERVAL)); + } +} + +function submitForm(action, params, target) { + const form = document.createElement('form'); + form.method = 'post'; + form.action = action; + form.target = target; + form.style.display = 'none'; + toNameValuePairs(params).forEach(({name, value = ''}) => { + const input = document.createElement('input'); + input.type = 'hidden'; + input.name = name; + input.value = value; + form.appendChild(input); + }); + document.body.appendChild(form); + form.submit(); + form.remove(); +} + /** * create the default download filename diff --git a/src/firefly/test/edu/caltech/ipac/util/download/URLDownloadTest.java b/src/firefly/test/edu/caltech/ipac/util/download/URLDownloadTest.java new file mode 100644 index 0000000000..706f65e99a --- /dev/null +++ b/src/firefly/test/edu/caltech/ipac/util/download/URLDownloadTest.java @@ -0,0 +1,48 @@ +/* + * License information at https://github.com/Caltech-IPAC/firefly/blob/master/License.txt + */ + +package edu.caltech.ipac.util.download; + +import edu.caltech.ipac.firefly.ConfigTest; +import org.junit.Assert; +import org.junit.Test; + +import static edu.caltech.ipac.util.download.URLDownload.getSuggestedFileName; +import static edu.caltech.ipac.util.download.URLDownload.makeContentDisposition; + +public class URLDownloadTest extends ConfigTest { + + @Test + public void makeHeader() { + Assert.assertEquals("attachment; filename=\"table.csv\"; filename*=UTF-8''table.csv", + makeContentDisposition("table.csv")); + + // characters that break an unquoted header value + Assert.assertEquals("attachment; filename=\"a, b; c.csv\"; filename*=UTF-8''a%2C%20b%3B%20c.csv", + makeContentDisposition("a, b; c.csv")); + + // quote, path separators, control chars and non-ascii + Assert.assertEquals("attachment; filename=\"_x_y_z__.fits\"; filename*=UTF-8''%22x_y_z_%C3%A9.fits", + makeContentDisposition("\"x/y\\z\né.fits")); + Assert.assertTrue(makeContentDisposition("é*.fits").endsWith("filename*=UTF-8''%C3%A9%2A.fits")); + } + + @Test + public void parseHeader() { + Assert.assertEquals("table.csv", getSuggestedFileName("attachment; filename=table.csv")); + Assert.assertEquals("table.csv", getSuggestedFileName("attachment; filename=\"table.csv\"")); + Assert.assertEquals("a__b.csv", getSuggestedFileName("attachment; filename=\"a; b.csv\"")); + Assert.assertEquals("table.csv", getSuggestedFileName("inline; filename=table.csv; size=100")); + Assert.assertEquals("t_1.csv", getSuggestedFileName("attachment; filename=\"fallback.csv\"; filename*=UTF-8''t%201.csv")); + Assert.assertNull(getSuggestedFileName("attachment")); + } + + @Test + public void roundTrip() { + for (String name : new String[] {"table.csv", "my table, v2.tbl", "image;1.fits"}) { + String expected = URLDownload.sanitizeFilename(name); + Assert.assertEquals(expected, getSuggestedFileName(makeContentDisposition(name))); + } + } +} From 81433eebaee66244b023d04807da545714db2e76 Mon Sep 17 00:00:00 2001 From: loi Date: Thu, 24 Sep 2026 17:15:19 -0700 Subject: [PATCH 2/2] address reviewer comments --- src/firefly/js/util/fetch.js | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/src/firefly/js/util/fetch.js b/src/firefly/js/util/fetch.js index fe1dd20500..b3f4b2e4cb 100644 --- a/src/firefly/js/util/fetch.js +++ b/src/firefly/js/util/fetch.js @@ -77,6 +77,16 @@ const MAX_WAIT = 30 * 60 * 1000; // stop waiting for the down * @return {Promise} true when started, false on error; never rejects. */ export async function download(url) { + try { + return await submitDownload(url); + } catch (e) { + // e.g. websocket setup failed or the url is malformed + showInfoPopup(truncate(e?.message || DOWNLOAD_ERROR_MSG, {length: 200}), 'Unexpected error'); + return false; + } +} + +async function submitDownload(url) { if (!url) return false; const {origin, protocol, host, path, hash, searchObject = {}} = parseUrl(url); const {connId, channel} = await getOrCreateWsConn(); @@ -109,7 +119,10 @@ export async function download(url) { let stopped = false; try { return await Promise.race([ - waitForCookie(DOWNLOAD_COOKIE_PREFIX + token, () => stopped).then(() => true), + waitForCookie(DOWNLOAD_COOKIE_PREFIX + token, () => stopped).then((found) => { + if (found) setTimeout(() => iframe.remove(), 60_000); // Handed off to the browser's download manager. Safe to remove the iframe after a brief delay. + return true; // on timeout: cannot tell, same as cross-origin + }), failed.then(() => false) ]); } finally { @@ -154,17 +167,18 @@ function getJsonErrorMsg(text) { * Wait for the cookie to appear, then remove it. * @param {string} name cookie name * @param {function(): boolean} isStopped stop waiting when it returns true - * @return {Promise} resolves when the cookie appears, when stopped, or after MAX_WAIT + * @return {Promise} true if the cookie appeared; false if stopped or timed out after MAX_WAIT */ async function waitForCookie(name, isStopped) { const startTime = Date.now(); while (!isStopped() && Date.now() - startTime < MAX_WAIT) { if (document.cookie.split(';').some((c) => c.trim().startsWith(name + '='))) { document.cookie = `${name}=; max-age=0; path=/`; - return; + return true; } await new Promise((resolve) => setTimeout(resolve, POLL_INTERVAL)); } + return false; } function submitForm(action, params, target) {