From 69c5ed31900f17cdd1606fad69dfae2a8b04f400 Mon Sep 17 00:00:00 2001 From: Vijay Mamidi Date: Wed, 20 Jul 2022 10:01:46 -0700 Subject: [PATCH 1/4] make doc corrupt for content length mismatch --- iocore/cache/CacheRead.cc | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/iocore/cache/CacheRead.cc b/iocore/cache/CacheRead.cc index 4c33cdf5d26..ab7df9dc6cc 100644 --- a/iocore/cache/CacheRead.cc +++ b/iocore/cache/CacheRead.cc @@ -721,13 +721,20 @@ CacheVC::openReadMain(int /* event ATS_UNUSED */, Event * /* e ATS_UNUSED */) key.toHexStr(target_key_str)); } + bool doc_corrupt = false; + CacheHTTPInfo *ainfo = nullptr; + get_http_info(&ainfo); + MIMEField *field = ainfo->m_alt->m_response_hdr.field_find(MIME_FIELD_CONTENT_LENGTH, MIME_LEN_CONTENT_LENGTH); + if (field && field->value_get_int64() != doc->len) + doc_corrupt = true; + // This shouldn't happen for HTTP assets but it does // occasionally in production. This is a temporary fix // to clean up broken objects until the root cause can // be found. It must be the case that either the fragment // offsets are incorrect or a fragment table isn't being // created when it should be. - if (frag_type == CACHE_FRAG_TYPE_HTTP && bytes < 0) { + if (frag_type == CACHE_FRAG_TYPE_HTTP && (bytes < 0 || doc_corrupt)) { char xt[CRYPTO_HEX_SIZE]; char yt[CRYPTO_HEX_SIZE]; From 706248ab03bb8e46806502f81aae8e7931cad2cb Mon Sep 17 00:00:00 2001 From: Chris McFarlen Date: Wed, 20 Jul 2022 14:40:14 -0500 Subject: [PATCH 2/4] Move validation to openReadStartHead --- iocore/cache/CacheRead.cc | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/iocore/cache/CacheRead.cc b/iocore/cache/CacheRead.cc index ab7df9dc6cc..1a3812eb0b4 100644 --- a/iocore/cache/CacheRead.cc +++ b/iocore/cache/CacheRead.cc @@ -721,20 +721,13 @@ CacheVC::openReadMain(int /* event ATS_UNUSED */, Event * /* e ATS_UNUSED */) key.toHexStr(target_key_str)); } - bool doc_corrupt = false; - CacheHTTPInfo *ainfo = nullptr; - get_http_info(&ainfo); - MIMEField *field = ainfo->m_alt->m_response_hdr.field_find(MIME_FIELD_CONTENT_LENGTH, MIME_LEN_CONTENT_LENGTH); - if (field && field->value_get_int64() != doc->len) - doc_corrupt = true; - // This shouldn't happen for HTTP assets but it does // occasionally in production. This is a temporary fix // to clean up broken objects until the root cause can // be found. It must be the case that either the fragment // offsets are incorrect or a fragment table isn't being // created when it should be. - if (frag_type == CACHE_FRAG_TYPE_HTTP && (bytes < 0 || doc_corrupt)) { + if (frag_type == CACHE_FRAG_TYPE_HTTP && bytes < 0) { char xt[CRYPTO_HEX_SIZE]; char yt[CRYPTO_HEX_SIZE]; @@ -1182,6 +1175,19 @@ CacheVC::openReadStartHead(int event, Event *e) } else { f.single_fragment = false; } + + // Now that we have selected an alternate, validate that the content length and object size match + MIMEField *field = alternate.request_get()->field_find(MIME_FIELD_CONTENT_LENGTH, MIME_LEN_CONTENT_LENGTH); + if (field) { + int64_t cl = field->value_get_int64(); + if (cl != doc_len) { + Note("OpenReadHead failed for cachekey %X : alternate content length doesn't match doc_len %ld != %ld", key.slice32(0), + cl, doc_len); + err = ECACHE_BAD_META_DATA; + goto Ldone; + } + } + } else { next_CacheKey(&key, &doc->key); f.single_fragment = doc->single_fragment(); From 13d986fe1a16c0c4a4f340ee052d47d513ecb3eb Mon Sep 17 00:00:00 2001 From: Chris McFarlen Date: Thu, 21 Jul 2022 15:43:55 -0500 Subject: [PATCH 3/4] use response headers to validate content-length. Add stat to track validation failures on cache read. --- iocore/cache/Cache.cc | 1 + iocore/cache/CacheRead.cc | 7 ++++--- iocore/cache/P_CacheInternal.h | 1 + 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/iocore/cache/Cache.cc b/iocore/cache/Cache.cc index 22b502aac89..ca5fcc60966 100644 --- a/iocore/cache/Cache.cc +++ b/iocore/cache/Cache.cc @@ -3089,6 +3089,7 @@ register_cache_stats(RecRawStatBlock *rsb, const char *prefix) REG_INT("read.success", cache_read_success_stat); REG_INT("read.failure", cache_read_failure_stat); REG_INT("read.seek.failure", cache_read_seek_fail_stat); + REG_INT("read.invalid", cache_read_invalid_stat); REG_INT("write.active", cache_write_active_stat); REG_INT("write.success", cache_write_success_stat); REG_INT("write.failure", cache_write_failure_stat); diff --git a/iocore/cache/CacheRead.cc b/iocore/cache/CacheRead.cc index 1a3812eb0b4..b463c796b5f 100644 --- a/iocore/cache/CacheRead.cc +++ b/iocore/cache/CacheRead.cc @@ -1177,12 +1177,13 @@ CacheVC::openReadStartHead(int event, Event *e) } // Now that we have selected an alternate, validate that the content length and object size match - MIMEField *field = alternate.request_get()->field_find(MIME_FIELD_CONTENT_LENGTH, MIME_LEN_CONTENT_LENGTH); + MIMEField *field = alternate.response_get()->field_find(MIME_FIELD_CONTENT_LENGTH, MIME_LEN_CONTENT_LENGTH); if (field) { int64_t cl = field->value_get_int64(); if (cl != doc_len) { - Note("OpenReadHead failed for cachekey %X : alternate content length doesn't match doc_len %ld != %ld", key.slice32(0), - cl, doc_len); + Warning("OpenReadHead failed for cachekey %X : alternate content length doesn't match doc_len %ld != %ld", key.slice32(0), + cl, doc_len); + CACHE_INCREMENT_DYN_STAT(cache_read_invalid_stat); err = ECACHE_BAD_META_DATA; goto Ldone; } diff --git a/iocore/cache/P_CacheInternal.h b/iocore/cache/P_CacheInternal.h index cf747de60cf..2d7a8cb8432 100644 --- a/iocore/cache/P_CacheInternal.h +++ b/iocore/cache/P_CacheInternal.h @@ -114,6 +114,7 @@ enum { cache_read_success_stat, cache_read_failure_stat, cache_read_seek_fail_stat, + cache_read_invalid_stat, cache_write_active_stat, cache_write_success_stat, cache_write_failure_stat, From 6d725bda4da0e76a74f18d8906cb497cbf3e808d Mon Sep 17 00:00:00 2001 From: Chris McFarlen Date: Tue, 26 Jul 2022 17:57:14 -0500 Subject: [PATCH 4/4] compare same types --- iocore/cache/CacheRead.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/iocore/cache/CacheRead.cc b/iocore/cache/CacheRead.cc index b463c796b5f..9d9f2452b26 100644 --- a/iocore/cache/CacheRead.cc +++ b/iocore/cache/CacheRead.cc @@ -1179,7 +1179,7 @@ CacheVC::openReadStartHead(int event, Event *e) // Now that we have selected an alternate, validate that the content length and object size match MIMEField *field = alternate.response_get()->field_find(MIME_FIELD_CONTENT_LENGTH, MIME_LEN_CONTENT_LENGTH); if (field) { - int64_t cl = field->value_get_int64(); + uint64_t cl = static_cast(field->value_get_int64()); if (cl != doc_len) { Warning("OpenReadHead failed for cachekey %X : alternate content length doesn't match doc_len %ld != %ld", key.slice32(0), cl, doc_len);