Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HPCC-29721 setReplicateDir fails if config dir ends with path seperator #17974

Merged
merged 1 commit into from
Nov 9, 2023
Merged

HPCC-29721 setReplicateDir fails if config dir ends with path seperator #17974

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 41 additions & 9 deletions dali/base/dafdesc.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1721,7 +1721,10 @@ class CFileDescriptor: public CFileDescriptorBase, implements ISuperFileDescrip
if (!sc)
sc = getPathSepChar(dirname);
StringBuffer tmp;
tmp.append(queryBaseDirectory(grp_unknown, 0, SepCharBaseOs(sc))).append(sc).append(s);
tmp.append(queryBaseDirectory(grp_unknown, 0, SepCharBaseOs(sc)));
if (sc != tmp.charAt(tmp.length()-1))
tmp.append(sc);
tmp.append(s);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NB: this is an incidental fix, observed while testing. A relative path and a config dir suffixed with a path separator caused a double path separator to be inserted.
I think this only ever happened (relative lfn dir path) via the unittests.

directory.set(tmp.str());
}
else
Expand Down Expand Up @@ -2934,23 +2937,52 @@ bool setReplicateDir(const char *dir,StringBuffer &out,bool isrep,const char *ba
unsigned count = 0;
unsigned i;
for (i=0;d[i]&&dir[i]&&(d[i]==dir[i]);i++)
if (isPathSepChar(dir[i])) {
{
if (isPathSepChar(dir[i]))
{
match = i;
count++;
}
}
const char *r = repDir?repDir:queryBaseDirectory(grp_unknown, isrep ? 1 : 0,os);
if (d[i]==0) {
if ((dir[i]==0)||isPathSepChar(dir[i])) {
out.append(r).append(dir+i);
if (d[i]==0)
{
if (dir[i]==0)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Worth adding a comment to clarify the cases. E.g.,
//directory is an exact match for the current base, so return replica unmodified.

{
// dir was an exact match for the base directory, return replica directory in output
out.append(r);
return true;
}
else if (isPathSepChar(dir[i]))
{
// dir matched the prefix of the base directory and the remaining part leads with a path separator
// replace the base directory with the replica directory in the output, and append the remaining part of dir
out.append(r);
if (isPathSepChar(out.charAt(out.length()-1))) // if r has trailing path separator, skip the leading one in dir
i++;
out.append(dir+i);
return true;
}
else if (i>0 && isPathSepChar(d[i-1])) // implies dir[i-1] is also a pathsepchar
{
// dir matched the prefix of the base directory, including the trailing path separator
// replace the base directory with the replica directory in the output, and append the remaining part of dir
out.append(r);
addPathSepChar(out); // NB: this is an ensure-has-trailing-path-separator
out.append(dir+i); // NB: dir+i is beyond a path separator in dir
return true;
}
}
else if (count) { // this is a bit of a kludge to handle roxie backup
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you mean to delete this code? If so the count/match variables are no longer needed.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hm, I did not, well spotted.

else if (count) // this is a bit of a kludge to handle roxie backup
{
const char *s = r;
const char *b = s;
while (s&&*s) {
if (isPathSepChar(*s)) {
if (--count==0) {
while (s&&*s)
{
if (isPathSepChar(*s))
{
if (--count==0)
{
out.append(s-b,b).append(dir+match);
return true;
}
Expand Down