Skip to content

Commit 632358b

Browse files
committed
test and fix If-Match handling
Seeking in Chrome 55 wasn't working. It apparently sends If-Match requests with the correct etag, which Moonfire NVR was incorrectly responding to with "Precondition failed" responses. Fix and test that. I hadn't noticed the problem in earlier versions of Chrome. I think they were using If-Range instead, which is already tested and working.
1 parent 8df0eae commit 632358b

File tree

1 file changed

+81
-3
lines changed

1 file changed

+81
-3
lines changed

src/resource.rs

Lines changed: 81 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,9 @@ fn none_match(etag: Option<&header::EntityTag>, req: &Request) -> bool {
124124
/// Returns true if `req` has no `If-Match` header or one which matches `etag`.
125125
fn any_match(etag: Option<&header::EntityTag>, req: &Request) -> bool {
126126
match req.headers.get::<header::IfMatch>() {
127-
Some(&header::IfMatch::Any) => true,
127+
// The absent header and "If-Match: *" cases differ only when there is no entity to serve.
128+
// We always have an entity to serve, so consider them identical.
129+
None | Some(&header::IfMatch::Any) => true,
128130
Some(&header::IfMatch::Items(ref items)) => {
129131
if let Some(some_etag) = etag {
130132
for item in items {
@@ -135,7 +137,6 @@ fn any_match(etag: Option<&header::EntityTag>, req: &Request) -> bool {
135137
}
136138
false
137139
},
138-
None => false,
139140
}
140141
}
141142

@@ -173,7 +174,7 @@ pub fn serve(rsrc: &Resource, req: &Request, mut res: Response<Fresh>) -> Result
173174
}
174175
}
175176

176-
if any_match(etag, req) {
177+
if !any_match(etag, req) {
177178
*res.status_mut() = hyper::status::StatusCode::PreconditionFailed;
178179
res.send(b"Precondition failed")?;
179180
return Ok(());
@@ -438,6 +439,27 @@ mod tests {
438439
resp.read_to_end(&mut buf).unwrap();
439440
assert_eq!(b"01234", &buf[..]);
440441

442+
// If-Match any should still send the full body.
443+
let mut resp = client.get(&*SERVER)
444+
.header(header::IfMatch::Any)
445+
.send()
446+
.unwrap();
447+
assert_eq!(hyper::status::StatusCode::Ok, resp.status);
448+
assert_eq!(Some(&header::ContentType(mime!(Application/OctetStream))),
449+
resp.headers.get::<header::ContentType>());
450+
assert_eq!(None, resp.headers.get::<header::ContentRange>());
451+
buf.clear();
452+
resp.read_to_end(&mut buf).unwrap();
453+
assert_eq!(b"01234", &buf[..]);
454+
455+
// If-Match by etag doesn't match (as this request has no etag).
456+
let resp =
457+
client.get(&*SERVER)
458+
.header(header::IfMatch::Items(vec![EntityTag::strong("foo".to_owned())]))
459+
.send()
460+
.unwrap();
461+
assert_eq!(hyper::status::StatusCode::PreconditionFailed, resp.status);
462+
441463
// If-None-Match any.
442464
let mut resp = client.get(&*SERVER)
443465
.header(header::IfNoneMatch::Any)
@@ -571,6 +593,41 @@ mod tests {
571593
let client = hyper::Client::new();
572594
let mut buf = Vec::new();
573595

596+
// If-Match any should still send the full body.
597+
let mut resp = client.get(&*SERVER)
598+
.header(header::IfMatch::Any)
599+
.send()
600+
.unwrap();
601+
assert_eq!(hyper::status::StatusCode::Ok, resp.status);
602+
assert_eq!(Some(&header::ContentType(mime!(Application/OctetStream))),
603+
resp.headers.get::<header::ContentType>());
604+
assert_eq!(None, resp.headers.get::<header::ContentRange>());
605+
buf.clear();
606+
resp.read_to_end(&mut buf).unwrap();
607+
assert_eq!(b"01234", &buf[..]);
608+
609+
// If-Match by matching etag should send the full body.
610+
let mut resp =
611+
client.get(&*SERVER)
612+
.header(header::IfMatch::Items(vec![EntityTag::strong("foo".to_owned())]))
613+
.send()
614+
.unwrap();
615+
assert_eq!(hyper::status::StatusCode::Ok, resp.status);
616+
assert_eq!(Some(&header::ContentType(mime!(Application/OctetStream))),
617+
resp.headers.get::<header::ContentType>());
618+
assert_eq!(None, resp.headers.get::<header::ContentRange>());
619+
buf.clear();
620+
resp.read_to_end(&mut buf).unwrap();
621+
assert_eq!(b"01234", &buf[..]);
622+
623+
// If-Match by etag which doesn't match.
624+
let resp =
625+
client.get(&*SERVER)
626+
.header(header::IfMatch::Items(vec![EntityTag::strong("bar".to_owned())]))
627+
.send()
628+
.unwrap();
629+
assert_eq!(hyper::status::StatusCode::PreconditionFailed, resp.status);
630+
574631
// If-None-Match by etag which matches.
575632
let mut resp =
576633
client.get(&*SERVER)
@@ -640,6 +697,27 @@ mod tests {
640697
let client = hyper::Client::new();
641698
let mut buf = Vec::new();
642699

700+
// If-Match any should still send the full body.
701+
let mut resp = client.get(&*SERVER)
702+
.header(header::IfMatch::Any)
703+
.send()
704+
.unwrap();
705+
assert_eq!(hyper::status::StatusCode::Ok, resp.status);
706+
assert_eq!(Some(&header::ContentType(mime!(Application/OctetStream))),
707+
resp.headers.get::<header::ContentType>());
708+
assert_eq!(None, resp.headers.get::<header::ContentRange>());
709+
buf.clear();
710+
resp.read_to_end(&mut buf).unwrap();
711+
assert_eq!(b"01234", &buf[..]);
712+
713+
// If-Match by etag doesn't match because matches use the strong comparison function.
714+
let resp =
715+
client.get(&*SERVER)
716+
.header(header::IfMatch::Items(vec![EntityTag::weak("foo".to_owned())]))
717+
.send()
718+
.unwrap();
719+
assert_eq!(hyper::status::StatusCode::PreconditionFailed, resp.status);
720+
643721
// If-None-Match by identical weak etag is sufficient.
644722
let mut resp =
645723
client.get(&*SERVER)

0 commit comments

Comments
 (0)