Skip to content

Commit 9b71ada

Browse files
Glue for range request ABI
1 parent 60d2f3b commit 9b71ada

File tree

3 files changed

+96
-46
lines changed

3 files changed

+96
-46
lines changed

lib/src/cache.rs

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -263,11 +263,6 @@ impl From<Arc<CacheData>> for Found {
263263
}
264264

265265
impl Found {
266-
/// Access the body of the cached object.
267-
pub async fn body(&self) -> Result<Body, crate::Error> {
268-
self.get_body().build().await
269-
}
270-
271266
fn get_body(&self) -> GetBodyBuilder {
272267
self.data.as_ref().body()
273268
}
@@ -616,7 +611,7 @@ mod tests {
616611

617612
let nonempty = cache.lookup(&key, &HeaderMap::default()).await;
618613
let found = nonempty.found().expect("should have found inserted key");
619-
let got = found.body().await.unwrap().read_into_vec().await.unwrap();
614+
let got = found.get_body().build().await.unwrap().read_into_vec().await.unwrap();
620615
assert_eq!(got, value);
621616
});
622617
}

lib/src/component/cache.rs

Lines changed: 47 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ fn load_lookup_options(
123123
session: &Session,
124124
options_mask: api::LookupOptionsMask,
125125
options: api::LookupOptions,
126-
) -> Result<HeaderMap, Error> {
126+
) -> Result<(HeaderMap, bool), Error> {
127127
let headers = if options_mask.contains(api::LookupOptionsMask::REQUEST_HEADERS) {
128128
let handle = options.request_headers;
129129
let parts = session.request_parts(handle.into())?;
@@ -133,10 +133,14 @@ fn load_lookup_options(
133133
};
134134
let options_mask = options_mask & !api::LookupOptionsMask::REQUEST_HEADERS;
135135

136+
let always_use_requested_range =
137+
options_mask.contains(api::LookupOptionsMask::ALWAYS_USE_REQUESTED_RANGE);
138+
let options_mask = options_mask & !api::LookupOptionsMask::ALWAYS_USE_REQUESTED_RANGE;
139+
136140
if options_mask != api::LookupOptionsMask::empty() {
137141
return Err(Error::NotAvailable("unknown cache lookup option"));
138142
}
139-
Ok(headers)
143+
Ok((headers, always_use_requested_range))
140144
}
141145

142146
#[async_trait::async_trait]
@@ -147,14 +151,18 @@ impl api::Host for ComponentCtx {
147151
options_mask: api::LookupOptionsMask,
148152
options: api::LookupOptions,
149153
) -> Result<api::Handle, types::Error> {
150-
let headers = load_lookup_options(&self.session, options_mask, options)?;
154+
let (headers, always_use_requested_range) =
155+
load_lookup_options(&self.session, options_mask, options)?;
151156

152157
let key: CacheKey = get_key(key)?;
153158
let cache = Arc::clone(self.session.cache());
154159

155-
let task = PeekableTask::spawn(Box::pin(
156-
async move { Ok(cache.lookup(&key, &headers).await) },
157-
))
160+
let task = PeekableTask::spawn(Box::pin(async move {
161+
Ok(cache
162+
.lookup(&key, &headers)
163+
.await
164+
.with_always_use_requested_range(always_use_requested_range))
165+
}))
158166
.await;
159167
let task = PendingCacheTask::new(task);
160168
let handle: CacheHandle = self.session.insert_cache_op(task).into();
@@ -298,33 +306,44 @@ impl api::Host for ComponentCtx {
298306
async fn get_body(
299307
&mut self,
300308
handle: api::Handle,
301-
options_mask: api::GetBodyOptionsMask,
302-
_options: api::GetBodyOptions,
309+
mut options_mask: api::GetBodyOptionsMask,
310+
options: api::GetBodyOptions,
303311
) -> Result<http_types::BodyHandle, types::Error> {
312+
let from = if options_mask.contains(api::GetBodyOptionsMask::FROM) {
313+
Some(options.from)
314+
} else {
315+
None
316+
};
317+
options_mask &= !api::GetBodyOptionsMask::FROM;
318+
let to = if options_mask.contains(api::GetBodyOptionsMask::TO) {
319+
Some(options.to)
320+
} else {
321+
None
322+
};
323+
options_mask &= !api::GetBodyOptionsMask::TO;
324+
304325
if options_mask != api::GetBodyOptionsMask::empty() {
305326
return Err(Error::NotAvailable("unknown cache get_body option").into());
306327
}
307328

308329
// We wind up re-borrowing `found` and `self.session` several times here, to avoid
309-
// borrowing the both of them at once. Ultimately it is possible that inserting a body
310-
// would change the address of Found, by re-shuffling the AsyncItems table; once again,
311-
// borrowck wins the day.
330+
// borrowing the both of them at once.
331+
// (It possible that inserting a body would change the address of Found, by re-shuffling
332+
// the AsyncItems table; we have to live by borrowck's rules.)
312333
//
313334
// We have an exclusive borrow &mut self.session for the lifetime of this call,
314335
// so even though we're re-borrowing/repeating lookups, we know we won't run into TOCTOU.
315336

316-
let found = self
317-
.session
318-
.cache_entry(handle.into())
319-
.await?
320-
.found()
321-
.ok_or_else(|| Error::CacheError(cache::Error::Missing))?;
337+
let entry = self.session.cache_entry(handle.into()).await?;
322338

323339
// Preemptively (optimistically) start a read. Don't worry, the Drop impl for Body will
324340
// clean up the copying task.
325341
// We have to do this to allow `found`'s lifetime to end before self.session.body, which
326342
// has to re-borrow self.self.session.
327-
let body = found.body().await?;
343+
let body = entry.body(from, to).await?;
344+
let found = entry
345+
.found()
346+
.ok_or(Error::CacheError(crate::cache::Error::Missing))?;
328347

329348
if let Some(prev_handle) = found.last_body_handle {
330349
// Check if they're still reading the previous handle.
@@ -364,21 +383,28 @@ impl api::Host for ComponentCtx {
364383
options_mask: api::LookupOptionsMask,
365384
options: api::LookupOptions,
366385
) -> Result<api::BusyHandle, types::Error> {
367-
let headers = load_lookup_options(&self.session, options_mask, options)?;
386+
let (headers, always_use_requested_range) =
387+
load_lookup_options(&self.session, options_mask, options)?;
368388

369389
let key: CacheKey = get_key(key)?;
370390
let cache = Arc::clone(self.session.cache());
371391

372392
// Look up once, joining the transaction only if obligated:
373-
let e = cache.transaction_lookup(&key, &headers, false).await;
393+
let e = cache
394+
.transaction_lookup(&key, &headers, false)
395+
.await
396+
.with_always_use_requested_range(always_use_requested_range);
374397
let ready = e.found().is_some() || e.go_get().is_some();
375398
// If we already got _something_, we can provide an already-complete PeekableTask.
376399
// Otherwise we need to spawn it and let it block in the background.
377400
let task = if ready {
378401
PeekableTask::complete(e)
379402
} else {
380403
PeekableTask::spawn(Box::pin(async move {
381-
Ok(cache.transaction_lookup(&key, &headers, true).await)
404+
Ok(cache
405+
.transaction_lookup(&key, &headers, true)
406+
.await
407+
.with_always_use_requested_range(always_use_requested_range))
382408
}))
383409
.await
384410
};

lib/src/wiggle_abi/cache.rs

Lines changed: 48 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ fn load_lookup_options(
139139
memory: &wiggle::GuestMemory<'_>,
140140
mut options_mask: types::CacheLookupOptionsMask,
141141
options: wiggle::GuestPtr<types::CacheLookupOptions>,
142-
) -> Result<HeaderMap, Error> {
142+
) -> Result<(HeaderMap, bool), Error> {
143143
let options = memory.read(options)?;
144144
let headers = if options_mask.contains(types::CacheLookupOptionsMask::REQUEST_HEADERS) {
145145
let handle = options.request_headers;
@@ -158,11 +158,15 @@ fn load_lookup_options(
158158
});
159159
}
160160

161+
let always_use_requested_range =
162+
options_mask.contains(types::CacheLookupOptionsMask::ALWAYS_USE_REQUESTED_RANGE);
163+
options_mask &= !types::CacheLookupOptionsMask::ALWAYS_USE_REQUESTED_RANGE;
164+
161165
if !options_mask.is_empty() {
162166
return Err(Error::NotAvailable("unknown cache lookup option"));
163167
}
164168

165-
Ok(headers)
169+
Ok((headers, always_use_requested_range))
166170
}
167171

168172
#[allow(unused_variables)]
@@ -175,13 +179,17 @@ impl FastlyCache for Session {
175179
options_mask: types::CacheLookupOptionsMask,
176180
options: wiggle::GuestPtr<types::CacheLookupOptions>,
177181
) -> Result<types::CacheHandle, Error> {
178-
let headers = load_lookup_options(self, memory, options_mask, options)?;
182+
let (headers, always_use_requested_range) =
183+
load_lookup_options(self, memory, options_mask, options)?;
179184
let key = load_cache_key(memory, cache_key)?;
180185
let cache = Arc::clone(self.cache());
181186

182-
let task = PeekableTask::spawn(Box::pin(
183-
async move { Ok(cache.lookup(&key, &headers).await) },
184-
))
187+
let task = PeekableTask::spawn(Box::pin(async move {
188+
Ok(cache
189+
.lookup(&key, &headers)
190+
.await
191+
.with_always_use_requested_range(always_use_requested_range))
192+
}))
185193
.await;
186194
let task = PendingCacheTask::new(task);
187195
let handle = self.insert_cache_op(task);
@@ -330,20 +338,27 @@ impl FastlyCache for Session {
330338
options_mask: types::CacheLookupOptionsMask,
331339
options: wiggle::GuestPtr<types::CacheLookupOptions>,
332340
) -> Result<types::CacheBusyHandle, Error> {
333-
let headers = load_lookup_options(self, memory, options_mask, options)?;
341+
let (headers, always_use_requested_range) =
342+
load_lookup_options(self, memory, options_mask, options)?;
334343
let key = load_cache_key(memory, cache_key)?;
335344
let cache = Arc::clone(self.cache());
336345

337346
// Look up once, joining the transaction only if obligated:
338-
let e = cache.transaction_lookup(&key, &headers, false).await;
347+
let e = cache
348+
.transaction_lookup(&key, &headers, false)
349+
.await
350+
.with_always_use_requested_range(always_use_requested_range);
339351
let ready = e.found().is_some() || e.go_get().is_some();
340352
// If we already got _something_, we can provide an already-complete PeekableTask.
341353
// Otherwise we need to spawn it and let it block in the background.
342354
let task = if ready {
343355
PeekableTask::complete(e)
344356
} else {
345357
PeekableTask::spawn(Box::pin(async move {
346-
Ok(cache.transaction_lookup(&key, &headers, true).await)
358+
Ok(cache
359+
.transaction_lookup(&key, &headers, true)
360+
.await
361+
.with_always_use_requested_range(always_use_requested_range))
347362
}))
348363
.await
349364
};
@@ -529,31 +544,45 @@ impl FastlyCache for Session {
529544
&mut self,
530545
memory: &mut wiggle::GuestMemory<'_>,
531546
handle: types::CacheHandle,
532-
options_mask: types::CacheGetBodyOptionsMask,
547+
mut options_mask: types::CacheGetBodyOptionsMask,
533548
options: &types::CacheGetBodyOptions,
534549
) -> Result<types::BodyHandle, Error> {
550+
let from = if options_mask.contains(types::CacheGetBodyOptionsMask::FROM) {
551+
Some(options.from)
552+
} else {
553+
None
554+
};
555+
options_mask &= !types::CacheGetBodyOptionsMask::FROM;
556+
let to = if options_mask.contains(types::CacheGetBodyOptionsMask::TO) {
557+
Some(options.to)
558+
} else {
559+
None
560+
};
561+
options_mask &= !types::CacheGetBodyOptionsMask::TO;
562+
535563
if !options_mask.is_empty() {
536564
return Err(Error::NotAvailable("unknown cache get_body option").into());
537565
}
538566

539567
// We wind up re-borrowing `found` and `self.session` several times here, to avoid
540-
// borrowing the both of them at once. Ultimately it is possible that inserting a body
541-
// would change the address of Found, by re-shuffling the AsyncItems table; once again,
542-
// borrowck wins the day.
568+
// borrowing the both of them at once.
569+
// (It possible that inserting a body would change the address of Found, by re-shuffling
570+
// the AsyncItems table; we have to live by borrowck's rules.)
543571
//
544572
// We have an exclusive borrow &mut self.session for the lifetime of this call,
545573
// so even though we're re-borrowing/repeating lookups, we know we won't run into TOCTOU.
546574

547-
let found = self
548-
.cache_entry(handle.into())
549-
.await?
550-
.found()
551-
.ok_or(Error::CacheError(crate::cache::Error::Missing))?;
575+
let entry = self.cache_entry(handle.into()).await?;
576+
552577
// Preemptively (optimistically) start a read. Don't worry, the Drop impl for Body will
553578
// clean up the copying task.
554579
// We have to do this to allow `found`'s lifetime to end before self.session.body, which
555580
// has to re-borrow self.self.session.
556-
let body = found.body().await?;
581+
let body = entry.body(from, to).await?;
582+
583+
let found = entry
584+
.found()
585+
.ok_or(Error::CacheError(crate::cache::Error::Missing))?;
557586

558587
if let Some(prev_handle) = found.last_body_handle {
559588
// Check if they're still reading the previous handle.

0 commit comments

Comments
 (0)