Copilot commented on code in PR #12494:
URL: https://github.com/apache/trafficserver/pull/12494#discussion_r2342008614


##########
plugins/esi/combo_handler.cc:
##########
@@ -45,6 +45,18 @@
 using namespace std;
 using namespace EsiLib;
 using namespace swoc::literals;
+#include "utils.h"
+
+/** Extracts a URL string from buffer and location objects.
+ *
+ * @param[in] bufp The buffer containing the URL data.
+ * @param[in] url_loc The memory location pointing to the URL within the
+ *   buffer.
+ *
+ * @return The URL. Returns "(unknown)" if the URL cannot be retrieved due to
+ *   API failures or invalid parameters.
+ */
+// Use shared helper from url_utils.h

Review Comment:
   The comment mentions "url_utils.h" but the actual file is "utils.h". The 
comment should reference the correct header file.
   ```suggestion
   // Use shared helper from utils.h
   ```



##########
plugins/esi/esi.cc:
##########
@@ -291,22 +293,25 @@ ContData::getClientState()
     TSMBuffer bufp;
     TSMLoc    url_loc;
     if (TSHttpTxnPristineUrlGet(txnp, &bufp, &url_loc) != TS_SUCCESS) {
-      TSError("[esi][%s] Error while retrieving hdr url", __FUNCTION__);
+      TSError("[esi][%s] Error while retrieving hdr url for URL [%s]", 
__FUNCTION__, request_url.c_str());
       return;
     }
     if (url_loc) {
-      if (request_url) {
-        TSfree(request_url);
+      int   length;
+      char *url_string = TSUrlStringGet(bufp, url_loc, &length);
+      if (url_string) {
+        request_url.assign(url_string, length);
+        TSfree(url_string);
+      } else {
+        request_url = "(unknown)";

Review Comment:
   The fallback value "(unknown)" is duplicated throughout the codebase. 
Consider defining this as a constant to ensure consistency and make future 
changes easier.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to