You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by GitBox <gi...@apache.org> on 2022/02/11 18:42:39 UTC
[GitHub] [trafficserver] traeak opened a new pull request #8666: slice and cache_range_requests: allow header override
traeak opened a new pull request #8666:
URL: https://github.com/apache/trafficserver/pull/8666
This adds the ability to override the internal slice skip header (was hard coded to X-Slicer-Info) and the cache_range_requests ims header (was hard coded to X-Crr-Ims).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] traeak commented on a change in pull request #8666: slice and cache_range_requests: allow header override
Posted by GitBox <gi...@apache.org>.
traeak commented on a change in pull request #8666:
URL: https://github.com/apache/trafficserver/pull/8666#discussion_r810231067
##########
File path: plugins/experimental/slice/client.cc
##########
@@ -74,7 +75,7 @@ handle_client_req(TSCont contp, TSEvent event, Data *const data)
data->m_statustype = TS_HTTP_STATUS_REQUESTED_RANGE_NOT_SATISFIABLE;
// First block will give Content-Length
- rangebe = Range(0, data->m_config->m_blockbytes);
+ rangebe = Range(0, conf->m_blockbytes);
}
} else {
DEBUG_LOG("%p Full content request", data);
Review comment:
stupid missed this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] ezelkow1 commented on pull request #8666: slice and cache_range_requests: allow header override
Posted by GitBox <gi...@apache.org>.
ezelkow1 commented on pull request #8666:
URL: https://github.com/apache/trafficserver/pull/8666#issuecomment-1045219609
[approve ci rocky]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] traeak merged pull request #8666: slice and cache_range_requests: allow header override
Posted by GitBox <gi...@apache.org>.
traeak merged pull request #8666:
URL: https://github.com/apache/trafficserver/pull/8666
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] elsloo commented on a change in pull request #8666: slice and cache_range_requests: allow header override
Posted by GitBox <gi...@apache.org>.
elsloo commented on a change in pull request #8666:
URL: https://github.com/apache/trafficserver/pull/8666#discussion_r808309499
##########
File path: plugins/cache_range_requests/cache_range_requests.cc
##########
@@ -111,24 +112,29 @@ create_pluginconfig(int argc, char *const argv[])
--argv;
for (;;) {
- int const opt = getopt_long(argc, argv, "", longopts, nullptr);
+ int const opt = getopt_long(argc, argv, "i:", longopts, nullptr);
if (-1 == opt) {
break;
}
switch (opt) {
- case 'p': {
- DEBUG_LOG("Plugin modifies parent selection key");
- pc->ps_mode = PS_CACHEKEY_URL;
- } break;
case 'c': {
- DEBUG_LOG("Plugin considers the '%.*s' header", (int)X_IMS_HEADER.size(), X_IMS_HEADER.data());
+ DEBUG_LOG("Plugin considers the CRR ims header");
Review comment:
My apologies, I overlooked the section that follows this block and was solely focusing on the removal of the header from this specific debug line.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] traeak commented on a change in pull request #8666: slice and cache_range_requests: allow header override
Posted by GitBox <gi...@apache.org>.
traeak commented on a change in pull request #8666:
URL: https://github.com/apache/trafficserver/pull/8666#discussion_r810231209
##########
File path: plugins/experimental/slice/server.cc
##########
@@ -268,8 +268,8 @@ logSliceError(char const *const message, Data const *const data, HttpHeader cons
crange.toStringClosed(normstr, &normlen);
Review comment:
missed this sry
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] elsloo commented on a change in pull request #8666: slice and cache_range_requests: allow header override
Posted by GitBox <gi...@apache.org>.
elsloo commented on a change in pull request #8666:
URL: https://github.com/apache/trafficserver/pull/8666#discussion_r807227416
##########
File path: plugins/cache_range_requests/cache_range_requests.cc
##########
@@ -111,24 +112,29 @@ create_pluginconfig(int argc, char *const argv[])
--argv;
for (;;) {
- int const opt = getopt_long(argc, argv, "", longopts, nullptr);
+ int const opt = getopt_long(argc, argv, "i:", longopts, nullptr);
if (-1 == opt) {
break;
}
switch (opt) {
- case 'p': {
- DEBUG_LOG("Plugin modifies parent selection key");
- pc->ps_mode = PS_CACHEKEY_URL;
- } break;
case 'c': {
- DEBUG_LOG("Plugin considers the '%.*s' header", (int)X_IMS_HEADER.size(), X_IMS_HEADER.data());
+ DEBUG_LOG("Plugin considers the CRR ims header");
Review comment:
Can this be reverted such that the default header is printed out when a custom header is not in use? Specifically, print the value of `DefaultImsHeader` here.
##########
File path: plugins/experimental/slice/client.cc
##########
@@ -74,7 +75,7 @@ handle_client_req(TSCont contp, TSEvent event, Data *const data)
data->m_statustype = TS_HTTP_STATUS_REQUESTED_RANGE_NOT_SATISFIABLE;
// First block will give Content-Length
- rangebe = Range(0, data->m_config->m_blockbytes);
+ rangebe = Range(0, conf->m_blockbytes);
}
} else {
DEBUG_LOG("%p Full content request", data);
Review comment:
Please mitigate the reference to `SLICER_MIME_FIELD_INFO` on line 84 below.
##########
File path: plugins/experimental/slice/slice.h
##########
@@ -32,8 +32,6 @@
#define PLUGIN_NAME "slice"
#endif
-constexpr std::string_view X_CRR_IMS_HEADER = {"X-Crr-Ims"};
Review comment:
This constant and all references to `X_CRR_IMS_HEADER` were removed, but `SLICER_MIME_FIELD_INFO` still exists in `HttpHeader.h` (line 39). Consequently there are two areas that still reference the old default `X-Slicer-Info` header name using the constant, and it is injected into requests despite having a "custom" header configuration.
##########
File path: plugins/experimental/slice/server.cc
##########
@@ -268,8 +268,8 @@ logSliceError(char const *const message, Data const *const data, HttpHeader cons
crange.toStringClosed(normstr, &normlen);
Review comment:
Please mitigate the reference to `SLICER_MIME_FIELD_INFO` on line 262 above.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] traeak commented on a change in pull request #8666: slice and cache_range_requests: allow header override
Posted by GitBox <gi...@apache.org>.
traeak commented on a change in pull request #8666:
URL: https://github.com/apache/trafficserver/pull/8666#discussion_r807328170
##########
File path: plugins/cache_range_requests/cache_range_requests.cc
##########
@@ -111,24 +112,29 @@ create_pluginconfig(int argc, char *const argv[])
--argv;
for (;;) {
- int const opt = getopt_long(argc, argv, "", longopts, nullptr);
+ int const opt = getopt_long(argc, argv, "i:", longopts, nullptr);
if (-1 == opt) {
break;
}
switch (opt) {
- case 'p': {
- DEBUG_LOG("Plugin modifies parent selection key");
- pc->ps_mode = PS_CACHEKEY_URL;
- } break;
case 'c': {
- DEBUG_LOG("Plugin considers the '%.*s' header", (int)X_IMS_HEADER.size(), X_IMS_HEADER.data());
+ DEBUG_LOG("Plugin considers the CRR ims header");
Review comment:
The other 2 messages in the same function should cover it (based on override being used or not):
DEBUG_LOG("Plugin uses custom CRR ims header: %s", optarg);
DEBUG_LOG("Plugin uses default ims header: %s", pc->ims_header.c_str())
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] bryancall commented on pull request #8666: slice and cache_range_requests: allow header override
Posted by GitBox <gi...@apache.org>.
bryancall commented on pull request #8666:
URL: https://github.com/apache/trafficserver/pull/8666#issuecomment-1039701748
@elsloo Can you please review this?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] bryancall commented on pull request #8666: slice and cache_range_requests: allow header override
Posted by GitBox <gi...@apache.org>.
bryancall commented on pull request #8666:
URL: https://github.com/apache/trafficserver/pull/8666#issuecomment-1039701748
@elsloo Can you please review this?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org