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/09/26 19:48:36 UTC
[GitHub] [trafficserver] zwoop opened a new pull request, #9107: Adds support for serving statichit content out of a directory
zwoop opened a new pull request, #9107:
URL: https://github.com/apache/trafficserver/pull/9107
This does some fairly expensive computations when producing the directory paths. This is intentional,
to make sure requests aren't escaping the specified directory and out elsewhere. This cleans out abusive
requests such as get `/../../etc/hosts` etc.
The idea is that rather than having to specify every individual file that the server should service, you can specify a
directory and every file (and subdirectory thereunder) can now be served.
--
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] SolidWallOfCode commented on a diff in pull request #9107: Adds support for serving statichit content out of a directory
Posted by GitBox <gi...@apache.org>.
SolidWallOfCode commented on code in PR #9107:
URL: https://github.com/apache/trafficserver/pull/9107#discussion_r993670223
##########
plugins/experimental/statichit/statichit.cc:
##########
@@ -62,21 +63,80 @@ static int StaticHitInterceptHook(TSCont contp, TSEvent event, void *edata);
static int StaticHitTxnHook(TSCont contp, TSEvent event, void *edata);
struct StaticHitConfig {
- explicit StaticHitConfig(const std::string &filePath, const std::string &mimeType, bool disableExact)
- : filePath(filePath), mimeType(mimeType), disableExact(disableExact)
+ explicit StaticHitConfig(const std::string &fileOrDir, const std::string &mimeType, bool exact) : mimeType(mimeType)
{
+ std::filesystem::path base_path;
+
+ if (fileOrDir.find('/') != 0) {
+ base_path = std::filesystem::weakly_canonical(std::string(TSConfigDirGet()) + "/" + fileOrDir);
+ } else {
+ base_path = std::filesystem::weakly_canonical(fileOrDir);
+ }
+
+ if (std::filesystem::is_directory(base_path)) {
+ dirPath = base_path;
+ filePath = "";
+ disableExact = true;
+ } else if (std::filesystem::is_regular_file(base_path)) {
+ dirPath = "";
+ filePath = base_path;
+ disableExact = exact;
+ } else {
+ VERROR("Invalid file path: %s", filePath.c_str());
+ filePath = "";
+ dirPath = "";
+ }
}
~StaticHitConfig() { TSContDestroy(cont); }
+ std::string_view
+ makePath(TSHttpTxn txnp, std::string &output) const
Review Comment:
To avoid a copy when it's not needed.
--
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 #9107: Adds support for serving statichit content out of a directory
Posted by GitBox <gi...@apache.org>.
bryancall commented on PR #9107:
URL: https://github.com/apache/trafficserver/pull/9107#issuecomment-1258751461
@zwoop You should really add labels to the PRs.
--
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] zwoop commented on a diff in pull request #9107: Adds support for serving statichit content out of a directory
Posted by GitBox <gi...@apache.org>.
zwoop commented on code in PR #9107:
URL: https://github.com/apache/trafficserver/pull/9107#discussion_r995156194
##########
plugins/experimental/statichit/statichit.cc:
##########
@@ -62,21 +63,80 @@ static int StaticHitInterceptHook(TSCont contp, TSEvent event, void *edata);
static int StaticHitTxnHook(TSCont contp, TSEvent event, void *edata);
struct StaticHitConfig {
- explicit StaticHitConfig(const std::string &filePath, const std::string &mimeType, bool disableExact)
- : filePath(filePath), mimeType(mimeType), disableExact(disableExact)
+ explicit StaticHitConfig(const std::string &fileOrDir, const std::string &mimeType, bool exact) : mimeType(mimeType)
{
+ std::filesystem::path base_path;
+
+ if (fileOrDir.find('/') != 0) {
+ base_path = std::filesystem::weakly_canonical(std::string(TSConfigDirGet()) + "/" + fileOrDir);
+ } else {
+ base_path = std::filesystem::weakly_canonical(fileOrDir);
+ }
+
+ if (std::filesystem::is_directory(base_path)) {
+ dirPath = base_path;
+ filePath = "";
+ disableExact = true;
+ } else if (std::filesystem::is_regular_file(base_path)) {
+ dirPath = "";
+ filePath = base_path;
+ disableExact = exact;
+ } else {
+ VERROR("Invalid file path: %s", filePath.c_str());
+ filePath = "";
+ dirPath = "";
+ }
}
~StaticHitConfig() { TSContDestroy(cont); }
+ std::string_view
+ makePath(TSHttpTxn txnp, std::string &output) const
+ {
+ if (!dirPath.empty()) {
Review Comment:
Just staying with the way the original plugin was written, where public member variables are not underscored, and we do not explicitly specify 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] SolidWallOfCode commented on a diff in pull request #9107: Adds support for serving statichit content out of a directory
Posted by GitBox <gi...@apache.org>.
SolidWallOfCode commented on code in PR #9107:
URL: https://github.com/apache/trafficserver/pull/9107#discussion_r993660844
##########
plugins/experimental/statichit/statichit.cc:
##########
@@ -62,21 +63,80 @@ static int StaticHitInterceptHook(TSCont contp, TSEvent event, void *edata);
static int StaticHitTxnHook(TSCont contp, TSEvent event, void *edata);
struct StaticHitConfig {
- explicit StaticHitConfig(const std::string &filePath, const std::string &mimeType, bool disableExact)
- : filePath(filePath), mimeType(mimeType), disableExact(disableExact)
+ explicit StaticHitConfig(const std::string &fileOrDir, const std::string &mimeType, bool exact) : mimeType(mimeType)
{
+ std::filesystem::path base_path;
+
+ if (fileOrDir.find('/') != 0) {
+ base_path = std::filesystem::weakly_canonical(std::string(TSConfigDirGet()) + "/" + fileOrDir);
+ } else {
+ base_path = std::filesystem::weakly_canonical(fileOrDir);
+ }
Review Comment:
I'd restructure this as
```
std::filesystem::base_path { fileOrDir };
if (! base_path.is_absolute()) {
base_path = std::filesystem::path(TSConfigDirGet()) / base_path;
}
base_path = std::filesystem::weakly_canonical(base_path);
```
--
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] SolidWallOfCode commented on a diff in pull request #9107: Adds support for serving statichit content out of a directory
Posted by GitBox <gi...@apache.org>.
SolidWallOfCode commented on code in PR #9107:
URL: https://github.com/apache/trafficserver/pull/9107#discussion_r993669920
##########
plugins/experimental/statichit/statichit.cc:
##########
@@ -62,21 +63,80 @@ static int StaticHitInterceptHook(TSCont contp, TSEvent event, void *edata);
static int StaticHitTxnHook(TSCont contp, TSEvent event, void *edata);
struct StaticHitConfig {
- explicit StaticHitConfig(const std::string &filePath, const std::string &mimeType, bool disableExact)
- : filePath(filePath), mimeType(mimeType), disableExact(disableExact)
+ explicit StaticHitConfig(const std::string &fileOrDir, const std::string &mimeType, bool exact) : mimeType(mimeType)
{
+ std::filesystem::path base_path;
+
+ if (fileOrDir.find('/') != 0) {
+ base_path = std::filesystem::weakly_canonical(std::string(TSConfigDirGet()) + "/" + fileOrDir);
+ } else {
+ base_path = std::filesystem::weakly_canonical(fileOrDir);
+ }
+
+ if (std::filesystem::is_directory(base_path)) {
+ dirPath = base_path;
+ filePath = "";
+ disableExact = true;
+ } else if (std::filesystem::is_regular_file(base_path)) {
+ dirPath = "";
+ filePath = base_path;
+ disableExact = exact;
+ } else {
+ VERROR("Invalid file path: %s", filePath.c_str());
+ filePath = "";
+ dirPath = "";
+ }
Review Comment:
If you're going to do multiple checks, it's better to grab the status and check that.
```
std::error_code ec;
auto stat = std::filesystem::status(base_path, ec);
if (ec) {
return swoc::Errata("{} is not a valid path - {}", base_path, ec);
} else if (std::filesystem::is_directory(stat)) {
// ...
} else if (std::filesystem::is_regular_file(stat)) {
// ...
}
--
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] zwoop commented on a diff in pull request #9107: Adds support for serving statichit content out of a directory
Posted by GitBox <gi...@apache.org>.
zwoop commented on code in PR #9107:
URL: https://github.com/apache/trafficserver/pull/9107#discussion_r1016286285
##########
plugins/experimental/statichit/statichit.cc:
##########
@@ -62,21 +63,74 @@ static int StaticHitInterceptHook(TSCont contp, TSEvent event, void *edata);
static int StaticHitTxnHook(TSCont contp, TSEvent event, void *edata);
struct StaticHitConfig {
- explicit StaticHitConfig(const std::string &filePath, const std::string &mimeType, bool disableExact)
- : filePath(filePath), mimeType(mimeType), disableExact(disableExact)
+ explicit StaticHitConfig(const std::string &fileOrDir, const std::string &mimeType, bool exact) : mimeType(mimeType)
{
+ std::filesystem::path base_path{fileOrDir};
+
+ if (!base_path.is_absolute()) {
+ base_path = std::filesystem::path(TSConfigDirGet()) / base_path;
+ }
+ base_path = std::filesystem::weakly_canonical(base_path);
+
+ if (std::filesystem::is_directory(base_path)) {
+ dirPath = base_path;
+ filePath = "";
+ disableExact = true;
+ } else {
+ dirPath = "";
+ filePath = base_path;
+ disableExact = exact;
+ }
}
~StaticHitConfig() { TSContDestroy(cont); }
+ std::string_view
+ makePath(TSHttpTxn txnp, std::string &output) const
+ {
+ std::string_view ret = {};
+
+ if (!dirPath.empty()) {
+ TSMBuffer reqp;
+ TSMLoc hdr_loc = nullptr, url_loc = nullptr;
+
+ if (TS_SUCCESS == TSHttpTxnClientReqGet(txnp, &reqp, &hdr_loc)) {
+ if (TS_SUCCESS == TSHttpHdrUrlGet(reqp, hdr_loc, &url_loc)) {
+ int path_len = 0;
+ auto path = TSUrlPathGet(reqp, url_loc, &path_len);
+
+ std::filesystem::path requested_file_path(
+ std::filesystem::weakly_canonical(dirPath / std::string_view{path, static_cast<size_t>(path_len)}));
+
+ TSHandleMLocRelease(reqp, hdr_loc, url_loc);
+ TSHandleMLocRelease(reqp, TS_NULL_MLOC, hdr_loc);
+
+ if (std::equal(dirPath.begin(), dirPath.end(), requested_file_path.begin()) &&
+ std::filesystem::is_regular_file(requested_file_path)) {
+ output = requested_file_path.string();
+ ret = {output.c_str(), output.size()};
+ }
+ } else {
+ TSHandleMLocRelease(reqp, TS_NULL_MLOC, hdr_loc);
+ }
+ }
+ } else {
+ ret = {filePath};
Review Comment:
Gah, thanks. I don't think it's a "clang" issue per se, it obviously worked with some clang compilers. Will look again when I get back.
--
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] randall commented on pull request #9107: Adds support for serving statichit content out of a directory
Posted by GitBox <gi...@apache.org>.
randall commented on PR #9107:
URL: https://github.com/apache/trafficserver/pull/9107#issuecomment-1258600253
[approve ci]
--
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] zwoop commented on a diff in pull request #9107: Adds support for serving statichit content out of a directory
Posted by GitBox <gi...@apache.org>.
zwoop commented on code in PR #9107:
URL: https://github.com/apache/trafficserver/pull/9107#discussion_r995151866
##########
plugins/experimental/statichit/statichit.cc:
##########
@@ -62,21 +63,80 @@ static int StaticHitInterceptHook(TSCont contp, TSEvent event, void *edata);
static int StaticHitTxnHook(TSCont contp, TSEvent event, void *edata);
struct StaticHitConfig {
- explicit StaticHitConfig(const std::string &filePath, const std::string &mimeType, bool disableExact)
- : filePath(filePath), mimeType(mimeType), disableExact(disableExact)
+ explicit StaticHitConfig(const std::string &fileOrDir, const std::string &mimeType, bool exact) : mimeType(mimeType)
{
+ std::filesystem::path base_path;
+
+ if (fileOrDir.find('/') != 0) {
+ base_path = std::filesystem::weakly_canonical(std::string(TSConfigDirGet()) + "/" + fileOrDir);
+ } else {
+ base_path = std::filesystem::weakly_canonical(fileOrDir);
+ }
+
+ if (std::filesystem::is_directory(base_path)) {
+ dirPath = base_path;
+ filePath = "";
+ disableExact = true;
+ } else if (std::filesystem::is_regular_file(base_path)) {
+ dirPath = "";
+ filePath = base_path;
+ disableExact = exact;
+ } else {
+ VERROR("Invalid file path: %s", filePath.c_str());
+ filePath = "";
+ dirPath = "";
+ }
Review Comment:
Actually, this may be a bug, the file (or even directory) may not exist on startup.
--
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] SolidWallOfCode commented on a diff in pull request #9107: Adds support for serving statichit content out of a directory
Posted by GitBox <gi...@apache.org>.
SolidWallOfCode commented on code in PR #9107:
URL: https://github.com/apache/trafficserver/pull/9107#discussion_r993657981
##########
plugins/experimental/statichit/statichit.cc:
##########
@@ -62,21 +63,80 @@ static int StaticHitInterceptHook(TSCont contp, TSEvent event, void *edata);
static int StaticHitTxnHook(TSCont contp, TSEvent event, void *edata);
struct StaticHitConfig {
- explicit StaticHitConfig(const std::string &filePath, const std::string &mimeType, bool disableExact)
- : filePath(filePath), mimeType(mimeType), disableExact(disableExact)
+ explicit StaticHitConfig(const std::string &fileOrDir, const std::string &mimeType, bool exact) : mimeType(mimeType)
{
+ std::filesystem::path base_path;
+
+ if (fileOrDir.find('/') != 0) {
+ base_path = std::filesystem::weakly_canonical(std::string(TSConfigDirGet()) + "/" + fileOrDir);
Review Comment:
No - use `std::filesystem::operator /`. E.g.
```
base_path = std::filesystem::weakly_canonical(std::filesystem::path(TSConfigDirGet()) / fileOrDir);
```
That operator takes care of the little details like trailing / leading separators, etc.
--
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] SolidWallOfCode commented on a diff in pull request #9107: Adds support for serving statichit content out of a directory
Posted by GitBox <gi...@apache.org>.
SolidWallOfCode commented on code in PR #9107:
URL: https://github.com/apache/trafficserver/pull/9107#discussion_r993673119
##########
plugins/experimental/statichit/statichit.cc:
##########
@@ -62,21 +63,80 @@ static int StaticHitInterceptHook(TSCont contp, TSEvent event, void *edata);
static int StaticHitTxnHook(TSCont contp, TSEvent event, void *edata);
struct StaticHitConfig {
- explicit StaticHitConfig(const std::string &filePath, const std::string &mimeType, bool disableExact)
- : filePath(filePath), mimeType(mimeType), disableExact(disableExact)
+ explicit StaticHitConfig(const std::string &fileOrDir, const std::string &mimeType, bool exact) : mimeType(mimeType)
{
+ std::filesystem::path base_path;
+
+ if (fileOrDir.find('/') != 0) {
+ base_path = std::filesystem::weakly_canonical(std::string(TSConfigDirGet()) + "/" + fileOrDir);
+ } else {
+ base_path = std::filesystem::weakly_canonical(fileOrDir);
+ }
+
+ if (std::filesystem::is_directory(base_path)) {
+ dirPath = base_path;
+ filePath = "";
+ disableExact = true;
+ } else if (std::filesystem::is_regular_file(base_path)) {
+ dirPath = "";
+ filePath = base_path;
+ disableExact = exact;
+ } else {
+ VERROR("Invalid file path: %s", filePath.c_str());
+ filePath = "";
+ dirPath = "";
+ }
}
~StaticHitConfig() { TSContDestroy(cont); }
+ std::string_view
+ makePath(TSHttpTxn txnp, std::string &output) const
+ {
+ if (!dirPath.empty()) {
Review Comment:
If this is a member variable, you need to either use leading underscores or `this->dirPath` so it's clear where the value is from.
--
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] SolidWallOfCode commented on a diff in pull request #9107: Adds support for serving statichit content out of a directory
Posted by GitBox <gi...@apache.org>.
SolidWallOfCode commented on code in PR #9107:
URL: https://github.com/apache/trafficserver/pull/9107#discussion_r993660844
##########
plugins/experimental/statichit/statichit.cc:
##########
@@ -62,21 +63,80 @@ static int StaticHitInterceptHook(TSCont contp, TSEvent event, void *edata);
static int StaticHitTxnHook(TSCont contp, TSEvent event, void *edata);
struct StaticHitConfig {
- explicit StaticHitConfig(const std::string &filePath, const std::string &mimeType, bool disableExact)
- : filePath(filePath), mimeType(mimeType), disableExact(disableExact)
+ explicit StaticHitConfig(const std::string &fileOrDir, const std::string &mimeType, bool exact) : mimeType(mimeType)
{
+ std::filesystem::path base_path;
+
+ if (fileOrDir.find('/') != 0) {
+ base_path = std::filesystem::weakly_canonical(std::string(TSConfigDirGet()) + "/" + fileOrDir);
+ } else {
+ base_path = std::filesystem::weakly_canonical(fileOrDir);
+ }
Review Comment:
I'd restructure this as
```
std::filesystem::base_path { fileOrDir };
if (! fileOrDir.empty() && fileOrDir[0] == '/') {
base_path = std::filesystem::path(TSConfigDirGet()) / base_path;
}
```
--
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] zwoop commented on pull request #9107: Adds support for serving statichit content out of a directory
Posted by GitBox <gi...@apache.org>.
zwoop commented on PR #9107:
URL: https://github.com/apache/trafficserver/pull/9107#issuecomment-1319291492
Cherry-picked to v9.2.x
--
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] zwoop commented on a diff in pull request #9107: Adds support for serving statichit content out of a directory
Posted by GitBox <gi...@apache.org>.
zwoop commented on code in PR #9107:
URL: https://github.com/apache/trafficserver/pull/9107#discussion_r995144496
##########
plugins/experimental/statichit/statichit.cc:
##########
@@ -62,21 +63,80 @@ static int StaticHitInterceptHook(TSCont contp, TSEvent event, void *edata);
static int StaticHitTxnHook(TSCont contp, TSEvent event, void *edata);
struct StaticHitConfig {
- explicit StaticHitConfig(const std::string &filePath, const std::string &mimeType, bool disableExact)
- : filePath(filePath), mimeType(mimeType), disableExact(disableExact)
+ explicit StaticHitConfig(const std::string &fileOrDir, const std::string &mimeType, bool exact) : mimeType(mimeType)
{
+ std::filesystem::path base_path;
+
+ if (fileOrDir.find('/') != 0) {
+ base_path = std::filesystem::weakly_canonical(std::string(TSConfigDirGet()) + "/" + fileOrDir);
Review Comment:
Changed.
##########
plugins/experimental/statichit/statichit.cc:
##########
@@ -62,21 +63,80 @@ static int StaticHitInterceptHook(TSCont contp, TSEvent event, void *edata);
static int StaticHitTxnHook(TSCont contp, TSEvent event, void *edata);
struct StaticHitConfig {
- explicit StaticHitConfig(const std::string &filePath, const std::string &mimeType, bool disableExact)
- : filePath(filePath), mimeType(mimeType), disableExact(disableExact)
+ explicit StaticHitConfig(const std::string &fileOrDir, const std::string &mimeType, bool exact) : mimeType(mimeType)
Review Comment:
I didn't change 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] ywkaras commented on a diff in pull request #9107: Adds support for serving statichit content out of a directory
Posted by GitBox <gi...@apache.org>.
ywkaras commented on code in PR #9107:
URL: https://github.com/apache/trafficserver/pull/9107#discussion_r1018433218
##########
plugins/experimental/statichit/statichit.cc:
##########
@@ -62,21 +63,74 @@ static int StaticHitInterceptHook(TSCont contp, TSEvent event, void *edata);
static int StaticHitTxnHook(TSCont contp, TSEvent event, void *edata);
struct StaticHitConfig {
- explicit StaticHitConfig(const std::string &filePath, const std::string &mimeType, bool disableExact)
- : filePath(filePath), mimeType(mimeType), disableExact(disableExact)
+ explicit StaticHitConfig(const std::string &fileOrDir, const std::string &mimeType, bool exact) : mimeType(mimeType)
{
+ std::filesystem::path base_path{fileOrDir};
+
+ if (!base_path.is_absolute()) {
+ base_path = std::filesystem::path(TSConfigDirGet()) / base_path;
+ }
+ base_path = std::filesystem::weakly_canonical(base_path);
+
+ if (std::filesystem::is_directory(base_path)) {
+ dirPath = base_path;
+ filePath = "";
+ disableExact = true;
+ } else {
+ dirPath = "";
+ filePath = base_path;
+ disableExact = exact;
+ }
}
~StaticHitConfig() { TSContDestroy(cont); }
+ std::string_view
+ makePath(TSHttpTxn txnp, std::string &output) const
+ {
+ std::string_view ret = {};
+
+ if (!dirPath.empty()) {
+ TSMBuffer reqp;
+ TSMLoc hdr_loc = nullptr, url_loc = nullptr;
+
+ if (TS_SUCCESS == TSHttpTxnClientReqGet(txnp, &reqp, &hdr_loc)) {
+ if (TS_SUCCESS == TSHttpHdrUrlGet(reqp, hdr_loc, &url_loc)) {
+ int path_len = 0;
+ auto path = TSUrlPathGet(reqp, url_loc, &path_len);
+
+ std::filesystem::path requested_file_path(
+ std::filesystem::weakly_canonical(dirPath / std::string_view{path, static_cast<size_t>(path_len)}));
+
+ TSHandleMLocRelease(reqp, hdr_loc, url_loc);
+ TSHandleMLocRelease(reqp, TS_NULL_MLOC, hdr_loc);
+
+ if (std::equal(dirPath.begin(), dirPath.end(), requested_file_path.begin()) &&
+ std::filesystem::is_regular_file(requested_file_path)) {
+ output = requested_file_path.string();
+ ret = {output.c_str(), output.size()};
+ }
+ } else {
+ TSHandleMLocRelease(reqp, TS_NULL_MLOC, hdr_loc);
+ }
+ }
+ } else {
+ ret = filePath;
+ }
+
+ return ret;
+ }
+
+ std::filesystem::path dirPath;
std::string filePath;
- std::string mimeType;
- int successCode = 200;
- int failureCode = 404;
- int maxAge = 0;
+ std::string mimeType = "";
Review Comment:
"" is the default value for std::string anyway.
--
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] ywkaras commented on a diff in pull request #9107: Adds support for serving statichit content out of a directory
Posted by GitBox <gi...@apache.org>.
ywkaras commented on code in PR #9107:
URL: https://github.com/apache/trafficserver/pull/9107#discussion_r1018430961
##########
plugins/experimental/statichit/statichit.cc:
##########
@@ -62,21 +63,74 @@ static int StaticHitInterceptHook(TSCont contp, TSEvent event, void *edata);
static int StaticHitTxnHook(TSCont contp, TSEvent event, void *edata);
struct StaticHitConfig {
- explicit StaticHitConfig(const std::string &filePath, const std::string &mimeType, bool disableExact)
- : filePath(filePath), mimeType(mimeType), disableExact(disableExact)
+ explicit StaticHitConfig(const std::string &fileOrDir, const std::string &mimeType, bool exact) : mimeType(mimeType)
{
+ std::filesystem::path base_path{fileOrDir};
+
+ if (!base_path.is_absolute()) {
+ base_path = std::filesystem::path(TSConfigDirGet()) / base_path;
+ }
+ base_path = std::filesystem::weakly_canonical(base_path);
+
+ if (std::filesystem::is_directory(base_path)) {
+ dirPath = base_path;
+ filePath = "";
+ disableExact = true;
+ } else {
+ dirPath = "";
+ filePath = base_path;
+ disableExact = exact;
+ }
}
~StaticHitConfig() { TSContDestroy(cont); }
+ std::string_view
+ makePath(TSHttpTxn txnp, std::string &output) const
+ {
+ std::string_view ret = {};
+
+ if (!dirPath.empty()) {
+ TSMBuffer reqp;
+ TSMLoc hdr_loc = nullptr, url_loc = nullptr;
+
+ if (TS_SUCCESS == TSHttpTxnClientReqGet(txnp, &reqp, &hdr_loc)) {
+ if (TS_SUCCESS == TSHttpHdrUrlGet(reqp, hdr_loc, &url_loc)) {
+ int path_len = 0;
+ auto path = TSUrlPathGet(reqp, url_loc, &path_len);
+
+ std::filesystem::path requested_file_path(
+ std::filesystem::weakly_canonical(dirPath / std::string_view{path, static_cast<size_t>(path_len)}));
+
+ TSHandleMLocRelease(reqp, hdr_loc, url_loc);
+ TSHandleMLocRelease(reqp, TS_NULL_MLOC, hdr_loc);
+
+ if (std::equal(dirPath.begin(), dirPath.end(), requested_file_path.begin()) &&
Review Comment:
Do we need a case insensitive comparison for OSX?
--
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] ywkaras commented on a diff in pull request #9107: Adds support for serving statichit content out of a directory
Posted by GitBox <gi...@apache.org>.
ywkaras commented on code in PR #9107:
URL: https://github.com/apache/trafficserver/pull/9107#discussion_r989462744
##########
plugins/experimental/statichit/statichit.cc:
##########
@@ -62,21 +63,80 @@ static int StaticHitInterceptHook(TSCont contp, TSEvent event, void *edata);
static int StaticHitTxnHook(TSCont contp, TSEvent event, void *edata);
struct StaticHitConfig {
- explicit StaticHitConfig(const std::string &filePath, const std::string &mimeType, bool disableExact)
- : filePath(filePath), mimeType(mimeType), disableExact(disableExact)
+ explicit StaticHitConfig(const std::string &fileOrDir, const std::string &mimeType, bool exact) : mimeType(mimeType)
{
+ std::filesystem::path base_path;
+
+ if (fileOrDir.find('/') != 0) {
+ base_path = std::filesystem::weakly_canonical(std::string(TSConfigDirGet()) + "/" + fileOrDir);
+ } else {
+ base_path = std::filesystem::weakly_canonical(fileOrDir);
+ }
+
+ if (std::filesystem::is_directory(base_path)) {
+ dirPath = base_path;
+ filePath = "";
+ disableExact = true;
+ } else if (std::filesystem::is_regular_file(base_path)) {
+ dirPath = "";
+ filePath = base_path;
+ disableExact = exact;
+ } else {
+ VERROR("Invalid file path: %s", filePath.c_str());
+ filePath = "";
+ dirPath = "";
+ }
}
~StaticHitConfig() { TSContDestroy(cont); }
+ std::string_view
+ makePath(TSHttpTxn txnp, std::string &output) const
+ {
+ if (!dirPath.empty()) {
+ TSMBuffer reqp;
+ TSMLoc hdr_loc = nullptr, url_loc = nullptr;
+
+ if (TS_SUCCESS == TSHttpTxnClientReqGet(txnp, &reqp, &hdr_loc)) {
+ if (TS_SUCCESS == TSHttpHdrUrlGet(reqp, hdr_loc, &url_loc)) {
+ int path_len = 0;
+ auto path = TSUrlPathGet(reqp, url_loc, &path_len);
+
+ std::filesystem::path requested_file_path(
+ std::filesystem::weakly_canonical(dirPath / std::string_view{path, static_cast<size_t>(path_len)}));
+
+ TSHandleMLocRelease(reqp, hdr_loc, url_loc);
+ TSHandleMLocRelease(reqp, TS_NULL_MLOC, hdr_loc);
+
+ if (std::equal(dirPath.begin(), dirPath.end(), requested_file_path.begin()) &&
+ std::filesystem::is_regular_file(requested_file_path)) {
+ output = requested_file_path.string();
+ return {output.c_str(), output.size()};
+ } else {
+ return {};
+ }
+ } else {
+ TSHandleMLocRelease(reqp, TS_NULL_MLOC, hdr_loc);
+ return {};
+ }
+ } else {
+ return {};
+ }
+ } else {
+ return {filePath};
+ }
+ }
+
+ std::filesystem::path dirPath;
std::string filePath;
- std::string mimeType;
- int successCode = 200;
- int failureCode = 404;
- int maxAge = 0;
+ std::string mimeType = "";
Review Comment:
Doesn't look like this is used.
--
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] zwoop merged pull request #9107: Adds support for serving statichit content out of a directory
Posted by GitBox <gi...@apache.org>.
zwoop merged PR #9107:
URL: https://github.com/apache/trafficserver/pull/9107
--
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] SolidWallOfCode commented on a diff in pull request #9107: Adds support for serving statichit content out of a directory
Posted by GitBox <gi...@apache.org>.
SolidWallOfCode commented on code in PR #9107:
URL: https://github.com/apache/trafficserver/pull/9107#discussion_r993660844
##########
plugins/experimental/statichit/statichit.cc:
##########
@@ -62,21 +63,80 @@ static int StaticHitInterceptHook(TSCont contp, TSEvent event, void *edata);
static int StaticHitTxnHook(TSCont contp, TSEvent event, void *edata);
struct StaticHitConfig {
- explicit StaticHitConfig(const std::string &filePath, const std::string &mimeType, bool disableExact)
- : filePath(filePath), mimeType(mimeType), disableExact(disableExact)
+ explicit StaticHitConfig(const std::string &fileOrDir, const std::string &mimeType, bool exact) : mimeType(mimeType)
{
+ std::filesystem::path base_path;
+
+ if (fileOrDir.find('/') != 0) {
+ base_path = std::filesystem::weakly_canonical(std::string(TSConfigDirGet()) + "/" + fileOrDir);
+ } else {
+ base_path = std::filesystem::weakly_canonical(fileOrDir);
+ }
Review Comment:
I'd restructure this as
```
std::filesystem::base_path { fileOrDir };
if (! base_path.is_absolute()) {
base_path = std::filesystem::path(TSConfigDirGet()) / base_path;
}
```
--
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] SolidWallOfCode commented on a diff in pull request #9107: Adds support for serving statichit content out of a directory
Posted by GitBox <gi...@apache.org>.
SolidWallOfCode commented on code in PR #9107:
URL: https://github.com/apache/trafficserver/pull/9107#discussion_r993676823
##########
plugins/experimental/statichit/statichit.cc:
##########
@@ -62,21 +63,80 @@ static int StaticHitInterceptHook(TSCont contp, TSEvent event, void *edata);
static int StaticHitTxnHook(TSCont contp, TSEvent event, void *edata);
struct StaticHitConfig {
- explicit StaticHitConfig(const std::string &filePath, const std::string &mimeType, bool disableExact)
- : filePath(filePath), mimeType(mimeType), disableExact(disableExact)
+ explicit StaticHitConfig(const std::string &fileOrDir, const std::string &mimeType, bool exact) : mimeType(mimeType)
{
+ std::filesystem::path base_path;
+
+ if (fileOrDir.find('/') != 0) {
+ base_path = std::filesystem::weakly_canonical(std::string(TSConfigDirGet()) + "/" + fileOrDir);
+ } else {
+ base_path = std::filesystem::weakly_canonical(fileOrDir);
+ }
+
+ if (std::filesystem::is_directory(base_path)) {
+ dirPath = base_path;
+ filePath = "";
+ disableExact = true;
+ } else if (std::filesystem::is_regular_file(base_path)) {
+ dirPath = "";
+ filePath = base_path;
+ disableExact = exact;
+ } else {
+ VERROR("Invalid file path: %s", filePath.c_str());
+ filePath = "";
+ dirPath = "";
+ }
}
~StaticHitConfig() { TSContDestroy(cont); }
+ std::string_view
+ makePath(TSHttpTxn txnp, std::string &output) const
+ {
+ if (!dirPath.empty()) {
+ TSMBuffer reqp;
+ TSMLoc hdr_loc = nullptr, url_loc = nullptr;
+
+ if (TS_SUCCESS == TSHttpTxnClientReqGet(txnp, &reqp, &hdr_loc)) {
+ if (TS_SUCCESS == TSHttpHdrUrlGet(reqp, hdr_loc, &url_loc)) {
+ int path_len = 0;
+ auto path = TSUrlPathGet(reqp, url_loc, &path_len);
+
+ std::filesystem::path requested_file_path(
+ std::filesystem::weakly_canonical(dirPath / std::string_view{path, static_cast<size_t>(path_len)}));
+
+ TSHandleMLocRelease(reqp, hdr_loc, url_loc);
+ TSHandleMLocRelease(reqp, TS_NULL_MLOC, hdr_loc);
+
+ if (std::equal(dirPath.begin(), dirPath.end(), requested_file_path.begin()) &&
+ std::filesystem::is_regular_file(requested_file_path)) {
+ output = requested_file_path.string();
+ return {output.c_str(), output.size()};
+ } else {
+ return {};
Review Comment:
I really don't like all these separate returns, since they're all alternates (e.g. not earlier exits). This is what I use `zret` for.
--
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] zwoop commented on a diff in pull request #9107: Adds support for serving statichit content out of a directory
Posted by GitBox <gi...@apache.org>.
zwoop commented on code in PR #9107:
URL: https://github.com/apache/trafficserver/pull/9107#discussion_r995157721
##########
plugins/experimental/statichit/statichit.cc:
##########
@@ -62,21 +63,80 @@ static int StaticHitInterceptHook(TSCont contp, TSEvent event, void *edata);
static int StaticHitTxnHook(TSCont contp, TSEvent event, void *edata);
struct StaticHitConfig {
- explicit StaticHitConfig(const std::string &filePath, const std::string &mimeType, bool disableExact)
- : filePath(filePath), mimeType(mimeType), disableExact(disableExact)
+ explicit StaticHitConfig(const std::string &fileOrDir, const std::string &mimeType, bool exact) : mimeType(mimeType)
{
+ std::filesystem::path base_path;
+
+ if (fileOrDir.find('/') != 0) {
+ base_path = std::filesystem::weakly_canonical(std::string(TSConfigDirGet()) + "/" + fileOrDir);
+ } else {
+ base_path = std::filesystem::weakly_canonical(fileOrDir);
+ }
+
+ if (std::filesystem::is_directory(base_path)) {
+ dirPath = base_path;
+ filePath = "";
+ disableExact = true;
+ } else if (std::filesystem::is_regular_file(base_path)) {
+ dirPath = "";
+ filePath = base_path;
+ disableExact = exact;
+ } else {
+ VERROR("Invalid file path: %s", filePath.c_str());
+ filePath = "";
+ dirPath = "";
+ }
}
~StaticHitConfig() { TSContDestroy(cont); }
+ std::string_view
+ makePath(TSHttpTxn txnp, std::string &output) const
+ {
+ if (!dirPath.empty()) {
+ TSMBuffer reqp;
+ TSMLoc hdr_loc = nullptr, url_loc = nullptr;
+
+ if (TS_SUCCESS == TSHttpTxnClientReqGet(txnp, &reqp, &hdr_loc)) {
+ if (TS_SUCCESS == TSHttpHdrUrlGet(reqp, hdr_loc, &url_loc)) {
+ int path_len = 0;
+ auto path = TSUrlPathGet(reqp, url_loc, &path_len);
+
+ std::filesystem::path requested_file_path(
+ std::filesystem::weakly_canonical(dirPath / std::string_view{path, static_cast<size_t>(path_len)}));
+
+ TSHandleMLocRelease(reqp, hdr_loc, url_loc);
+ TSHandleMLocRelease(reqp, TS_NULL_MLOC, hdr_loc);
+
+ if (std::equal(dirPath.begin(), dirPath.end(), requested_file_path.begin()) &&
+ std::filesystem::is_regular_file(requested_file_path)) {
+ output = requested_file_path.string();
+ return {output.c_str(), output.size()};
+ } else {
+ return {};
Review Comment:
Fixed.
--
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] zwoop commented on a diff in pull request #9107: Adds support for serving statichit content out of a directory
Posted by GitBox <gi...@apache.org>.
zwoop commented on code in PR #9107:
URL: https://github.com/apache/trafficserver/pull/9107#discussion_r995148135
##########
plugins/experimental/statichit/statichit.cc:
##########
@@ -62,21 +63,80 @@ static int StaticHitInterceptHook(TSCont contp, TSEvent event, void *edata);
static int StaticHitTxnHook(TSCont contp, TSEvent event, void *edata);
struct StaticHitConfig {
- explicit StaticHitConfig(const std::string &filePath, const std::string &mimeType, bool disableExact)
- : filePath(filePath), mimeType(mimeType), disableExact(disableExact)
+ explicit StaticHitConfig(const std::string &fileOrDir, const std::string &mimeType, bool exact) : mimeType(mimeType)
{
+ std::filesystem::path base_path;
+
+ if (fileOrDir.find('/') != 0) {
+ base_path = std::filesystem::weakly_canonical(std::string(TSConfigDirGet()) + "/" + fileOrDir);
+ } else {
+ base_path = std::filesystem::weakly_canonical(fileOrDir);
+ }
Review Comment:
But I don't think that's the same. The directory can be e.g. foo/bar, which is not absolute.
--
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] zwoop commented on a diff in pull request #9107: Adds support for serving statichit content out of a directory
Posted by GitBox <gi...@apache.org>.
zwoop commented on code in PR #9107:
URL: https://github.com/apache/trafficserver/pull/9107#discussion_r995161394
##########
plugins/experimental/statichit/statichit.cc:
##########
@@ -62,21 +63,80 @@ static int StaticHitInterceptHook(TSCont contp, TSEvent event, void *edata);
static int StaticHitTxnHook(TSCont contp, TSEvent event, void *edata);
struct StaticHitConfig {
- explicit StaticHitConfig(const std::string &filePath, const std::string &mimeType, bool disableExact)
- : filePath(filePath), mimeType(mimeType), disableExact(disableExact)
+ explicit StaticHitConfig(const std::string &fileOrDir, const std::string &mimeType, bool exact) : mimeType(mimeType)
{
+ std::filesystem::path base_path;
+
+ if (fileOrDir.find('/') != 0) {
+ base_path = std::filesystem::weakly_canonical(std::string(TSConfigDirGet()) + "/" + fileOrDir);
+ } else {
+ base_path = std::filesystem::weakly_canonical(fileOrDir);
+ }
+
+ if (std::filesystem::is_directory(base_path)) {
+ dirPath = base_path;
+ filePath = "";
+ disableExact = true;
+ } else if (std::filesystem::is_regular_file(base_path)) {
+ dirPath = "";
+ filePath = base_path;
+ disableExact = exact;
+ } else {
+ VERROR("Invalid file path: %s", filePath.c_str());
+ filePath = "";
+ dirPath = "";
+ }
}
~StaticHitConfig() { TSContDestroy(cont); }
+ std::string_view
+ makePath(TSHttpTxn txnp, std::string &output) const
+ {
+ if (!dirPath.empty()) {
+ TSMBuffer reqp;
+ TSMLoc hdr_loc = nullptr, url_loc = nullptr;
+
+ if (TS_SUCCESS == TSHttpTxnClientReqGet(txnp, &reqp, &hdr_loc)) {
+ if (TS_SUCCESS == TSHttpHdrUrlGet(reqp, hdr_loc, &url_loc)) {
+ int path_len = 0;
+ auto path = TSUrlPathGet(reqp, url_loc, &path_len);
+
+ std::filesystem::path requested_file_path(
+ std::filesystem::weakly_canonical(dirPath / std::string_view{path, static_cast<size_t>(path_len)}));
+
+ TSHandleMLocRelease(reqp, hdr_loc, url_loc);
+ TSHandleMLocRelease(reqp, TS_NULL_MLOC, hdr_loc);
+
+ if (std::equal(dirPath.begin(), dirPath.end(), requested_file_path.begin()) &&
Review Comment:
That's C++20 only for std::string
--
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] zwoop commented on a diff in pull request #9107: Adds support for serving statichit content out of a directory
Posted by GitBox <gi...@apache.org>.
zwoop commented on code in PR #9107:
URL: https://github.com/apache/trafficserver/pull/9107#discussion_r995165286
##########
plugins/experimental/statichit/statichit.cc:
##########
@@ -62,21 +63,80 @@ static int StaticHitInterceptHook(TSCont contp, TSEvent event, void *edata);
static int StaticHitTxnHook(TSCont contp, TSEvent event, void *edata);
struct StaticHitConfig {
- explicit StaticHitConfig(const std::string &filePath, const std::string &mimeType, bool disableExact)
- : filePath(filePath), mimeType(mimeType), disableExact(disableExact)
+ explicit StaticHitConfig(const std::string &fileOrDir, const std::string &mimeType, bool exact) : mimeType(mimeType)
Review Comment:
Also, this was changed in C++11.
--
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] ywkaras commented on a diff in pull request #9107: Adds support for serving statichit content out of a directory
Posted by GitBox <gi...@apache.org>.
ywkaras commented on code in PR #9107:
URL: https://github.com/apache/trafficserver/pull/9107#discussion_r998285247
##########
plugins/experimental/statichit/statichit.cc:
##########
@@ -62,21 +63,74 @@ static int StaticHitInterceptHook(TSCont contp, TSEvent event, void *edata);
static int StaticHitTxnHook(TSCont contp, TSEvent event, void *edata);
struct StaticHitConfig {
- explicit StaticHitConfig(const std::string &filePath, const std::string &mimeType, bool disableExact)
- : filePath(filePath), mimeType(mimeType), disableExact(disableExact)
+ explicit StaticHitConfig(const std::string &fileOrDir, const std::string &mimeType, bool exact) : mimeType(mimeType)
{
+ std::filesystem::path base_path{fileOrDir};
+
+ if (!base_path.is_absolute()) {
+ base_path = std::filesystem::path(TSConfigDirGet()) / base_path;
+ }
+ base_path = std::filesystem::weakly_canonical(base_path);
+
+ if (std::filesystem::is_directory(base_path)) {
+ dirPath = base_path;
+ filePath = "";
+ disableExact = true;
+ } else {
+ dirPath = "";
+ filePath = base_path;
+ disableExact = exact;
+ }
}
~StaticHitConfig() { TSContDestroy(cont); }
+ std::string_view
+ makePath(TSHttpTxn txnp, std::string &output) const
+ {
+ std::string_view ret = {};
+
+ if (!dirPath.empty()) {
+ TSMBuffer reqp;
+ TSMLoc hdr_loc = nullptr, url_loc = nullptr;
+
+ if (TS_SUCCESS == TSHttpTxnClientReqGet(txnp, &reqp, &hdr_loc)) {
+ if (TS_SUCCESS == TSHttpHdrUrlGet(reqp, hdr_loc, &url_loc)) {
+ int path_len = 0;
+ auto path = TSUrlPathGet(reqp, url_loc, &path_len);
+
+ std::filesystem::path requested_file_path(
+ std::filesystem::weakly_canonical(dirPath / std::string_view{path, static_cast<size_t>(path_len)}));
+
+ TSHandleMLocRelease(reqp, hdr_loc, url_loc);
+ TSHandleMLocRelease(reqp, TS_NULL_MLOC, hdr_loc);
+
+ if (std::equal(dirPath.begin(), dirPath.end(), requested_file_path.begin()) &&
+ std::filesystem::is_regular_file(requested_file_path)) {
+ output = requested_file_path.string();
+ ret = {output.c_str(), output.size()};
+ }
+ } else {
+ TSHandleMLocRelease(reqp, TS_NULL_MLOC, hdr_loc);
+ }
+ }
+ } else {
+ ret = {filePath};
Review Comment:
I guess clang doesn't like the braces cause you're using a conversion operator not a constructor: https://en.cppreference.com/w/cpp/string/basic_string/operator_basic_string_view
--
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] zwoop commented on pull request #9107: Adds support for serving statichit content out of a directory
Posted by GitBox <gi...@apache.org>.
zwoop commented on PR #9107:
URL: https://github.com/apache/trafficserver/pull/9107#issuecomment-1308615459
[approve ci ubuntu]
--
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] SolidWallOfCode commented on a diff in pull request #9107: Adds support for serving statichit content out of a directory
Posted by GitBox <gi...@apache.org>.
SolidWallOfCode commented on code in PR #9107:
URL: https://github.com/apache/trafficserver/pull/9107#discussion_r993675024
##########
plugins/experimental/statichit/statichit.cc:
##########
@@ -62,21 +63,80 @@ static int StaticHitInterceptHook(TSCont contp, TSEvent event, void *edata);
static int StaticHitTxnHook(TSCont contp, TSEvent event, void *edata);
struct StaticHitConfig {
- explicit StaticHitConfig(const std::string &filePath, const std::string &mimeType, bool disableExact)
- : filePath(filePath), mimeType(mimeType), disableExact(disableExact)
+ explicit StaticHitConfig(const std::string &fileOrDir, const std::string &mimeType, bool exact) : mimeType(mimeType)
{
+ std::filesystem::path base_path;
+
+ if (fileOrDir.find('/') != 0) {
+ base_path = std::filesystem::weakly_canonical(std::string(TSConfigDirGet()) + "/" + fileOrDir);
+ } else {
+ base_path = std::filesystem::weakly_canonical(fileOrDir);
+ }
+
+ if (std::filesystem::is_directory(base_path)) {
+ dirPath = base_path;
+ filePath = "";
+ disableExact = true;
+ } else if (std::filesystem::is_regular_file(base_path)) {
+ dirPath = "";
+ filePath = base_path;
+ disableExact = exact;
+ } else {
+ VERROR("Invalid file path: %s", filePath.c_str());
+ filePath = "";
+ dirPath = "";
+ }
}
~StaticHitConfig() { TSContDestroy(cont); }
+ std::string_view
+ makePath(TSHttpTxn txnp, std::string &output) const
+ {
+ if (!dirPath.empty()) {
+ TSMBuffer reqp;
+ TSMLoc hdr_loc = nullptr, url_loc = nullptr;
+
+ if (TS_SUCCESS == TSHttpTxnClientReqGet(txnp, &reqp, &hdr_loc)) {
+ if (TS_SUCCESS == TSHttpHdrUrlGet(reqp, hdr_loc, &url_loc)) {
+ int path_len = 0;
+ auto path = TSUrlPathGet(reqp, url_loc, &path_len);
+
+ std::filesystem::path requested_file_path(
+ std::filesystem::weakly_canonical(dirPath / std::string_view{path, static_cast<size_t>(path_len)}));
+
+ TSHandleMLocRelease(reqp, hdr_loc, url_loc);
+ TSHandleMLocRelease(reqp, TS_NULL_MLOC, hdr_loc);
+
+ if (std::equal(dirPath.begin(), dirPath.end(), requested_file_path.begin()) &&
Review Comment:
`std::string::starts_with` would be clearer.
https://en.cppreference.com/w/cpp/string/basic_string/starts_with
--
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] SolidWallOfCode commented on a diff in pull request #9107: Adds support for serving statichit content out of a directory
Posted by GitBox <gi...@apache.org>.
SolidWallOfCode commented on code in PR #9107:
URL: https://github.com/apache/trafficserver/pull/9107#discussion_r993673119
##########
plugins/experimental/statichit/statichit.cc:
##########
@@ -62,21 +63,80 @@ static int StaticHitInterceptHook(TSCont contp, TSEvent event, void *edata);
static int StaticHitTxnHook(TSCont contp, TSEvent event, void *edata);
struct StaticHitConfig {
- explicit StaticHitConfig(const std::string &filePath, const std::string &mimeType, bool disableExact)
- : filePath(filePath), mimeType(mimeType), disableExact(disableExact)
+ explicit StaticHitConfig(const std::string &fileOrDir, const std::string &mimeType, bool exact) : mimeType(mimeType)
{
+ std::filesystem::path base_path;
+
+ if (fileOrDir.find('/') != 0) {
+ base_path = std::filesystem::weakly_canonical(std::string(TSConfigDirGet()) + "/" + fileOrDir);
+ } else {
+ base_path = std::filesystem::weakly_canonical(fileOrDir);
+ }
+
+ if (std::filesystem::is_directory(base_path)) {
+ dirPath = base_path;
+ filePath = "";
+ disableExact = true;
+ } else if (std::filesystem::is_regular_file(base_path)) {
+ dirPath = "";
+ filePath = base_path;
+ disableExact = exact;
+ } else {
+ VERROR("Invalid file path: %s", filePath.c_str());
+ filePath = "";
+ dirPath = "";
+ }
}
~StaticHitConfig() { TSContDestroy(cont); }
+ std::string_view
+ makePath(TSHttpTxn txnp, std::string &output) const
+ {
+ if (!dirPath.empty()) {
Review Comment:
If `dirPath` is a member variable, you need to either use leading underscores or `this->dirPath` so it's clear where the value is from.
--
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] ywkaras commented on a diff in pull request #9107: Adds support for serving statichit content out of a directory
Posted by GitBox <gi...@apache.org>.
ywkaras commented on code in PR #9107:
URL: https://github.com/apache/trafficserver/pull/9107#discussion_r1018436713
##########
plugins/experimental/statichit/statichit.cc:
##########
@@ -166,12 +220,16 @@ struct StaticHitRequest {
std::string mimeType;
static StaticHitRequest *
- createStaticHitRequest(StaticHitConfig *tc)
+ createStaticHitRequest(StaticHitConfig *tc, TSHttpTxn txn)
{
StaticHitRequest *shr = new StaticHitRequest;
std::ifstream ifstr;
+ std::string output;
+ std::string_view filePath = tc->makePath(txn, output);
- ifstr.open(tc->filePath);
+ VDEBUG("Requested file path: %s", filePath.data());
Review Comment:
You may spend centuries in Purgatory for assuming an std::string_view is null terminated. Especially when you could just have makePath() return `std::string const &`.
--
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] zwoop commented on pull request #9107: Adds support for serving statichit content out of a directory
Posted by GitBox <gi...@apache.org>.
zwoop commented on PR #9107:
URL: https://github.com/apache/trafficserver/pull/9107#issuecomment-1258938934
As a side note for the curious reader, if this is acceptable by the community, I'd like to add support for the HTTPD `mime.types` file, allowing the mime type to be defined by the suffix of the filename.
--
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] ywkaras commented on a diff in pull request #9107: Adds support for serving statichit content out of a directory
Posted by GitBox <gi...@apache.org>.
ywkaras commented on code in PR #9107:
URL: https://github.com/apache/trafficserver/pull/9107#discussion_r989460859
##########
plugins/experimental/statichit/statichit.cc:
##########
@@ -62,21 +63,80 @@ static int StaticHitInterceptHook(TSCont contp, TSEvent event, void *edata);
static int StaticHitTxnHook(TSCont contp, TSEvent event, void *edata);
struct StaticHitConfig {
- explicit StaticHitConfig(const std::string &filePath, const std::string &mimeType, bool disableExact)
- : filePath(filePath), mimeType(mimeType), disableExact(disableExact)
+ explicit StaticHitConfig(const std::string &fileOrDir, const std::string &mimeType, bool exact) : mimeType(mimeType)
{
+ std::filesystem::path base_path;
+
+ if (fileOrDir.find('/') != 0) {
+ base_path = std::filesystem::weakly_canonical(std::string(TSConfigDirGet()) + "/" + fileOrDir);
+ } else {
+ base_path = std::filesystem::weakly_canonical(fileOrDir);
+ }
+
+ if (std::filesystem::is_directory(base_path)) {
+ dirPath = base_path;
+ filePath = "";
+ disableExact = true;
+ } else if (std::filesystem::is_regular_file(base_path)) {
+ dirPath = "";
+ filePath = base_path;
+ disableExact = exact;
+ } else {
+ VERROR("Invalid file path: %s", filePath.c_str());
+ filePath = "";
+ dirPath = "";
+ }
}
~StaticHitConfig() { TSContDestroy(cont); }
+ std::string_view
+ makePath(TSHttpTxn txnp, std::string &output) const
Review Comment:
Why not just have this return std::string and eliminate the output parameter? C++98 is so five minutes ago.
--
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] zwoop commented on a diff in pull request #9107: Adds support for serving statichit content out of a directory
Posted by GitBox <gi...@apache.org>.
zwoop commented on code in PR #9107:
URL: https://github.com/apache/trafficserver/pull/9107#discussion_r995151866
##########
plugins/experimental/statichit/statichit.cc:
##########
@@ -62,21 +63,80 @@ static int StaticHitInterceptHook(TSCont contp, TSEvent event, void *edata);
static int StaticHitTxnHook(TSCont contp, TSEvent event, void *edata);
struct StaticHitConfig {
- explicit StaticHitConfig(const std::string &filePath, const std::string &mimeType, bool disableExact)
- : filePath(filePath), mimeType(mimeType), disableExact(disableExact)
+ explicit StaticHitConfig(const std::string &fileOrDir, const std::string &mimeType, bool exact) : mimeType(mimeType)
{
+ std::filesystem::path base_path;
+
+ if (fileOrDir.find('/') != 0) {
+ base_path = std::filesystem::weakly_canonical(std::string(TSConfigDirGet()) + "/" + fileOrDir);
+ } else {
+ base_path = std::filesystem::weakly_canonical(fileOrDir);
+ }
+
+ if (std::filesystem::is_directory(base_path)) {
+ dirPath = base_path;
+ filePath = "";
+ disableExact = true;
+ } else if (std::filesystem::is_regular_file(base_path)) {
+ dirPath = "";
+ filePath = base_path;
+ disableExact = exact;
+ } else {
+ VERROR("Invalid file path: %s", filePath.c_str());
+ filePath = "";
+ dirPath = "";
+ }
Review Comment:
Meh
--
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] zwoop commented on a diff in pull request #9107: Adds support for serving statichit content out of a directory
Posted by GitBox <gi...@apache.org>.
zwoop commented on code in PR #9107:
URL: https://github.com/apache/trafficserver/pull/9107#discussion_r995150959
##########
plugins/experimental/statichit/statichit.cc:
##########
@@ -62,21 +63,80 @@ static int StaticHitInterceptHook(TSCont contp, TSEvent event, void *edata);
static int StaticHitTxnHook(TSCont contp, TSEvent event, void *edata);
struct StaticHitConfig {
- explicit StaticHitConfig(const std::string &filePath, const std::string &mimeType, bool disableExact)
- : filePath(filePath), mimeType(mimeType), disableExact(disableExact)
+ explicit StaticHitConfig(const std::string &fileOrDir, const std::string &mimeType, bool exact) : mimeType(mimeType)
{
+ std::filesystem::path base_path;
+
+ if (fileOrDir.find('/') != 0) {
+ base_path = std::filesystem::weakly_canonical(std::string(TSConfigDirGet()) + "/" + fileOrDir);
+ } else {
+ base_path = std::filesystem::weakly_canonical(fileOrDir);
+ }
Review Comment:
Fine ...
--
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] SolidWallOfCode commented on a diff in pull request #9107: Adds support for serving statichit content out of a directory
Posted by GitBox <gi...@apache.org>.
SolidWallOfCode commented on code in PR #9107:
URL: https://github.com/apache/trafficserver/pull/9107#discussion_r993655261
##########
plugins/experimental/statichit/statichit.cc:
##########
@@ -62,21 +63,80 @@ static int StaticHitInterceptHook(TSCont contp, TSEvent event, void *edata);
static int StaticHitTxnHook(TSCont contp, TSEvent event, void *edata);
struct StaticHitConfig {
- explicit StaticHitConfig(const std::string &filePath, const std::string &mimeType, bool disableExact)
- : filePath(filePath), mimeType(mimeType), disableExact(disableExact)
+ explicit StaticHitConfig(const std::string &fileOrDir, const std::string &mimeType, bool exact) : mimeType(mimeType)
Review Comment:
Use `explicit` only on constructors that have exactly one parameter.
--
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