diff --git a/CHANGELOG.md b/CHANGELOG.md index cdcbb38..f8a8fac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,16 @@ +in development +-------------- +- Serve certain small files inline rather than redirecting to S3, so that they + display in the browser instead of being downloaded: + - JSON files (any `.json` file, plus Zarr metadata files `.zattrs`, + `.zarray`, and `.zgroup`), served as `application/json` + - YAML files (`.yaml`, `.yml`), served as `text/yaml`, matching the virtual + `dandiset.yaml` files + - Other text-based files (`.tsv`, `.csv`, `.md`, `.txt`, and `.bidsignore`), + served as `text/plain` (which browsers reliably display inline) + Only files no larger than 10 MiB are served inline; larger files continue to + be redirected to S3 as before. + v0.7.0 (2025-11-11) ------------------- - Use the Archive API's `/webdav/assets/atpath/` endpoint for fetching diff --git a/src/consts.rs b/src/consts.rs index 32153ce..9f7d04b 100644 --- a/src/consts.rs +++ b/src/consts.rs @@ -32,14 +32,50 @@ pub(crate) static HTML_CONTENT_TYPE: &str = "text/html; charset=utf-8"; /// The "Content-Type" value for the stylesheet pub(crate) static CSS_CONTENT_TYPE: &str = "text/css; charset=utf-8"; -/// The "Content-Type" value (reported in both `GET` and `PROPFIND` responses) -/// for virtual `dandiset.yaml` files +/// The "Content-Type" value used when serving YAML files inline: the virtual +/// `dandiset.yaml` files as well as general `.yaml`/`.yml` files (reported in +/// both `GET` and `PROPFIND` responses for the former) pub(crate) static YAML_CONTENT_TYPE: &str = "text/yaml; charset=utf-8"; /// The "Content-Type" value given in `PROPFIND` responses for blob assets with /// no `encodingFormat` set pub(crate) static DEFAULT_CONTENT_TYPE: &str = "application/octet-stream"; +/// The "Content-Type" value used when serving JSON files (general `.json` +/// files as well as Zarr metadata) inline +pub(crate) static JSON_CONTENT_TYPE: &str = "application/json; charset=utf-8"; + +/// The "Content-Type" value used when serving other text-based files inline. +/// +/// `text/plain` is used rather than more specific types (such as +/// `text/tab-separated-values` or `text/markdown`) because browsers reliably +/// display `text/plain` inline, whereas they tend to download the more +/// specific types. +pub(crate) static PLAIN_TEXT_CONTENT_TYPE: &str = "text/plain; charset=utf-8"; + +/// File extensions (case insensitive, without the leading period) of +/// text-based files that `dandidav` fetches and serves inline as `text/plain` +/// (so that they display in the browser rather than being downloaded). JSON +/// and YAML files are handled separately; see [`JSON_CONTENT_TYPE`] and +/// [`YAML_CONTENT_TYPE`]. +pub(crate) static INLINE_TEXT_EXTENSIONS: [&str; 5] = ["tsv", "csv", "md", "txt", "bidsignore"]; + +/// The base names of Zarr metadata files. +/// +/// Rather than redirecting `GET` requests for these files to S3 (which serves +/// them with a `binary/octet-stream` content type, causing browsers to +/// download rather than display them), `dandidav` fetches their (small) +/// contents itself and serves them inline as JSON. This covers both Zarr v2 +/// (`.zgroup`, `.zarray`, `.zattrs`) and Zarr v3 (`zarr.json`) metadata. +pub(crate) static ZARR_METADATA_FILENAMES: [&str; 4] = + [".zgroup", ".zarray", ".zattrs", "zarr.json"]; + +/// The maximum size (in bytes) of a file that `dandidav` will fetch and serve +/// inline (so that it displays in the browser rather than downloading). Files +/// larger than this are redirected to S3 as usual, to avoid buffering large +/// amounts of data in memory. +pub(crate) static MAX_INLINE_SIZE: i64 = 10 * 1024 * 1024; + /// The "Content-Type" value for `PROPFIND` XML responses /// /// Quoth §8.2 of RFC 4918: diff --git a/src/dav/mod.rs b/src/dav/mod.rs index 3b3c6d7..eadab9d 100644 --- a/src/dav/mod.rs +++ b/src/dav/mod.rs @@ -12,6 +12,7 @@ use self::util::*; use self::xml::*; use crate::consts::{DAV_XML_CONTENT_TYPE, HTML_CONTENT_TYPE}; use crate::dandi::*; +use crate::httputil::{Client, HttpError}; use crate::paths::Component; use crate::paths::PurePath; use crate::zarrman::*; @@ -42,6 +43,11 @@ pub(crate) struct DandiDav { /// pub(crate) zarrman: ZarrManClient, + /// A general-purpose HTTP client used to fetch the contents of resources + /// that `dandidav` serves inline (i.e., Zarr metadata files) rather than + /// redirecting to + pub(crate) web: Client, + /// Manager for templating of HTML responses pub(crate) templater: Templater, @@ -125,6 +131,13 @@ impl DandiDav { Redirect::temporary(redir.get_url(self.prefer_s3_redirects).as_str()) .into_response(), ), + DavResourceWithChildren::Item(DavItem { + content: DavContent::Inline { url, content_type }, + .. + }) => { + let blob = self.web.get_bytes(url).await?; + Ok(([(CONTENT_TYPE, content_type)], blob).into_response()) + } DavResourceWithChildren::Item(DavItem { content: DavContent::Missing, .. @@ -417,6 +430,8 @@ pub(crate) enum DavError { Dandi(#[from] DandiError), #[error("failed to fetch data from Zarr manifests")] ZarrMan(#[from] ZarrManError), + #[error("failed to fetch resource content for inline display")] + Fetch(#[from] HttpError), #[error( "latest version was requested for Dandiset {dandiset_id}, but it has not been published" )] @@ -433,6 +448,7 @@ impl DavError { match self { DavError::Dandi(e) => e.class(), DavError::ZarrMan(e) => e.class(), + DavError::Fetch(e) => e.class(), DavError::NoLatestVersion { .. } => ErrorClass::NotFound, DavError::Template(_) | DavError::Xml(_) => ErrorClass::Internal, } diff --git a/src/dav/types.rs b/src/dav/types.rs index 27b7a3e..d0422d1 100644 --- a/src/dav/types.rs +++ b/src/dav/types.rs @@ -1,7 +1,10 @@ use super::util::{format_creationdate, format_modifieddate, version_path, Href}; use super::xml::{PropValue, Property}; use super::VersionSpec; -use crate::consts::{DEFAULT_CONTENT_TYPE, YAML_CONTENT_TYPE}; +use crate::consts::{ + DEFAULT_CONTENT_TYPE, INLINE_TEXT_EXTENSIONS, JSON_CONTENT_TYPE, MAX_INLINE_SIZE, + PLAIN_TEXT_CONTENT_TYPE, YAML_CONTENT_TYPE, ZARR_METADATA_FILENAMES, +}; use crate::dandi::*; use crate::httputil::HttpUrl; use crate::paths::{PureDirPath, PurePath}; @@ -642,6 +645,41 @@ impl From for DavItem { } } +/// If the file at `path` is one that `dandidav` serves inline (so that it +/// displays in the browser rather than being downloaded), return the +/// `Content-Type` to serve it with; otherwise, return `None`. +/// +/// This covers JSON files (general `.json` files as well as Zarr v2 metadata +/// files, which have no extension) — served as `application/json` — YAML files +/// (`.yaml`/`.yml`) — served as `text/yaml`, matching the virtual +/// `dandiset.yaml` files — and a set of other text-based files (see +/// [`INLINE_TEXT_EXTENSIONS`]) served as `text/plain`. Callers must +/// additionally check that the file is no larger than +/// [`MAX_INLINE_SIZE`](crate::consts::MAX_INLINE_SIZE) before serving inline. +fn inline_content_type(path: &PurePath) -> Option<&'static str> { + let name = path.name_str(); + let ext = name.rsplit_once('.').map(|(_, ext)| ext); + // Zarr v2 metadata files (`.zgroup`, `.zarray`, `.zattrs`) are JSON but + // have no `.json` extension, so they are matched by name; `zarr.json` and + // all other `.json` files are matched by extension below. + if ZARR_METADATA_FILENAMES.contains(&name) + || ext.is_some_and(|ext| ext.eq_ignore_ascii_case("json")) + { + return Some(JSON_CONTENT_TYPE); + } + if ext.is_some_and(|ext| ext.eq_ignore_ascii_case("yaml") || ext.eq_ignore_ascii_case("yml")) { + return Some(YAML_CONTENT_TYPE); + } + if ext.is_some_and(|ext| { + INLINE_TEXT_EXTENSIONS + .iter() + .any(|e| ext.eq_ignore_ascii_case(e)) + }) { + return Some(PLAIN_TEXT_CONTENT_TYPE); + } + None +} + impl From for DavItem { fn from(blob: BlobAsset) -> DavItem { // Call methods before moving out `path` field: @@ -650,14 +688,32 @@ impl From for DavItem { .unwrap_or(DEFAULT_CONTENT_TYPE) .to_owned(); let etag = blob.etag().map(String::from); - let content = match (blob.archive_url(), blob.s3_url()) { - (Some(archive), Some(s3)) => DavContent::Redirect(Redirect::Alt { - s3: s3.clone(), - archive: archive.clone(), - }), - (Some(u), None) | (None, Some(u)) => DavContent::Redirect(Redirect::Direct(u.clone())), - // TODO: Log a warning when asset doesn't have a download URL? - (None, None) => DavContent::Missing, + // Small files of certain types are fetched and served inline (so they + // display in the browser) rather than redirected to S3. We fetch such + // files from S3 directly when possible. + let inline = inline_content_type(&blob.path) + .filter(|_| blob.size <= MAX_INLINE_SIZE) + .and_then(|ct| { + blob.s3_url() + .or_else(|| blob.archive_url()) + .map(|u| DavContent::Inline { + url: u.clone(), + content_type: ct, + }) + }); + let content = match inline { + Some(content) => content, + None => match (blob.archive_url(), blob.s3_url()) { + (Some(archive), Some(s3)) => DavContent::Redirect(Redirect::Alt { + s3: s3.clone(), + archive: archive.clone(), + }), + (Some(u), None) | (None, Some(u)) => { + DavContent::Redirect(Redirect::Direct(u.clone())) + } + // TODO: Log a warning when asset doesn't have a download URL? + (None, None) => DavContent::Missing, + }, }; DavItem { path: blob.path, @@ -675,6 +731,18 @@ impl From for DavItem { impl From for DavItem { fn from(entry: ZarrEntry) -> DavItem { + // Zarr metadata files are served inline as JSON (so they display in the + // browser) rather than redirected to S3; all other entries redirect. + // Note that we intentionally do not set `content_type` (i.e., the + // `getcontenttype` PROPFIND property) for Zarr entries, in keeping with + // the rest of the codebase. + let content = match inline_content_type(&entry.path) { + Some(content_type) if entry.size <= MAX_INLINE_SIZE => DavContent::Inline { + url: entry.url, + content_type, + }, + _ => DavContent::Redirect(Redirect::Direct(entry.url)), + }; DavItem { path: entry.zarr_path.to_dir_path().join(&entry.path), created: None, @@ -683,7 +751,7 @@ impl From for DavItem { size: Some(entry.size), etag: Some(entry.etag), kind: ResourceKind::ZarrEntry, - content: DavContent::Redirect(Redirect::Direct(entry.url)), + content, metadata_url: None, } } @@ -691,6 +759,15 @@ impl From for DavItem { impl From for DavItem { fn from(entry: ManifestEntry) -> DavItem { + // See the note in `From` regarding inline serving and + // `content_type`. + let content = match inline_content_type(&entry.web_path) { + Some(content_type) if entry.size <= MAX_INLINE_SIZE => DavContent::Inline { + url: entry.url, + content_type, + }, + _ => DavContent::Redirect(Redirect::Direct(entry.url)), + }; DavItem { path: entry.web_path, created: None, @@ -699,7 +776,7 @@ impl From for DavItem { size: Some(entry.size), etag: Some(entry.etag), kind: ResourceKind::ZarrEntry, - content: DavContent::Redirect(Redirect::Direct(entry.url)), + content, metadata_url: None, } } @@ -719,6 +796,17 @@ pub(super) enum DavContent { /// for the resource Redirect(Redirect), + /// A URL whose contents `dandidav` should fetch and serve inline (with the + /// given content type) in response to a `GET` request, rather than + /// redirecting. + /// + /// This is used for small text-based files (see [`inline_content_type`]) + /// so that they display in the browser instead of being downloaded. + Inline { + url: HttpUrl, + content_type: &'static str, + }, + /// No download URL could be determined for the resource Missing, } @@ -822,3 +910,38 @@ impl Serialize for ResourceKind { serializer.serialize_str(self.as_str()) } } + +#[cfg(test)] +mod tests { + use super::*; + use rstest::rstest; + + #[rstest] + // Zarr metadata is served as JSON: + #[case("zarr.json", Some(JSON_CONTENT_TYPE))] + #[case(".zgroup", Some(JSON_CONTENT_TYPE))] + #[case(".zarray", Some(JSON_CONTENT_TYPE))] + #[case(".zattrs", Some(JSON_CONTENT_TYPE))] + // General JSON files are served as JSON: + #[case("dataset_description.json", Some(JSON_CONTENT_TYPE))] + #[case("foo.JSON", Some(JSON_CONTENT_TYPE))] + // YAML files are served as `text/yaml`, matching the virtual + // `dandiset.yaml` files: + #[case("config.yaml", Some(YAML_CONTENT_TYPE))] + #[case("config.yml", Some(YAML_CONTENT_TYPE))] + #[case("config.YAML", Some(YAML_CONTENT_TYPE))] + // Other text-based files are served as plain text: + #[case("participants.tsv", Some(PLAIN_TEXT_CONTENT_TYPE))] + #[case("data.csv", Some(PLAIN_TEXT_CONTENT_TYPE))] + #[case("README.md", Some(PLAIN_TEXT_CONTENT_TYPE))] + #[case("notes.txt", Some(PLAIN_TEXT_CONTENT_TYPE))] + #[case(".bidsignore", Some(PLAIN_TEXT_CONTENT_TYPE))] + #[case("README.MD", Some(PLAIN_TEXT_CONTENT_TYPE))] + // Everything else is not served inline: + #[case("data.nwb", None)] + #[case("image.png", None)] + #[case("noextension", None)] + fn test_inline_content_type(#[case] name: PurePath, #[case] expected: Option<&'static str>) { + assert_eq!(inline_content_type(&name), expected); + } +} diff --git a/src/httputil.rs b/src/httputil.rs index 4f1bfd2..2fcfebc 100644 --- a/src/httputil.rs +++ b/src/httputil.rs @@ -90,6 +90,29 @@ impl Client { self.request(Method::GET, url).await } + /// Perform a `GET` request to the given URL and return the response body + /// as raw bytes + /// + /// # Errors + /// + /// If sending the request fails, the response has a 4xx or 5xx status, or + /// reading the response body fails, an error is returned. + pub(crate) fn get_bytes( + &self, + url: HttpUrl, + ) -> impl Future, HttpError>> { + let client = self.clone(); + async move { + client + .get(url.clone()) + .await? + .bytes() + .await + .map(|b| b.to_vec()) + .map_err(move |source| HttpError::Read { url, source }) + } + } + /// Perform a `GET` request to the given URL and deserialize the response /// body as JSON into `T` /// @@ -178,6 +201,13 @@ pub(crate) enum HttpError { url: HttpUrl, source: reqwest::Error, }, + + /// Reading the response body failed + #[error("failed to read response body from {url}")] + Read { + url: HttpUrl, + source: reqwest::Error, + }, } impl HttpError { @@ -189,6 +219,7 @@ impl HttpError { HttpError::Deserialize { source, .. } if source.is_timeout() => { ErrorClass::GatewayTimeout } + HttpError::Read { source, .. } if source.is_timeout() => ErrorClass::GatewayTimeout, _ => ErrorClass::BadGateway, } } diff --git a/src/main.rs b/src/main.rs index e11875d..0e3bb37 100644 --- a/src/main.rs +++ b/src/main.rs @@ -13,7 +13,7 @@ mod zarrman; use crate::consts::*; use crate::dandi::DandiClient; use crate::dav::{DandiDav, Templater}; -use crate::httputil::HttpUrl; +use crate::httputil::{Client, HttpUrl}; use crate::zarrman::{ManifestFetcher, ZarrManClient}; use anyhow::Context; use axum::{ @@ -166,10 +166,12 @@ fn get_app(cfg: Config) -> anyhow::Result { zarrfetcher.install_periodic_dump(ZARR_MANIFEST_CACHE_DUMP_PERIOD); let zarrman = ZarrManClient::new(zarrfetcher); let templater = Templater::new(cfg.title)?; + let web = Client::new()?; let dav = Arc::new(DandiDav { dandi, zarrman, templater, + web, prefer_s3_redirects: cfg.prefer_s3_redirects, }); let mut app = Router::new() diff --git a/src/tests.rs b/src/tests.rs index f61d705..0d2fe4a 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -728,7 +728,7 @@ async fn get_version_toplevel() { CollectionEntry { name: Link { text: "JEhE.tsv".into(), - href: "https://api.dandiarchive.org/api/assets/faeb54dc-f906-40fa-b48e-28816c422ec5/download/".into() + href: "/dandisets/000002/draft/JEhE.tsv".into() }, metadata_link: Some(format!( "{}/dandisets/000002/versions/draft/assets/faeb54dc-f906-40fa-b48e-28816c422ec5/", @@ -789,7 +789,7 @@ async fn get_version_toplevel() { CollectionEntry { name: Link { text: "nPjB.json".into(), - href: "https://api.dandiarchive.org/api/assets/08938c9b-b248-4aa0-b963-859029a1f38b/download/".into() + href: "/dandisets/000002/draft/nPjB.json".into() }, metadata_link: Some(format!( "{}/dandisets/000002/versions/draft/assets/08938c9b-b248-4aa0-b963-859029a1f38b/", @@ -803,7 +803,7 @@ async fn get_version_toplevel() { CollectionEntry { name: Link { text: "ykBgN.tsv".into(), - href: "https://api.dandiarchive.org/api/assets/3a4bc5da-4920-467b-9a82-4e1a9cac90b7/download/".into() + href: "/dandisets/000002/draft/ykBgN.tsv".into() }, metadata_link: Some(format!( "{}/dandisets/000002/versions/draft/assets/3a4bc5da-4920-467b-9a82-4e1a9cac90b7/", @@ -1338,7 +1338,7 @@ async fn get_latest_version() { CollectionEntry { name: Link { text: "PYQIm.tsv".into(), - href: "https://api.dandiarchive.org/api/assets/34523ca7-ff7c-42b3-a311-2f2d0ccc780f/download/".into() + href: "/dandisets/000001/latest/PYQIm.tsv".into() }, metadata_link: Some(format!( "{}/dandisets/000001/versions/0.230629.1955/assets/34523ca7-ff7c-42b3-a311-2f2d0ccc780f/",