This is an automated email from the ASF dual-hosted git repository.
maskit pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
The following commit(s) were added to refs/heads/master by this push:
new b070028fc5 Remove the dependency for HttpConfig from iocore/cache
(#10711)
b070028fc5 is described below
commit b070028fc55cbcd46bcbfae0b37574a2cd650e2c
Author: Masakazu Kitajo <[email protected]>
AuthorDate: Thu Nov 2 11:05:24 2023 +0900
Remove the dependency for HttpConfig from iocore/cache (#10711)
* Remove the dependency for HttpConfig from iocore/cache
* Fix link issues
* Remove struct Vol
* Fix use-after-free issue
---
include/iocore/cache/Cache.h | 4 +--
include/iocore/cache/CacheVC.h | 3 +-
include/iocore/cache/HttpConfigAccessor.h | 38 +++++++++++++++++++++++++
include/iocore/cache/HttpTransactCache.h | 8 +++---
include/proxy/http/HttpCacheSM.h | 47 ++++++++++++++++++++++++++++---
src/iocore/cache/CMakeLists.txt | 2 +-
src/iocore/cache/Cache.cc | 4 +--
src/iocore/cache/CacheRead.cc | 2 +-
src/iocore/cache/HttpTransactCache.cc | 19 +++++++------
src/iocore/cache/P_CacheInternal.h | 2 +-
src/iocore/cache/unit_tests/main.h | 32 ++++++++++++++++++++-
src/proxy/http/HttpCacheSM.cc | 2 +-
src/tests/CMakeLists.txt | 2 +-
src/traffic_logcat/CMakeLists.txt | 1 +
14 files changed, 138 insertions(+), 28 deletions(-)
diff --git a/include/iocore/cache/Cache.h b/include/iocore/cache/Cache.h
index 8b4ae1a073..1ce26a1f50 100644
--- a/include/iocore/cache/Cache.h
+++ b/include/iocore/cache/Cache.h
@@ -28,6 +28,7 @@
#include "iocore/aio/AIO.h"
#include "iocore/cache/CacheDefs.h"
#include "iocore/cache/Store.h"
+#include "iocore/cache/HttpConfigAccessor.h"
static constexpr ts::ModuleVersion CACHE_MODULE_VERSION(1, 0);
@@ -51,7 +52,6 @@ enum { RAM_HIT_COMPRESS_NONE = 1, RAM_HIT_COMPRESS_FASTLZ,
RAM_HIT_COMPRESS_LIBZ
struct CacheVC;
class CacheEvacuateDocVC;
struct CacheDisk;
-struct OverridableHttpConfigParams;
class URL;
class HTTPHdr;
class HTTPInfo;
@@ -85,7 +85,7 @@ struct CacheProcessor : public Processor {
const char *hostname = nullptr, int host_len = 0);
Action *scan(Continuation *cont, char *hostname = nullptr, int host_len = 0,
int KB_per_second = SCAN_KB_PER_SECOND);
Action *lookup(Continuation *cont, const HttpCacheKey *key, CacheFragType
frag_type = CACHE_FRAG_TYPE_HTTP);
- Action *open_read(Continuation *cont, const HttpCacheKey *key, CacheHTTPHdr
*request, const OverridableHttpConfigParams *params,
+ Action *open_read(Continuation *cont, const HttpCacheKey *key, CacheHTTPHdr
*request, const HttpConfigAccessor *params,
time_t pin_in_cache = (time_t)0, CacheFragType frag_type =
CACHE_FRAG_TYPE_HTTP);
Action *open_write(Continuation *cont, int expected_size, const HttpCacheKey
*key, CacheHTTPHdr *request, CacheHTTPInfo *old_info,
time_t pin_in_cache = (time_t)0, CacheFragType frag_type
= CACHE_FRAG_TYPE_HTTP);
diff --git a/include/iocore/cache/CacheVC.h b/include/iocore/cache/CacheVC.h
index 22001bb456..dcfa47a02d 100644
--- a/include/iocore/cache/CacheVC.h
+++ b/include/iocore/cache/CacheVC.h
@@ -49,6 +49,7 @@
#include <cstdint>
struct Stripe;
+class HttpConfigAccessor;
struct CacheVC : public CacheVConnection {
CacheVC();
@@ -264,7 +265,7 @@ struct CacheVC : public CacheVConnection {
CacheFragType frag_type;
CacheHTTPInfo *info;
CacheHTTPInfoVector *write_vector;
- const OverridableHttpConfigParams *params;
+ const HttpConfigAccessor *params;
int header_len; // for communicating with agg_copy
int frag_len; // for communicating with agg_copy
uint32_t write_len; // for communicating with agg_copy
diff --git a/include/iocore/cache/HttpConfigAccessor.h
b/include/iocore/cache/HttpConfigAccessor.h
new file mode 100644
index 0000000000..6b37ec57b6
--- /dev/null
+++ b/include/iocore/cache/HttpConfigAccessor.h
@@ -0,0 +1,38 @@
+/** @file
+
+ A brief file description
+
+ @section license License
+
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+ */
+
+#pragma once
+
+class HttpConfigAccessor
+{
+public:
+ virtual int8_t get_ignore_accept_mismatch() const = 0;
+ virtual int8_t get_ignore_accept_charset_mismatch() const = 0;
+ virtual int8_t get_ignore_accept_encoding_mismatch() const = 0;
+ virtual int8_t get_ignore_accept_language_mismatch() const = 0;
+ virtual const char *get_global_user_agent_header() const = 0;
+
+protected:
+ HttpConfigAccessor() {}
+ virtual ~HttpConfigAccessor() {}
+};
diff --git a/include/iocore/cache/HttpTransactCache.h
b/include/iocore/cache/HttpTransactCache.h
index 09af4fdff8..ecf7f70eef 100644
--- a/include/iocore/cache/HttpTransactCache.h
+++ b/include/iocore/cache/HttpTransactCache.h
@@ -31,7 +31,7 @@
#pragma once
#include "tscore/ink_platform.h"
-#include "proxy/http/HttpConfig.h"
+#include "iocore/cache/HttpConfigAccessor.h"
// This is needed since txn_conf->cache_guaranteed_max_lifetime is currently
not
// readily available in the cache. ToDo: We should fix this with TS-1919
@@ -52,9 +52,9 @@ public:
/////////////////////////////////
static int SelectFromAlternates(CacheHTTPInfoVector *cache_vector_data,
HTTPHdr *client_request,
- const OverridableHttpConfigParams
*cache_lookup_http_config_params);
+ const HttpConfigAccessor
*cache_lookup_http_config_params);
- static float calculate_quality_of_match(const OverridableHttpConfigParams
*http_config_params, HTTPHdr *client_request,
+ static float calculate_quality_of_match(const HttpConfigAccessor
*http_config_params, HTTPHdr *client_request,
HTTPHdr *obj_client_request, HTTPHdr
*obj_origin_server_response);
static float calculate_quality_of_accept_match(MIMEField *accept_field,
MIMEField *content_field);
@@ -75,7 +75,7 @@ public:
// variability & server negotiation routines //
///////////////////////////////////////////////
- static Variability_t CalcVariability(const OverridableHttpConfigParams
*http_config_params, HTTPHdr *client_request,
+ static Variability_t CalcVariability(const HttpConfigAccessor
*http_config_params, HTTPHdr *client_request,
HTTPHdr *obj_client_request, HTTPHdr
*obj_origin_server_response);
static HTTPStatus match_response_to_request_conditionals(HTTPHdr
*ua_request, HTTPHdr *c_response,
diff --git a/include/proxy/http/HttpCacheSM.h b/include/proxy/http/HttpCacheSM.h
index 897ac8fe10..cb4cd7a10d 100644
--- a/include/proxy/http/HttpCacheSM.h
+++ b/include/proxy/http/HttpCacheSM.h
@@ -208,6 +208,45 @@ public:
}
private:
+ class HttpConfigAccessorImpl : public HttpConfigAccessor
+ {
+ public:
+ HttpConfigAccessorImpl(const OverridableHttpConfigParams *params) :
HttpConfigAccessor(), _params(params) {}
+
+ int8_t
+ get_ignore_accept_mismatch() const override
+ {
+ return this->_params->ignore_accept_mismatch;
+ }
+
+ int8_t
+ get_ignore_accept_charset_mismatch() const override
+ {
+ return this->_params->ignore_accept_charset_mismatch;
+ }
+
+ int8_t
+ get_ignore_accept_encoding_mismatch() const override
+ {
+ return this->_params->ignore_accept_encoding_mismatch;
+ }
+
+ int8_t
+ get_ignore_accept_language_mismatch() const override
+ {
+ return this->_params->ignore_accept_language_mismatch;
+ }
+
+ const char *
+ get_global_user_agent_header() const override
+ {
+ return this->_params->global_user_agent_header;
+ }
+
+ private:
+ const OverridableHttpConfigParams *_params = nullptr;
+ };
+
void do_schedule_in();
Action *do_cache_open_read(const HttpCacheKey &);
@@ -221,10 +260,10 @@ private:
bool open_write_cb = false;
// Open read parameters
- int open_read_tries = 0;
- HTTPHdr *read_request_hdr = nullptr;
- const OverridableHttpConfigParams *http_params = nullptr;
- time_t read_pin_in_cache = 0;
+ int open_read_tries = 0;
+ HTTPHdr *read_request_hdr = nullptr;
+ HttpConfigAccessorImpl http_params{nullptr};
+ time_t read_pin_in_cache = 0;
// Open write parameters
bool retry_write = true;
diff --git a/src/iocore/cache/CMakeLists.txt b/src/iocore/cache/CMakeLists.txt
index cc13085e7c..959c8f0381 100644
--- a/src/iocore/cache/CMakeLists.txt
+++ b/src/iocore/cache/CMakeLists.txt
@@ -46,7 +46,7 @@ target_include_directories(inkcache PRIVATE
${CMAKE_SOURCE_DIR}/lib)
target_link_libraries(
inkcache
- PUBLIC ts::aio ts::hdrs ts::http ts::inkevent ts::tscore
+ PUBLIC ts::aio ts::hdrs ts::inkevent ts::tscore
PRIVATE ts::proxy fastlz ZLIB::ZLIB
)
diff --git a/src/iocore/cache/Cache.cc b/src/iocore/cache/Cache.cc
index a01c22b790..76c264072c 100644
--- a/src/iocore/cache/Cache.cc
+++ b/src/iocore/cache/Cache.cc
@@ -1918,8 +1918,8 @@ ink_cache_init(ts::ModuleVersion v)
//----------------------------------------------------------------------------
Action *
-CacheProcessor::open_read(Continuation *cont, const HttpCacheKey *key,
CacheHTTPHdr *request,
- const OverridableHttpConfigParams *params, time_t
pin_in_cache, CacheFragType type)
+CacheProcessor::open_read(Continuation *cont, const HttpCacheKey *key,
CacheHTTPHdr *request, const HttpConfigAccessor *params,
+ time_t pin_in_cache, CacheFragType type)
{
return caches[type]->open_read(cont, &key->hash, request, params, type,
key->hostname, key->hostlen);
}
diff --git a/src/iocore/cache/CacheRead.cc b/src/iocore/cache/CacheRead.cc
index d5fb09acf8..5a3f5d3e9a 100644
--- a/src/iocore/cache/CacheRead.cc
+++ b/src/iocore/cache/CacheRead.cc
@@ -108,7 +108,7 @@ Lcallreturn:
}
Action *
-Cache::open_read(Continuation *cont, const CacheKey *key, CacheHTTPHdr
*request, const OverridableHttpConfigParams *params,
+Cache::open_read(Continuation *cont, const CacheKey *key, CacheHTTPHdr
*request, const HttpConfigAccessor *params,
CacheFragType type, const char *hostname, int host_len)
{
if (!CacheProcessor::IsCacheReady(type)) {
diff --git a/src/iocore/cache/HttpTransactCache.cc
b/src/iocore/cache/HttpTransactCache.cc
index dce3c1b8b1..a581e7b433 100644
--- a/src/iocore/cache/HttpTransactCache.cc
+++ b/src/iocore/cache/HttpTransactCache.cc
@@ -167,7 +167,7 @@ is_empty(char *s)
*/
int
HttpTransactCache::SelectFromAlternates(CacheHTTPInfoVector *cache_vector,
HTTPHdr *client_request,
- const OverridableHttpConfigParams
*http_config_params)
+ const HttpConfigAccessor
*http_config_params)
{
time_t current_age, best_age = CacheHighAgeWatermark;
time_t t_now = 0;
@@ -287,7 +287,7 @@ HttpTransactCache::SelectFromAlternates(CacheHTTPInfoVector
*cache_vector, HTTPH
*/
float
-HttpTransactCache::calculate_quality_of_match(const
OverridableHttpConfigParams *http_config_param, HTTPHdr *client_request,
+HttpTransactCache::calculate_quality_of_match(const HttpConfigAccessor
*http_config_param, HTTPHdr *client_request,
HTTPHdr *obj_client_request,
HTTPHdr *obj_origin_server_response)
{
// For PURGE requests, any alternate is good really.
@@ -320,7 +320,7 @@ HttpTransactCache::calculate_quality_of_match(const
OverridableHttpConfigParams
content_field =
obj_origin_server_response->field_find(MIME_FIELD_CONTENT_TYPE,
MIME_LEN_CONTENT_TYPE);
// Accept: header
- if (http_config_param->ignore_accept_mismatch & vary_skip_mask) {
+ if (http_config_param->get_ignore_accept_mismatch() & vary_skip_mask) {
// Ignore it
q[0] = 1.0;
} else {
@@ -336,7 +336,7 @@ HttpTransactCache::calculate_quality_of_match(const
OverridableHttpConfigParams
if (q[0] >= 0.0) {
// Accept-Charset: header
- if (http_config_param->ignore_accept_charset_mismatch & vary_skip_mask) {
+ if (http_config_param->get_ignore_accept_charset_mismatch() &
vary_skip_mask) {
// Ignore it
q[1] = 1.0;
} else {
@@ -354,7 +354,7 @@ HttpTransactCache::calculate_quality_of_match(const
OverridableHttpConfigParams
if (q[1] >= 0.0) {
// Accept-Encoding: header
- if (http_config_param->ignore_accept_encoding_mismatch & vary_skip_mask)
{
+ if (http_config_param->get_ignore_accept_encoding_mismatch() &
vary_skip_mask) {
// Ignore it
q[2] = 1.0;
} else {
@@ -373,7 +373,7 @@ HttpTransactCache::calculate_quality_of_match(const
OverridableHttpConfigParams
if (q[2] >= 0.0) {
// Accept-Language: header
- if (http_config_param->ignore_accept_language_mismatch &
vary_skip_mask) {
+ if (http_config_param->get_ignore_accept_language_mismatch() &
vary_skip_mask) {
// Ignore it
q[3] = 1.0;
} else {
@@ -1130,7 +1130,7 @@ language_wildcard:
*/
Variability_t
-HttpTransactCache::CalcVariability(const OverridableHttpConfigParams
*http_config_params, HTTPHdr *client_request,
+HttpTransactCache::CalcVariability(const HttpConfigAccessor
*http_config_params, HTTPHdr *client_request,
HTTPHdr *obj_client_request, HTTPHdr
*obj_origin_server_response)
{
ink_assert(http_config_params != nullptr);
@@ -1170,13 +1170,14 @@ HttpTransactCache::CalcVariability(const
OverridableHttpConfigParams *http_confi
// Special case: if 'proxy.config.http.global_user_agent_header' set
//
// we should ignore Vary: User-Agent.
//
////////////////////////////////////////////////////////////////////////////////////////
- if (http_config_params->global_user_agent_header &&
!strcasecmp(const_cast<char *>(field->str), "User-Agent")) {
+ if (http_config_params->get_global_user_agent_header() &&
!strcasecmp(const_cast<char *>(field->str), "User-Agent")) {
continue;
}
// Disable Vary mismatch checking for Accept-Encoding. This is only
safe to
// set if you are promising to fix any
Accept-Encoding/Content-Encoding mismatches.
- if (http_config_params->ignore_accept_encoding_mismatch &&
!strcasecmp(const_cast<char *>(field->str), "Accept-Encoding")) {
+ if (http_config_params->get_ignore_accept_encoding_mismatch() &&
+ !strcasecmp(const_cast<char *>(field->str), "Accept-Encoding")) {
continue;
}
diff --git a/src/iocore/cache/P_CacheInternal.h
b/src/iocore/cache/P_CacheInternal.h
index 7372d6fd73..d6d38e424d 100644
--- a/src/iocore/cache/P_CacheInternal.h
+++ b/src/iocore/cache/P_CacheInternal.h
@@ -595,7 +595,7 @@ struct Cache {
int host_len = 0);
Action *scan(Continuation *cont, const char *hostname = nullptr, int
host_len = 0, int KB_per_second = 2500);
- Action *open_read(Continuation *cont, const CacheKey *key, CacheHTTPHdr
*request, const OverridableHttpConfigParams *params,
+ Action *open_read(Continuation *cont, const CacheKey *key, CacheHTTPHdr
*request, const HttpConfigAccessor *params,
CacheFragType type, const char *hostname, int host_len);
Action *open_write(Continuation *cont, const CacheKey *key, CacheHTTPInfo
*old_info, time_t pin_in_cache = (time_t)0,
const CacheKey *key1 = nullptr, CacheFragType type =
CACHE_FRAG_TYPE_HTTP, const char *hostname = nullptr,
diff --git a/src/iocore/cache/unit_tests/main.h
b/src/iocore/cache/unit_tests/main.h
index 4ff70ccd5a..b6591c6c06 100644
--- a/src/iocore/cache/unit_tests/main.h
+++ b/src/iocore/cache/unit_tests/main.h
@@ -216,6 +216,36 @@ private:
MIOBuffer *_write_buffer = nullptr;
};
+class MockHttpConfigAccessor : public HttpConfigAccessor
+{
+public:
+ int8_t
+ get_ignore_accept_mismatch() const override
+ {
+ return 0;
+ }
+ int8_t
+ get_ignore_accept_charset_mismatch() const override
+ {
+ return 0;
+ }
+ int8_t
+ get_ignore_accept_encoding_mismatch() const override
+ {
+ return 0;
+ }
+ int8_t
+ get_ignore_accept_language_mismatch() const override
+ {
+ return 0;
+ }
+ const char *
+ get_global_user_agent_header() const override
+ {
+ return "";
+ }
+};
+
class CacheReadTest : public CacheTestBase
{
public:
@@ -250,7 +280,7 @@ private:
char *_cursor = nullptr;
MIOBuffer *_read_buffer = nullptr;
IOBufferReader *_reader = nullptr;
- OverridableHttpConfigParams params;
+ MockHttpConfigAccessor params;
};
// Does the test use stored cache files, or initialize new files?
diff --git a/src/proxy/http/HttpCacheSM.cc b/src/proxy/http/HttpCacheSM.cc
index b3f9dae6bc..fdfadc44a4 100644
--- a/src/proxy/http/HttpCacheSM.cc
+++ b/src/proxy/http/HttpCacheSM.cc
@@ -297,7 +297,7 @@ HttpCacheSM::do_cache_open_read(const HttpCacheKey &key)
}
// Initialising read-while-write-inprogress flag
this->readwhilewrite_inprogress = false;
- Action *action_handle = cacheProcessor.open_read(this, &key,
this->read_request_hdr, this->http_params, this->read_pin_in_cache);
+ Action *action_handle = cacheProcessor.open_read(this, &key,
this->read_request_hdr, &http_params, this->read_pin_in_cache);
if (action_handle != ACTION_RESULT_DONE) {
pending_action = action_handle;
diff --git a/src/tests/CMakeLists.txt b/src/tests/CMakeLists.txt
index 336184e79b..7220f20fbf 100644
--- a/src/tests/CMakeLists.txt
+++ b/src/tests/CMakeLists.txt
@@ -71,7 +71,7 @@ macro(add_net_test name)
${CMAKE_SOURCE_DIR}/src/iocore/net/libinknet_stub.cc
${CMAKE_SOURCE_DIR}/src/shared/overridable_txn_vars.cc
${ARGN})
- target_link_libraries(${name} PRIVATE ts::tsapicore ts::inknet
ts::configmanager ts::tsapi ts::inknet ts::configmanager)
+ target_link_libraries(${name} PRIVATE ts::tsapicore ts::inknet
ts::configmanager ts::tsapi ts::inknet ts::http ts::configmanager)
add_test(NAME test_cache_${name} COMMAND $<TARGET_FILE:${name}>)
endmacro()
diff --git a/src/traffic_logcat/CMakeLists.txt
b/src/traffic_logcat/CMakeLists.txt
index 714073a5f3..811685f5d8 100644
--- a/src/traffic_logcat/CMakeLists.txt
+++ b/src/traffic_logcat/CMakeLists.txt
@@ -24,6 +24,7 @@ target_link_libraries(
ts::inknet
ts::logging
ts::hdrs
+ ts::http
ts::tsapicore
ts::diagsconfig
ts::configmanager
