Skip to content

Commit 6e6471a

Browse files
authored
GET of a 0-byte object must not mmap 1 byte (#9)
* test: GET of a 0-byte object must not ask mmap for 1 byte Inclusive-end on size=0 is length 1. HEAD already advertises 0. Darwin mmap fails with EINVAL (500); Linux returns a phantom 0x00. * fix: empty REST GET is length 0, not mmap(1) Full GET uses the exclusive window (0, size), matching S3 GetObject and HEAD Content-Length. size=0 now takes Body::empty() instead of mmap past EOF.
1 parent aa7f8c4 commit 6e6471a

2 files changed

Lines changed: 108 additions & 12 deletions

File tree

‎crates/server/src/routes/objects.rs‎

Lines changed: 49 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -248,19 +248,19 @@ pub async fn get_object(
248248
let mut resp_headers = build_object_headers(&info);
249249

250250
let range = parse_range(&headers, info.size)?;
251-
let (status, start, end_inclusive) = match range {
252-
Some((s, e)) => {
253-
resp_headers.insert(
254-
header::CONTENT_RANGE,
255-
HeaderValue::from_str(&format!("bytes {s}-{e}/{}", info.size))
256-
.map_err(|_| ApiError::Internal(anyhow::anyhow!("content-range encode")))?,
257-
);
258-
(StatusCode::PARTIAL_CONTENT, s, e)
259-
}
260-
None => (StatusCode::OK, 0, info.size.saturating_sub(1)),
251+
if let Some((s, e)) = range {
252+
resp_headers.insert(
253+
header::CONTENT_RANGE,
254+
HeaderValue::from_str(&format!("bytes {s}-{e}/{}", info.size))
255+
.map_err(|_| ApiError::Internal(anyhow::anyhow!("content-range encode")))?,
256+
);
257+
}
258+
let status = if range.is_some() {
259+
StatusCode::PARTIAL_CONTENT
260+
} else {
261+
StatusCode::OK
261262
};
262-
263-
let length = end_inclusive.saturating_sub(start).saturating_add(1);
263+
let (start, length) = get_window(info.size, range);
264264
resp_headers.insert(
265265
header::CONTENT_LENGTH,
266266
HeaderValue::from_str(&length.to_string())
@@ -718,3 +718,40 @@ fn parse_range(headers: &HeaderMap, size: u64) -> Result<Option<(u64, u64)>, Api
718718
}
719719
Ok(Some((start, end)))
720720
}
721+
722+
/// Byte window for a GET: `(start, length)`.
723+
///
724+
/// `range` is the inclusive `(start, end)` from [`parse_range`]. A missing
725+
/// Range is the exclusive window `(0, size)` — same as S3 `GetObject`.
726+
/// Inclusive-end (`size.saturating_sub(1) + 1`) turns size=0 into length 1
727+
/// and mmap's one byte past EOF.
728+
fn get_window(size: u64, range: Option<(u64, u64)>) -> (u64, u64) {
729+
match range {
730+
Some((s, e)) => (s, e.saturating_sub(s).saturating_add(1)),
731+
None => (0, size),
732+
}
733+
}
734+
735+
#[cfg(test)]
736+
mod tests {
737+
use super::get_window;
738+
739+
#[test]
740+
fn empty_object_full_get_is_zero_bytes() {
741+
assert_eq!(
742+
get_window(0, None),
743+
(0, 0),
744+
"GET of a 0-byte object must not ask mmap for 1 byte"
745+
);
746+
}
747+
748+
#[test]
749+
fn nonempty_full_get_is_the_object_size() {
750+
assert_eq!(get_window(10, None), (0, 10));
751+
}
752+
753+
#[test]
754+
fn inclusive_range_length() {
755+
assert_eq!(get_window(10, Some((2, 5))), (2, 4));
756+
}
757+
}

‎crates/server/tests/rest/objects.rs‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,65 @@ async fn put_get_head_delete_roundtrip() {
7676
assert_eq!(res.status(), reqwest::StatusCode::NOT_FOUND);
7777
}
7878

79+
/// PUT of an empty object is legal. GET without Range must return
80+
/// Content-Length 0 and an empty body — not mmap 1 byte past EOF
81+
/// (Darwin 500, Linux a one-byte zero body).
82+
#[tokio::test]
83+
async fn empty_object_get_is_zero_bytes() {
84+
let bucket = unique_bucket("empty");
85+
create_bucket(&bucket, "private").await;
86+
let token = bucket_token(&bucket);
87+
88+
let res = client()
89+
.put(url(&format!("/v1/{bucket}/marker")))
90+
.bearer_auth(&token)
91+
.header("content-type", "application/octet-stream")
92+
.body(Vec::<u8>::new())
93+
.send()
94+
.await
95+
.unwrap();
96+
assert_eq!(res.status(), reqwest::StatusCode::CREATED);
97+
let put_body: serde_json::Value = res.json().await.unwrap();
98+
assert_eq!(put_body["size"], 0);
99+
100+
let res = client()
101+
.head(url(&format!("/v1/{bucket}/marker")))
102+
.bearer_auth(&token)
103+
.send()
104+
.await
105+
.unwrap();
106+
assert_eq!(res.status(), reqwest::StatusCode::OK);
107+
assert_eq!(
108+
res.headers()
109+
.get("content-length")
110+
.and_then(|v| v.to_str().ok()),
111+
Some("0")
112+
);
113+
114+
let res = client()
115+
.get(url(&format!("/v1/{bucket}/marker")))
116+
.bearer_auth(&token)
117+
.send()
118+
.await
119+
.unwrap();
120+
assert_eq!(
121+
res.status(),
122+
reqwest::StatusCode::OK,
123+
"GET empty must not 500 (mmap past EOF)"
124+
);
125+
assert_eq!(
126+
res.headers()
127+
.get("content-length")
128+
.and_then(|v| v.to_str().ok()),
129+
Some("0")
130+
);
131+
let bytes = res.bytes().await.unwrap();
132+
assert!(
133+
bytes.is_empty(),
134+
"GET empty must not invent a byte: {bytes:?}"
135+
);
136+
}
137+
79138
#[tokio::test]
80139
async fn private_object_requires_auth() {
81140
let bucket = unique_bucket("priv");

0 commit comments

Comments
 (0)