diff --git a/Cargo.lock b/Cargo.lock index 7574e8aa..b3c9c815 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2944,6 +2944,7 @@ dependencies = [ "tower", "tower-http 0.7.0", "tracing-subscriber", + "url", ] [[package]] diff --git a/clients/CHANGELOG.md b/clients/CHANGELOG.md index 8f0c8009..d30f26f9 100644 --- a/clients/CHANGELOG.md +++ b/clients/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## Unreleased + +### New Features ✨ + +- (Python) Add a content disposition option to object URLs and pre-signed URLs (FS-570). + ## 0.2.10 ### New Features ✨ diff --git a/clients/python/README.md b/clients/python/README.md index 924eb82a..4254eff4 100644 --- a/clients/python/README.md +++ b/clients/python/README.md @@ -254,6 +254,22 @@ with urllib.request.urlopen(url) as resp: content = resp.read() ``` +### Download response options + +Both `Session.object_url` and `Session.presigned_object_url` accept a +`response_content_disposition` option: + +```python +url = session.presigned_object_url( + "GET", + "my-key", + response_content_disposition="inline", +) +``` + +Disposition accepts `inline` or `attachment`, defaulting to `attachment` when omitted. +The stored filename is retained. + ## Configuration In production, store the `Client` and `Usecase` at module level and reuse them. diff --git a/clients/python/src/objectstore_client/client.py b/clients/python/src/objectstore_client/client.py index d4f82b3f..34b04530 100644 --- a/clients/python/src/objectstore_client/client.py +++ b/clients/python/src/objectstore_client/client.py @@ -52,6 +52,15 @@ USER_AGENT = f"objectstore-client/{version('objectstore-client')}" +def _response_options_query( + disposition: Literal["inline", "attachment"], +) -> str: + """Validate and encode the response-content-disposition query parameter.""" + if disposition not in ("inline", "attachment"): + raise ValueError(f"unsupported response content disposition {disposition!r}") + return utils.encode_query(f"response-content-disposition={disposition}") + + class GetResponse(NamedTuple): metadata: Metadata payload: IO[bytes] @@ -94,7 +103,7 @@ def __init__( "usecase name must contain only ASCII letters, digits, '_' or '-'" ) self._name = name - + if resumable_threshold_bytes != "none" and ( type(resumable_threshold_bytes) is not int or resumable_threshold_bytes < 0 ): @@ -709,7 +718,12 @@ def get( span.set_attribute("objectstore.decompressed", decompressed) return GetResponse(metadata, stream) - def object_url(self, key: str, token_validity: timedelta | None = None) -> str: + def object_url( + self, + key: str, + token_validity: timedelta | None = None, + response_content_disposition: Literal["inline", "attachment"] | None = None, + ) -> str: """ Generates a GET url to the object with the given `key`. @@ -721,10 +735,18 @@ def object_url(self, key: str, token_validity: timedelta | None = None) -> str: information is embedded in the returned URL's query string, valid for the given duration. + ``response_content_disposition`` selects ``attachment`` (default) or ``inline`` + for successful GET/HEAD responses, retaining the stored filename. + + These options are not bound by the auth token and can be changed by recipients. + Use :meth:`presigned_object_url` to bind the options to a signature. + Raises ``ValueError`` if ``token_validity`` is provided but no - ``SecretKey`` is configured on this session. + ``SecretKey`` is configured on this session, or if disposition is invalid. """ url = self._make_url(key, full=True) + if response_content_disposition is not None: + url = f"{url}?{_response_options_query(response_content_disposition)}" if token_validity is None: return url @@ -734,13 +756,15 @@ def object_url(self, key: str, token_validity: timedelta | None = None) -> str: ) # A JWT's compact serialization is base64url, whose alphabet is URL-safe, # so it can go in the query string as-is with no further encoding. - return f"{url}?{PARAM_AUTH}={token}" + separator = "&" if response_content_disposition is not None else "?" + return f"{url}{separator}{PARAM_AUTH}={token}" def presigned_object_url( self, method: Literal["GET", "HEAD"], key: str, duration: timedelta = timedelta(hours=1), + response_content_disposition: Literal["inline", "attachment"] | None = None, ) -> str: """ Generates a pre-signed URL authorizing a single ``method`` request on the @@ -752,12 +776,17 @@ def presigned_object_url( Raises ``ValueError`` if no ``SecretKey`` is configured on this session's client, if ``method`` is not supported, or if ``duration`` - is above the one-week maximum. + is above the one-week maximum, or if disposition is invalid. The returned URL carries a signature that allows the recipient to perform a request without an auth token. It can be handed to any HTTP client; the URL is already percent-encoded, and must be transmitted verbatim. + ``response_content_disposition`` selects ``attachment`` (default) or ``inline`` + for successful GET/HEAD responses, retaining the stored filename. + + Response options are included in the signature when supplied. + Note that `HEAD` and `GET` are considered equivalent from the point of view of signatures, meaning that a signature for `GET` can also be used for `HEAD`, and viceversa. @@ -795,6 +824,11 @@ def presigned_object_url( f"&{presign.PARAM_DURATION}={duration_secs}" ) + if response_content_disposition is not None: + encoded_query += ( + f"&{_response_options_query(response_content_disposition)}" + ) + canonical = presign.build_canonical_form( method, encoded_path, encoded_query ) diff --git a/clients/python/tests/test_e2e.py b/clients/python/tests/test_e2e.py index 1f08b373..309d1d35 100644 --- a/clients/python/tests/test_e2e.py +++ b/clients/python/tests/test_e2e.py @@ -1094,6 +1094,65 @@ def test_presigned_get_encoding_corner_cases(server_url: str, key: str) -> None: assert body == payload +def test_presigned_response_options(server_url: str) -> None: + session = _presign_session(server_url) + key = "inline-example" + session.put(b"example", key=key, content_type="text/html") + url = session.presigned_object_url( + "GET", + key, + response_content_disposition="inline", + ) + with urllib.request.urlopen(url) as response: + assert response.status == 200 + assert response.headers["content-disposition"] == "inline" + assert response.headers["content-type"] == "text/html" + assert response.headers["content-security-policy"] == "sandbox" + assert response.headers["x-content-type-options"] == "nosniff" + assert response.read() == b"example" + + status, _ = _fetch(url.replace("=inline", "=attachment")) + assert status == 401 + + +def test_jwt_response_options_remain_editable(server_url: str) -> None: + session = _presign_session(server_url) + session.put(b"example", key="jwt-options") + url = session.object_url( + "jwt-options", + token_validity=timedelta(minutes=5), + response_content_disposition="inline", + ) + for target, expected in [ + (url, "inline"), + (url.replace("=inline", "=attachment"), "attachment"), + ]: + with urllib.request.urlopen(target) as response: + assert response.status == 200 + assert response.headers["content-disposition"] == expected + + +def test_inline_preserves_stored_mime_type_and_compression(server_url: str) -> None: + session = Client(server_url, token=TestSecretKey.get()).session( + Usecase("test-usecase", compression="zstd"), org=42, project=1337 + ) + payload = b'' + session.put(payload, key="svg-options", content_type="image/svg+xml") + url = session.object_url( + "svg-options", + token_validity=timedelta(minutes=5), + response_content_disposition="inline", + ) + with urllib.request.urlopen(url) as response: + assert response.headers["content-type"] == "image/svg+xml" + assert response.headers["content-encoding"] == "zstd" + assert response.headers["content-security-policy"] == "sandbox" + with zstandard.ZstdDecompressor().stream_reader( + BytesIO(response.read()) + ) as reader: + assert reader.read() == payload + + def test_object_url_read_only_token_succeeds(server_url: str) -> None: session = _presign_session(server_url) session.put(b"read only hello", key="read-only-url") diff --git a/clients/python/tests/test_presign.py b/clients/python/tests/test_presign.py index 8a2fd20e..c5b3cbae 100644 --- a/clients/python/tests/test_presign.py +++ b/clients/python/tests/test_presign.py @@ -2,10 +2,13 @@ import base64 import os +from datetime import UTC, datetime, timedelta +from unittest.mock import patch +from urllib.parse import parse_qs, urlsplit from cryptography.hazmat.primitives.asymmetric.ed25519 import Ed25519PublicKey from cryptography.hazmat.primitives.serialization import load_pem_public_key -from objectstore_client import presign, utils +from objectstore_client import Client, Usecase, presign, utils from objectstore_client.auth import SecretKey TEST_EDDSA_PRIVKEY_PATH = os.path.join(os.path.dirname(__file__), "ed25519.private.pem") @@ -73,3 +76,52 @@ def test_sign_canonical_form_roundtrips_with_public_key() -> None: signature = base64.urlsafe_b64decode(signature_b64 + padding) assert len(signature) == 64 public_key.verify(signature, canonical.encode()) # raises on mismatch + + +def test_presigned_url_response_options() -> None: + with open(TEST_EDDSA_PRIVKEY_PATH) as f: + secret = SecretKey("test_kid", f.read()) + session = Client("https://example.com/prefix", token=secret).session( + Usecase("testing"), org=1 + ) + with patch("objectstore_client.client.datetime") as clock: + clock.now.return_value = datetime(2026, 4, 20, 13, 37, tzinfo=UTC) + legacy = session.presigned_object_url("GET", "key") + explicit_none = session.presigned_object_url( + "GET", "key", response_content_disposition=None + ) + url = session.presigned_object_url( + "GET", + "key", + response_content_disposition="inline", + ) + assert legacy == explicit_none + assert "response-content-disposition" not in parse_qs(urlsplit(legacy).query) + parsed = urlsplit(url) + assert parsed.path == urlsplit(legacy).path + query = parse_qs(parsed.query) + assert query["response-content-disposition"] == ["inline"] + assert len(query) == 5 + + +def test_jwt_url_options_roundtrip_without_binding_token() -> None: + with open(TEST_EDDSA_PRIVKEY_PATH) as f: + secret = SecretKey("test_kid", f.read()) + session = Client("https://example.com/prefix", token=secret).session( + Usecase("testing") + ) + with patch.object(session, "mint_token", return_value="same.jwt.token"): + legacy = session.object_url("key", token_validity=timedelta(minutes=5)) + url = session.object_url( + "key", + token_validity=timedelta(minutes=5), + response_content_disposition="inline", + ) + assert parse_qs(urlsplit(url).query) == { + "os_auth": ["same.jwt.token"], + "response-content-disposition": ["inline"], + } + assert legacy.endswith("?os_auth=same.jwt.token") + assert session.object_url( + "key", response_content_disposition=None + ) == session.object_url("key") diff --git a/objectstore-server/Cargo.toml b/objectstore-server/Cargo.toml index ee1cc168..d7d68ba8 100644 --- a/objectstore-server/Cargo.toml +++ b/objectstore-server/Cargo.toml @@ -50,6 +50,7 @@ thread_local = { workspace = true } tokio = { workspace = true, features = ["fs", "macros", "net", "rt-multi-thread", "signal", "sync", "time"] } tower = { workspace = true } tower-http = { workspace = true, features = ["catch-panic", "metrics", "set-header", "trace"] } +url = { workspace = true } [target.'cfg(target_os = "linux")'.dependencies] jemalloc_pprof = { workspace = true, optional = true } diff --git a/objectstore-server/src/endpoints/mod.rs b/objectstore-server/src/endpoints/mod.rs index a501bb46..a661dd82 100644 --- a/objectstore-server/src/endpoints/mod.rs +++ b/objectstore-server/src/endpoints/mod.rs @@ -23,6 +23,11 @@ //! Object metadata travels in request and response headers; see //! [`objectstore_types::metadata`] for the mapping and the separate JSON update contract. //! +//! GET and HEAD accept `response-content-disposition=inline|attachment` (default attachment). +//! Response options apply only to successful object responses, including ranged GETs. See +//! [`crate::extractors::response_options`] for validation. The object handlers construct +//! download headers with browser safeguards and filename formatting. +//! //! # Resumable Upload Endpoints //! //! A resumable upload transfers a single object across several requests. diff --git a/objectstore-server/src/endpoints/objects.rs b/objectstore-server/src/endpoints/objects.rs index cdc3f28a..0013ad9f 100644 --- a/objectstore-server/src/endpoints/objects.rs +++ b/objectstore-server/src/endpoints/objects.rs @@ -25,19 +25,19 @@ //! strings are rejected by JSON deserialization with 422. Requests exceeding the //! usecase's `expiration.max`, measured from request time, return 400. -use std::fmt::Write as _; +use std::convert::Infallible; use axum::body::Body; use axum::extract::{Request, State}; use axum::handler::Handler; use axum::http::{HeaderMap, StatusCode}; -use axum::response::{IntoResponse, Response}; +use axum::response::{IntoResponse, IntoResponseParts, Response, ResponseParts}; use axum::routing; use axum::{Json, Router}; +use http::{HeaderValue, header}; use objectstore_service::backend::common::{ExpiryTarget, ExpiryUpdate, SetExpiryResponse}; use objectstore_service::error::ErrorKind; use objectstore_service::id::{ObjectContext, ObjectId}; -use objectstore_types::headers::ExtValue; use objectstore_types::metadata::{ExpiryAnchor, ExpiryExtension, Metadata, MetadataUpdate}; use objectstore_types::range::ContentRange; use serde::Serialize; @@ -47,6 +47,7 @@ use crate::endpoints::common::{ApiError, ApiResult, insert_accept_ranges}; use crate::endpoints::resumable; use crate::extractors::byte_range::OptionalByteRange; use crate::extractors::request_time::RequestTime; +use crate::extractors::response_options::{ContentDisposition, ResponseOptions}; use crate::extractors::{Xt, body::MeteredBody}; use crate::resumable::ResumableTarget; use crate::state::ServiceState; @@ -176,7 +177,7 @@ async fn object_get( service: AuthAwareService, State(state): State, Xt(id): Xt, - _headers: HeaderMap, + options: ResponseOptions, RequestTime(access_time): RequestTime, OptionalByteRange(byte_range): OptionalByteRange, ) -> ApiResult { @@ -230,14 +231,15 @@ async fn object_get( } }; - insert_content_disposition(&mut response, &metadata); insert_accept_ranges(&mut response); - Ok(response) + let download_headers = DownloadHeaders::new(options, metadata.filename.as_deref()); + Ok((download_headers, response).into_response()) } async fn object_head( service: AuthAwareService, + options: ResponseOptions, Xt(id): Xt, RequestTime(access_time): RequestTime, ) -> ApiResult { @@ -251,9 +253,9 @@ async fn object_head( insert_content_length(&mut headers, &metadata); let mut response = (StatusCode::OK, headers).into_response(); - insert_content_disposition(&mut response, &metadata); insert_accept_ranges(&mut response); - Ok(response) + let download_headers = DownloadHeaders::new(options, metadata.filename.as_deref()); + Ok((download_headers, response).into_response()) } /// Inserts a `Content-Length` header covering the complete object. @@ -270,55 +272,6 @@ fn insert_content_length(headers: &mut HeaderMap, metadata: &Metadata) { } } -/// Serves every object as an attachment, named after its filename when it has one. -fn insert_content_disposition(response: &mut Response, metadata: &Metadata) { - let value = match metadata.filename.as_deref() { - Some(filename) => format_content_disposition(filename), - None => http::HeaderValue::from_static("attachment"), - }; - response - .headers_mut() - .insert(http::header::CONTENT_DISPOSITION, value); -} - -/// Formats a `Content-Disposition: attachment; filename="..."` header value. -/// -/// The filename is sanitized (`/` and `\` become `-`, dots-only names become all dashes, non-ASCII -/// and control characters become `_`) and then escaped for the RFC 6266 quoted-string (`"` is -/// backslash-escaped). -/// -/// A filename that is not pure ASCII cannot be represented in that quoted-string, so it -/// additionally gets an RFC 8187 `filename*` parameter carrying the full UTF-8 value. The -/// quoted-string then serves as the ASCII fallback for clients that ignore `filename*`. -fn format_content_disposition(filename: &str) -> http::HeaderValue { - let all_dots = filename.chars().all(|c| c == '.'); - - let mut result = String::from("attachment; filename=\""); - for c in filename.chars() { - let c = match c { - '/' | '\\' => '-', - '.' if all_dots => '-', - '"' => { - result.push('\\'); - '"' - } - c if !c.is_ascii() || c.is_control() => '_', - c => c, - }; - result.push(c); - } - result.push('"'); - - if !filename.is_ascii() { - write!(result, "; filename*={}", ExtValue(filename)) - .expect("writing to a string cannot fail"); - } - - // INVARIANT: every character written above is visible ASCII — the quoted-string replaces - // non-ASCII and control characters with `_`, and the `ext-value` is percent-encoded. - http::HeaderValue::from_str(&result).expect("content disposition is a valid header value") -} - async fn insert_object( service: AuthAwareService, State(state): State, @@ -356,6 +309,61 @@ async fn delete_object( Ok(StatusCode::NO_CONTENT) } +/// Object presentation headers, including mandatory browser safeguards. +/// +/// All successful downloads get `nosniff` and `no-referrer`; inline responses also get a CSP +/// sandbox without relaxation tokens. Sandboxing is a browser safeguard, not content sanitization, +/// and does not prevent all external resource loads. Apply these parts only to successful responses. +#[derive(Debug)] +struct DownloadHeaders { + disposition: HeaderValue, + inline: bool, +} + +impl DownloadHeaders { + /// Builds success-only headers using the response options and stored filename. + fn new(options: ResponseOptions, filename: Option<&str>) -> Self { + let header = match filename { + Some(filename) => options.disposition.with_filename(filename), + None => HeaderValue::from_static(options.disposition.as_str()), + }; + + Self { + disposition: header, + inline: options.disposition == ContentDisposition::Inline, + } + } +} + +impl IntoResponseParts for DownloadHeaders { + type Error = Infallible; + + fn into_response_parts( + self, + mut response: ResponseParts, + ) -> Result { + let headers = response.headers_mut(); + headers.insert(header::CONTENT_DISPOSITION, self.disposition); + headers.insert( + header::X_CONTENT_TYPE_OPTIONS, + HeaderValue::from_static("nosniff"), + ); + // Inline documents can load external resources; avoid exposing URL query credentials + // through referrers as defense in depth alongside the sandbox. + headers.insert( + header::REFERRER_POLICY, + HeaderValue::from_static("no-referrer"), + ); + if self.inline { + headers.insert( + header::CONTENT_SECURITY_POLICY, + HeaderValue::from_static("sandbox"), + ); + } + Ok(response) + } +} + #[cfg(test)] mod tests { use std::sync::Arc; @@ -618,4 +626,20 @@ mod tests { assert!(matches!(error, ApiError::Service(_))); assert_eq!(error.status(), StatusCode::TOO_MANY_REQUESTS); } + + #[test] + fn inline_filename_retains_escaping_and_unicode_encoding() { + let options = ResponseOptions { + disposition: ContentDisposition::Inline, + }; + let download_headers = DownloadHeaders::new(options, Some("réport.pdf")); + let response = (download_headers, ()).into_response(); + assert_eq!( + response.headers()["content-disposition"], + "inline; filename=\"r_port.pdf\"; filename*=UTF-8''r%C3%A9port.pdf" + ); + assert_eq!(response.headers()["content-security-policy"], "sandbox"); + assert_eq!(response.headers()["x-content-type-options"], "nosniff"); + assert_eq!(response.headers()["referrer-policy"], "no-referrer"); + } } diff --git a/objectstore-server/src/extractors/mod.rs b/objectstore-server/src/extractors/mod.rs index 8a2de118..8e92a68a 100644 --- a/objectstore-server/src/extractors/mod.rs +++ b/objectstore-server/src/extractors/mod.rs @@ -6,6 +6,7 @@ pub mod byte_range; pub mod downstream_service; mod id; pub mod request_time; +pub mod response_options; mod service; /// An extractor for a remote type. diff --git a/objectstore-server/src/extractors/response_options.rs b/objectstore-server/src/extractors/response_options.rs new file mode 100644 index 00000000..ba551cb3 --- /dev/null +++ b/objectstore-server/src/extractors/response_options.rs @@ -0,0 +1,143 @@ +//! Presentation options for successful individual-object downloads. +//! +//! The request extractor validates these options without modifying the signed URI. Handlers +//! attach their typed headers only to successful GET/HEAD responses, including partial content. +//! Error responses retain their existing headers. +//! +//! # Query options +//! +//! - `response-content-disposition`: exactly `inline` or `attachment` (the default). The stored +//! filename is retained and formatted using the existing download filename rules. +//! +//! Parameter names are case-insensitive, matching pre-signed URL canonicalization. Duplicate +//! supported options and invalid values are rejected with 400. Unrelated parameters are ignored +//! by this extractor and remain covered by the existing signature scheme. +//! +//! Content type and content encoding always come from stored metadata. Content-language and +//! expires overrides are not supported. No MIME allowlist is applied to inline responses. + +use std::fmt::Write as _; + +use axum::extract::FromRequestParts; +use http::HeaderValue; +use http::request::Parts; +use objectstore_types::headers::ExtValue; +use serde::Deserialize; +use serde::de::value::{Error, MapDeserializer}; +use url::form_urlencoded; + +use crate::endpoints::common::ApiError; + +/// Whether a browser should download an object or display it inline. +#[derive(Debug, Default, Clone, Copy, PartialEq, Eq, Deserialize)] +#[serde(rename_all = "lowercase")] +pub enum ContentDisposition { + /// Download the object (the default). + #[default] + Attachment, + /// Display the object with browser sandboxing enabled. + Inline, +} + +impl ContentDisposition { + /// Returns the disposition's HTTP header value. + pub fn as_str(self) -> &'static str { + match self { + Self::Attachment => "attachment", + Self::Inline => "inline", + } + } + + /// Formats the selected disposition with a stored filename. + /// + /// The filename is sanitized (`/` and `\` become `-`, dots-only names become all dashes, non-ASCII + /// and control characters become `_`) and then escaped for the RFC 6266 quoted-string (`"` is + /// backslash-escaped). + /// + /// A filename that is not pure ASCII cannot be represented in that quoted-string, so it + /// additionally gets an RFC 8187 `filename*` parameter carrying the full UTF-8 value. The + /// quoted-string then serves as the ASCII fallback for clients that ignore `filename*`. + pub fn with_filename(self, filename: &str) -> HeaderValue { + let all_dots = filename.chars().all(|c| c == '.'); + + let mut result = format!("{}; filename=\"", self.as_str()); + for c in filename.chars() { + let c = match c { + '/' | '\\' => '-', + '.' if all_dots => '-', + '"' => { + result.push('\\'); + '"' + } + c if !c.is_ascii() || c.is_control() => '_', + c => c, + }; + result.push(c); + } + result.push('"'); + + if !filename.is_ascii() { + write!(result, "; filename*={}", ExtValue(filename)) + .expect("writing to a string cannot fail"); + } + + // INVARIANT: every character written above is visible ASCII — the quoted-string replaces + // non-ASCII and control characters with `_`, and the `ext-value` is percent-encoded. + HeaderValue::from_str(&result).expect("content disposition is a valid header value") + } +} + +/// Validated presentation options for successful object downloads. +/// +/// Disposition defaults to attachment. Stored MIME type and encoding are never overridden, +/// and inline content is not restricted by a MIME allowlist. +#[derive(Debug, Default, Deserialize)] +pub struct ResponseOptions { + /// Selected browser disposition, defaulting to attachment. + #[serde(default, rename = "response-content-disposition")] + pub disposition: ContentDisposition, +} + +impl FromRequestParts for ResponseOptions { + type Rejection = ApiError; + + async fn from_request_parts(parts: &mut Parts, _state: &S) -> Result { + // Query matches names case-sensitively. Normalize them to match + // pre-signed URL canonicalization without modifying the original URI. + let pairs = form_urlencoded::parse(parts.uri.query().unwrap_or_default().as_bytes()) + .map(|(name, value)| (name.to_ascii_lowercase(), value.into_owned())); + + Self::deserialize(MapDeserializer::<_, Error>::new(pairs)) + .map_err(|error| ApiError::map_client("invalid response options", error)) + } +} + +#[cfg(test)] +mod tests { + use axum::extract::FromRequestParts; + use http::Request; + + use crate::extractors::response_options::{ContentDisposition, ResponseOptions}; + + #[tokio::test] + async fn decodes_options_without_changing_signed_uri() { + let uri = "/object?unknown=b&unknown=a&%72esponse-content-disposition=inline"; + let (mut parts, ()) = Request::builder().uri(uri).body(()).unwrap().into_parts(); + let options = ResponseOptions::from_request_parts(&mut parts, &()) + .await + .unwrap(); + assert_eq!(options.disposition, ContentDisposition::Inline); + assert_eq!(parts.uri.to_string(), uri); + } + + #[tokio::test] + async fn defaults_preserve_unrelated_parameters() { + for uri in ["/object", "/object?unknown=b&unknown=a&flag&&"] { + let (mut parts, ()) = Request::builder().uri(uri).body(()).unwrap().into_parts(); + let options = ResponseOptions::from_request_parts(&mut parts, &()) + .await + .unwrap(); + assert_eq!(options.disposition, ContentDisposition::Attachment); + } + } +} diff --git a/objectstore-server/tests/objects.rs b/objectstore-server/tests/objects.rs index 53503c58..c25b8000 100644 --- a/objectstore-server/tests/objects.rs +++ b/objectstore-server/tests/objects.rs @@ -1,9 +1,11 @@ use std::time::Duration; use anyhow::Result; +use http::{Method, StatusCode}; use objectstore_server::config::{AuthZ, Config}; use objectstore_test::server::TestServer; use objectstore_types::metadata::{ExpirationPolicy, Metadata}; +use reqwest::Url; async fn test_server() -> TestServer { TestServer::with_config(Config { @@ -343,3 +345,146 @@ async fn filename_dot_and_dotdot_are_sanitized() -> Result<()> { Ok(()) } + +#[tokio::test] +async fn successful_downloads_apply_options_and_preserve_metadata() -> Result<()> { + let server = test_server().await; + let client = reqwest::Client::new(); + let path = "/v1/objects/test/org=1/options"; + client + .put(server.url(path)) + .header("content-type", "text/html") + .header("x-sn-filename", "report.html") + .body("example") + .send() + .await? + .error_for_status()?; + + for method in [Method::GET, Method::HEAD] { + for disposition in [None, Some("attachment"), Some("inline")] { + let mut url = Url::parse(&server.url(path))?; + if let Some(disposition) = disposition { + url.query_pairs_mut() + .append_pair("response-content-disposition", disposition); + } + let response = client.request(method.clone(), url).send().await?; + assert_eq!(response.status(), StatusCode::OK); + assert_eq!(response.headers()["content-type"], "text/html"); + assert_eq!(response.headers()["x-sn-filename"], "report.html"); + assert_eq!(response.headers()["content-length"], "20"); + assert_eq!( + response.headers()["content-disposition"], + format!( + "{}; filename=\"report.html\"", + disposition.unwrap_or("attachment") + ) + ); + assert_eq!(response.headers()["x-content-type-options"], "nosniff"); + assert_eq!(response.headers()["referrer-policy"], "no-referrer"); + assert_eq!( + response + .headers() + .get("content-security-policy") + .map(|v| v.to_str().unwrap()), + (disposition == Some("inline")).then_some("sandbox") + ); + let body = response.text().await?; + assert_eq!( + body, + if method == Method::HEAD { + "" + } else { + "example" + } + ); + } + } + Ok(()) +} + +#[tokio::test] +async fn ranged_and_error_responses() -> Result<()> { + let server = test_server().await; + let client = reqwest::Client::new(); + let path = "/v1/objects/test/org=1/range-options"; + client + .put(server.url(path)) + .body("abcdef") + .send() + .await? + .error_for_status()?; + let mut url = Url::parse(&server.url(path))?; + url.set_query(Some("response-content-disposition=inline")); + let response = client + .get(url.clone()) + .header("range", "bytes=1-3") + .send() + .await?; + assert_eq!(response.status(), StatusCode::PARTIAL_CONTENT); + assert_eq!(response.headers()["content-range"], "bytes 1-3/6"); + assert_eq!(response.headers()["content-length"], "3"); + assert_eq!(response.headers()["content-disposition"], "inline"); + assert_eq!(response.headers()["content-security-policy"], "sandbox"); + assert_eq!(response.headers()["referrer-policy"], "no-referrer"); + assert_eq!(response.text().await?, "bcd"); + + for (target, range, expected) in [ + (url.clone(), "bytes=99-", StatusCode::RANGE_NOT_SATISFIABLE), + ( + { + let mut missing = url.clone(); + missing.set_path("/v1/objects/test/org=1/missing-options"); + missing + }, + "bytes=0-", + StatusCode::NOT_FOUND, + ), + ] { + let response = client.get(target).header("range", range).send().await?; + assert_eq!(response.status(), expected); + for name in [ + "content-disposition", + "x-content-type-options", + "content-security-policy", + "referrer-policy", + ] { + assert!( + !response.headers().contains_key(name), + "unexpected {name} on {expected}" + ); + } + } + url.set_path("/v1/objects/test/org=1/missing-options"); + let response = client.head(url).send().await?; + assert_eq!(response.status(), StatusCode::NOT_FOUND); + for name in [ + "content-disposition", + "x-content-type-options", + "content-security-policy", + "referrer-policy", + ] { + assert!(!response.headers().contains_key(name)); + } + + Ok(()) +} + +#[tokio::test] +async fn invalid_and_duplicate_options_are_rejected() -> Result<()> { + let server = test_server().await; + let client = reqwest::Client::new(); + for query in [ + "response-content-disposition=other", + "response-content-disposition=inline%3Bfilename%3Dx", + "response-content-disposition=inline&RESPONSE-CONTENT-DISPOSITION=attachment", + ] { + let mut url = Url::parse(&server.url("/v1/objects/test/org=1/invalid-options"))?; + url.set_query(Some(query)); + assert_eq!( + client.get(url).send().await?.status(), + StatusCode::BAD_REQUEST, + "{query}" + ); + } + Ok(()) +} diff --git a/objectstore-server/tests/presigned.rs b/objectstore-server/tests/presigned.rs index 81e920f6..fc01edc6 100644 --- a/objectstore-server/tests/presigned.rs +++ b/objectstore-server/tests/presigned.rs @@ -148,6 +148,56 @@ async fn presigned_head_succeeds() -> Result<()> { Ok(()) } +#[tokio::test] +async fn response_options_are_signed_and_case_insensitive() -> Result<()> { + let server = test_server().await; + seed_object(&server, "hello").await?; + let legacy_query = presign_query(&Method::GET, OBJECT_PATH, ONE_HOUR); + let (base, _) = legacy_query.rsplit_once("&os_sig=").unwrap(); + let query = format!("{base}&response-content-disposition=inline&unknown=b&unknown=a"); + let signature = CanonicalRequest::new(&Method::GET, OBJECT_PATH, Some(&query)) + .sign(signing_key().as_bytes()); + let signed = format!("{query}&os_sig={signature}"); + let client = reqwest::Client::new(); + for method in [Method::GET, Method::HEAD] { + for query in [ + signed.clone(), + signed.replace( + "response-content-disposition", + "RESPONSE-CONTENT-DISPOSITION", + ), + ] { + let response = client + .request(method.clone(), presigned_url(&server, OBJECT_PATH, &query)) + .send() + .await?; + assert_eq!(response.status(), reqwest::StatusCode::OK); + assert_eq!(response.headers()["content-disposition"], "inline"); + assert_eq!(response.headers()["content-security-policy"], "sandbox"); + } + } + for query in [ + signed.replace("=inline", "=attachment"), + signed.replace("&response-content-disposition=inline", ""), + signed.replace("&unknown=b", ""), + format!("{legacy_query}&response-content-disposition=inline"), + ] { + let response = client + .get(presigned_url(&server, OBJECT_PATH, &query)) + .send() + .await?; + assert_eq!(response.status(), reqwest::StatusCode::UNAUTHORIZED); + for name in [ + "content-disposition", + "x-content-type-options", + "content-security-policy", + ] { + assert!(!response.headers().contains_key(name)); + } + } + Ok(()) +} + #[tokio::test] async fn presigned_delete_is_rejected() -> Result<()> { let server = test_server().await; diff --git a/objectstore-types/src/metadata.rs b/objectstore-types/src/metadata.rs index 8d1c6160..f2515ae8 100644 --- a/objectstore-types/src/metadata.rs +++ b/objectstore-types/src/metadata.rs @@ -375,9 +375,7 @@ pub struct Metadata { /// An optional filename associated with this object (header: `x-sn-filename`). /// - /// When present, the server includes a `Content-Disposition: attachment; filename=""` - /// header in GET responses, prompting browsers and download tools to save the file - /// under this name. Non-ASCII filenames additionally get an RFC 8187 `filename*` parameter. + /// This filename is returned when downloading the object. /// /// This is a logical string and may contain arbitrary Unicode; it is escaped only on the /// wire (see [the module docs](self#escaping-free-form-values)).