@abh3 requested changes on this pull request.

In src/XrdCms/XrdCmsRedirLocal.cc:

> @@ -85,6 +85,11 @@ void XrdCmsRedirLocal::loadConfig(const char *filename) {
         httpRedirect = false;
       }
     }
+    // search for localroot,
+    // which manually sets localroot to prepend
+    else if (strcmp(word, "xrdcmsredirlocal.localroot") == 0){
+      localroot = std::string(Config.GetWord(true));//to lower case

Are you sure you want to restrict localroot to be nothing but lower case? Also, there should be a check that it starts with a slash since root paths should be absolute to avoid unpleasant surprises.

In src/XrdCms/XrdCmsRedirLocal.cc:

> @@ -113,6 +118,31 @@ int XrdCmsRedirLocal::Locate(XrdOucErrInfo &Resp, const char *path, int flags,
                         XrdOucEnv *EnvInfo) {
   int rcode = 0;
   if (nativeCmsFinder) {
+    // check if path contains localroot to know if potential redirection loop
+    // is happening. If yes, remove localroot from path, then rerun default
+    // locate with regular path and return to client
+    // localroot must be larger than 1 to avoid always being triggered by "/"
+    if (localroot.size() > 1 && strncmp(path, localroot.c_str(), localroot.size()) == 0)
+    {
+      // now check if localhost was tried before, to make sure we're handling
+      // the redirection loop
+      int param = 0; // need it to get Env
+      std::string envInfo(EnvInfo->Env(param)); // get already tried hosts
+      std::string searchPattern = "&tried=localhost"; //search for this pattern

Note that if "tried=" is the first token after the question mark it will not (typically) have a leading '&'. This doesn't take care of that.

In src/XrdCms/XrdCmsRedirLocal.cc:

> @@ -161,13 +191,8 @@ int XrdCmsRedirLocal::Locate(XrdOucErrInfo &Resp, const char *path, int flags,
         return rcode;
     }
     // passed all checks, now to actual business
-    // build a buffer with a total acceptable buffer length,
-    // which must have a larger capacity than localroot and filename concatenated
-    int rc = 0;
-    int maxPathLength = 4096;
-    char *buff = new char[maxPathLength];
-    // prepend oss.localroot
-    std::string ppath = "file://" + std::string(theSS->Lfn2Pfn(path, buff, maxPathLength, rc));
+    // prepend manually configured localroot

This doesn't look like it is backward compatible with existing config files. What about those that never specified the new localroot directive and rely on the oss.localroot. Those sites would quickly find that the changes here make local redirection no longer work. That has to be corrected.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.

Use REPLY-ALL to reply to list

To unsubscribe from the XROOTD-DEV list, click the following link:
https://listserv.slac.stanford.edu/cgi-bin/wa?SUBED1=XROOTD-DEV&A=1