HPCC-22124 File services permits and defaults to copying a super index into a invalid single index with multiple TLK's

Add Superfile check for a simple logical file copy.

Add supercopy.ecl to Regression Suite

Remove 'foreignfdesc'

Signed-off-by: Attila Vamos <attila.vamos@gmail.com>

dali/dfu/dfurun.cpp

@@ -1143,8 +1143,8 @@ public:
         bool multiclustermerge = false;
         bool useserverreplicate = false;
         Owned<IFileDescriptor> multifdesc;
-        Owned<IFileDescriptor> foreignfdesc;
         Owned<IFileDescriptor> auxfdesc;        // used for multicopy
+        Owned<IFileDescriptor> srcFdesc;
         DFUstate  finalstate = DFUstate_finished;
         try {
             DFUcmd cmd = wu->getCommand();
@@ -1199,13 +1199,21 @@ public:
                     if (!tmp.length())
                         throw MakeStringException(-1,"Source file not specified");
                     foreigncopy = false;
-                    if ((cmd==DFUcmd_copy)||multiclustermerge) {
+                    if ((cmd==DFUcmd_copy)||multiclustermerge)
+                    {
                         foreigndalinode.setown(getForeignDali(source));
                         foreigncopy = foreigndalinode.get()!=NULL;
-                        if (foreigncopy) {
+                        if (foreigncopy)
+                        {
+                            CDfsLogicalFileName lfn;
+                            lfn.set(tmp);
+                            lfn.setForeign(foreigndalinode->endpoint(),true);
+                            tmp = lfn.get();
+
                             StringBuffer fu;
                             StringBuffer fp;
-                            if (source->getForeignUser(fu,fp)) {
+                            if (source->getForeignUser(fu,fp))
+                            {
                                  foreignuserdesc.setown(createUserDescriptor());
                                  foreignuserdesc->set(fu.str(),fp.str());
                             }
@@ -1213,46 +1221,44 @@ public:
                                 foreignuserdesc.set(userdesc);
                         }
                     }
-                    if (foreigncopy) {
-                        foreignfdesc.setown(queryDistributedFileDirectory().getFileDescriptor(tmp.str(),foreignuserdesc,foreigndalinode));
-                        if (!foreignfdesc) {
-                            StringBuffer s;
-                            throw MakeStringException(-1,"Source file %s could not be found in Dali %s",tmp.str(),foreigndalinode->endpoint().getUrlStr(s).str());
-                        }
-                        kind.set(foreignfdesc->queryProperties().queryProp("@kind"));
-                        oldRoxiePrefix.set(foreignfdesc->queryProperties().queryProp("@roxiePrefix"));
-                        iskey = strsame("key", kind);
-                        if (destination->getWrap()||iskey)
-                            destination->setNumPartsOverride(foreignfdesc->numParts());
-                        if (options->getPush()) {// need to set ftslave location
+                    srcFile.setown(fdir.lookup(tmp.str(),userdesc,
+                            (cmd==DFUcmd_move)||(cmd==DFUcmd_rename)||((cmd==DFUcmd_copy)&&multiclusterinsert)));
+
+                    if (!srcFile)
+                        throw MakeStringException(-1,"Source file %s could not be found",tmp.str());
+
+                    srcName.set(tmp);
+                    srcFdesc.setown(srcFile->getFileDescriptor());
+                    iskey = isFileKey(srcFile);
+                    if ((cmd==DFUcmd_copy) && (srcFile->querySuperFile() != nullptr) && iskey)
+                        throwError1(DFTERR_InvalidSuperindexCopy, srcName.str());
+
+                    oldRoxiePrefix.set(srcFile->queryAttributes().queryProp("@roxiePrefix"));
+                    kind.set(srcFile->queryAttributes().queryProp("@kind"));
+
+                    // keys default wrap for copy
+                    if (destination->getWrap()||(iskey&&(cmd==DFUcmd_copy)))
+                        destination->setNumPartsOverride(srcFile->numParts());
+
+                    if (options->getSubfileCopy())
+                        opttree->setPropBool("@compress",srcFile->isCompressed());
+
+                    if (foreigncopy)
+                    {
+                        if (options->getPush())
+                        {
+                            // need to set ftslave location
                             StringBuffer progpath;
                             StringBuffer workdir;
-                            INode *n = foreignfdesc->queryNode(0);
-                            if (n&&getRemoteRunInfo("FTSlaveProcess", "ftslave", NULL, n->endpoint(), progpath, workdir, foreigndalinode, 1000*60*5)) {
+                            INode *n = srcFdesc->queryNode(0);
+                            if (n&&getRemoteRunInfo("FTSlaveProcess", "ftslave", NULL, n->endpoint(), progpath, workdir, foreigndalinode, 1000*60*5))
+                            {
                                 opttree->setProp("@slave",progpath.str());
                             }
                         }
-                        if (options->getSubfileCopy())
-                            opttree->setPropBool("@compress",foreignfdesc->isCompressed());
-                    }
-                    else {
-                        srcFile.setown(fdir.lookup(tmp.str(),userdesc,
-                              (cmd==DFUcmd_move)||(cmd==DFUcmd_rename)||((cmd==DFUcmd_copy)&&multiclusterinsert)));
-                        if (!srcFile)
-                            throw MakeStringException(-1,"Source file %s could not be found",tmp.str());
-                        oldRoxiePrefix.set(srcFile->queryAttributes().queryProp("@roxiePrefix"));
-                        iskey = isFileKey(srcFile);
-                        kind.set(srcFile->queryAttributes().queryProp("@kind"));
-                        if (destination->getWrap()||(iskey&&(cmd==DFUcmd_copy)))    // keys default wrap for copy
-                            destination->setNumPartsOverride(srcFile->numParts());
-                        if (options->getSubfileCopy())
-                            opttree->setPropBool("@compress",srcFile->isCompressed());
-                    }
-                    if (destination->getMultiCopy()&&!destination->getWrap()) {
-                        Owned<IFileDescriptor> tmpfd = foreigncopy?foreignfdesc.getLink():srcFile->getFileDescriptor();
-                        auxfdesc.setown(createMultiCopyFileDescriptor(tmpfd,destination->getNumParts(0)));
                     }
-                    srcName.set(tmp.str());
+                    if (destination->getMultiCopy()&&!destination->getWrap())
+                        auxfdesc.setown(createMultiCopyFileDescriptor(srcFdesc,destination->getNumParts(0)));
                 }
                 break;
             }
@@ -1328,11 +1334,9 @@ public:
                                 bool dstCompressed = false;
                                 if (srcFile)
                                     dstCompressed = srcFile->isCompressed();
-                                else
-                                {
-                                    IFileDescriptor * srcDesc = (auxfdesc.get() ? auxfdesc.get() : foreignfdesc.get());
-                                    dstCompressed = srcDesc && srcDesc->isCompressed();
-                                }
+                                else if (auxfdesc)
+                                    dstCompressed = auxfdesc->isCompressed();
+
                                 if (dstCompressed)
                                     fdesc->queryProperties().setPropBool("@blockCompressed",true);
                             }
@@ -1405,28 +1409,27 @@ public:
             case DFUcmd_copymerge:
             case DFUcmd_copy:
                 {
-                    if (!replicating) {
+                    if (!replicating)
+                    {
                         Owned<IFileDescriptor> patchf;
                         Owned<IFileDescriptor> olddstf;
-                        if (diffNameSrc.get()||diffNameDst.get()) {
-                            Owned<IFileDescriptor> newf;
+                        if (diffNameSrc.get()||diffNameDst.get())
+                        {
                             Owned<IFileDescriptor> oldf;
-                            if (foreigncopy)
-                                newf.set(foreignfdesc);
-                            else
-                                newf.setown(srcFile->getFileDescriptor());
                             oldf.setown(queryDistributedFileDirectory().getFileDescriptor(diffNameSrc,foreigncopy?foreignuserdesc:userdesc,foreigncopy?foreigndalinode:NULL));
-                            if (!oldf.get()) {
+                            if (!oldf.get())
+                            {
                                 StringBuffer s;
                                 throw MakeStringException(-1,"Old key file %s could not be found in source",diffNameSrc.get());
                             }
                             olddstf.setown(queryDistributedFileDirectory().getFileDescriptor(diffNameDst,userdesc,NULL));
-                            if (!olddstf.get()) {
+                            if (!olddstf.get())
+                            {
                                 StringBuffer s;
                                 throw MakeStringException(-1,"Old key file %s could not be found in destination",diffNameDst.get());
                             }
                             patchf.setown(createFileDescriptor());
-                            doKeyDiff(oldf,newf,patchf);
+                            doKeyDiff(oldf,srcFdesc,patchf);
                         }
                         runningconn.setown(setRunning(runningpath.str()));
                         bool needrep = options->getReplicate();
@@ -1447,7 +1450,7 @@ public:
                         if (needrep)
                             feedback.repmode=cProgressReporter::REPbefore;
                         if (foreigncopy)
-                            checkPhysicalFilePermissions(foreignfdesc,userdesc,false);
+                            checkPhysicalFilePermissions(srcFdesc,userdesc,false);
                         if (patchf) { // patch assumes only 1 cluster
                             // need to create dstpatchf
                             StringBuffer gname;
@@ -1486,7 +1489,7 @@ public:
                         }
                         else if (foreigncopy||auxfdesc)
                         {
-                            IFileDescriptor * srcDesc = (auxfdesc.get() ? auxfdesc.get() : foreignfdesc.get());
+                            IFileDescriptor * srcDesc = (auxfdesc.get() ? auxfdesc.get() : srcFdesc.get());
                             fsys.import(srcDesc, dstFile, recovery, recoveryconn, filter, opttree, &feedback, &abortnotify, dfuwuid);
 
                             if (!abortnotify.abortRequested())
@@ -1510,7 +1513,7 @@ public:
                                         multifdesc->getClusterGroupName(0,cname,&queryNamedGroupStore());
                                     (multiclusterinsert?srcFile:dstFile)->addCluster(cname.str(),multifdesc->queryPartDiskMapping(0));
                                 }
-                                Audit(multiclusterinsert?"COPY":"COPYMERGE",userdesc,srcFile?srcFile->queryLogicalName():NULL,dstName.get());
+                                Audit(multiclusterinsert?"COPY":"COPYMERGE",userdesc,srcFile?srcName.str():NULL,dstName.get());
                             }
                         }
                         else {
@@ -1520,7 +1523,7 @@ public:
                                     replicating = true;
                                 else
                                     dstFile->attach(dstName.get(),userdesc);
-                                Audit("COPY",userdesc,srcFile?srcFile->queryLogicalName():NULL,dstName.get());
+                                Audit("COPY",userdesc,srcFile?srcName.str():NULL,dstName.get());
                             }
                         }
                         runningconn.clear();
@@ -1548,7 +1551,7 @@ public:
                     runningconn.clear();
                     if (!abortnotify.abortRequested()) {
                         dstFile->attach(dstName.get(),userdesc);
-                        Audit("MOVE",userdesc,srcFile?srcFile->queryLogicalName():NULL,dstName.get());
+                        Audit("MOVE",userdesc,srcFile?srcName.str():NULL,dstName.get());
                     }
                 }
                 break;
@@ -1570,6 +1573,7 @@ public:
                         newfile.clear();
                         StringBuffer fromname(srcName);
                         srcFile.clear();
+                        srcFdesc.clear();
                         queryDistributedFileDirectory().renamePhysical(fromname.str(),toname.str(),userdesc,NULL);
                         StringBuffer timetaken;
                         timetaken.appendf("%dms",msTick()-start);
@@ -1600,12 +1604,11 @@ public:
                         break;
                     }
                     setFileRepeatOptions(*srcFile,repcluster.str(),repeatlast,onlyrepeated);
-                    Owned<IFileDescriptor> fdesc = srcFile->getFileDescriptor();
-                    fdesc->ensureReplicate();
-                    fsys.replicate(fdesc.get(), mode, recovery, recoveryconn, filter, opttree, &feedback, &abortnotify, dfuwuid);
+                    srcFdesc->ensureReplicate();
+                    fsys.replicate(srcFdesc.get(), mode, recovery, recoveryconn, filter, opttree, &feedback, &abortnotify, dfuwuid);
                     runningconn.clear();
                     if (!abortnotify.abortRequested()) {
-                        Audit("REPLICATE",userdesc,srcFile?srcFile->queryLogicalName():NULL,NULL);
+                        Audit("REPLICATE",userdesc,srcFile?srcName.str():NULL,NULL);
                         // srcFile->queryPartDiskMapping(0).maxCopies = 2;   // ** TBD ?
                     }
                 }
@@ -1655,7 +1658,7 @@ public:
                     checkSourceTarget(fdesc);
                     fsys.exportFile(srcFile, fdesc, recovery, recoveryconn, filter, opttree, &feedback, &abortnotify, dfuwuid);
                     if (!abortnotify.abortRequested()) {
-                        Audit("EXPORT",userdesc,srcFile?srcFile->queryLogicalName():NULL,NULL);
+                        Audit("EXPORT",userdesc,srcFile?srcName.str():NULL,NULL);
                     }
                     runningconn.clear();
                 }

dali/ft/fterror.hpp

@@ -84,6 +84,7 @@
 #define DFTERR_InvalidFilePath                  8111
 #define DFTERR_NoMatchingDropzonePath           8112
 #define DFTERR_LocalhostAddressUsed             8113
+#define DFTERR_InvalidSuperindexCopy            8114
 
 //Internal errors
 #define DFTERR_UnknownFormatType                8190
@@ -155,6 +156,7 @@
 #define DFTERR_InvalidFilePath_Text             "Invalid file path: '%s'. For security reason it is forbidden to use '%s' or '%s' to build a path!"
 #define DFTERR_NoMatchingDropzonePath_Text      "No Drop Zone on '%s' configured at '%s'."
 #define DFTERR_LocalhostAddressUsed_Text        "Localhost address used in remote file name: '%s'"
+#define DFTERR_InvalidSuperindexCopy_Text       "Source file %s is a super index file but copying as a simple logical file"
 
 #define DFTERR_UnknownFormatType_Text           "INTERNAL: Save unknown format type"
 #define DFTERR_OutputOffsetMismatch_Text        "INTERNAL: Output offset does not match expected (%" I64F "d expected %" I64F "d) at %s of block %d"

testing/regress/ecl/key/supercopy.xml

@@ -0,0 +1,20 @@
+<Dataset name='Result 1'>
+</Dataset>
+<Dataset name='Result 2'>
+</Dataset>
+<Dataset name='Result 3'>
+</Dataset>
+<Dataset name='Result 4'>
+</Dataset>
+<Dataset name='Result 5'>
+ <Row><result>Copy superfile as logical file: Pass</result></Row>
+</Dataset>
+<Dataset name='Result 6'>
+ <Row><result>Copy superfile as superfle: Pass</result></Row>
+</Dataset>
+<Dataset name='Result 7'>
+ <Row><result>Copy superindex as logical file: Fail</result></Row>
+</Dataset>
+<Dataset name='Result 8'>
+ <Row><result>Copy superindex as superfle: Pass</result></Row>
+</Dataset>

testing/regress/ecl/supercopy.ecl

@@ -0,0 +1,203 @@
+/*##############################################################################
+
+    HPCC SYSTEMS software Copyright (C) 2012 HPCC Systems®.
+
+    Licensed under the Apache License, Version 2.0 (the "License");
+    you may not use this file except in compliance with the License.
+    You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+    Unless required by applicable law or agreed to in writing, software
+    distributed under the License is distributed on an "AS IS" BASIS,
+    WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+    See the License for the specific language governing permissions and
+    limitations under the License.
+############################################################################## */
+
+#onwarning(4523, ignore);
+
+//nothor
+//class=superfile
+
+import Std.File AS FileServices;
+import $.setup;
+prefix := setup.Files(false, false).QueryFilePrefix;
+// Super Fiel and Key Copy regression test
+
+unsigned VERBOSE := 0;
+
+AlbumRecordDef     := RECORD
+UNSIGNED                        Id;
+STRING80            Title;
+STRING64            Artist;
+UNSIGNED            Tracks;
+UNSIGNED            Mins;
+UNSIGNED            Secs;
+UNSIGNED            Player;
+UNSIGNED            Position;
+                      END;
+
+ds := DATASET(
+[
+  {251,'Kingsize','The Boo Radleys',14,63,55,0,252}
+], AlbumRecordDef );
+
+
+albumTableName := prefix + 'albums.d00';
+albumTable := DATASET(albumTableName,{AlbumRecordDef,UNSIGNED8 filepos {virtual(fileposition)}},FLAT);
+
+albumIndex1Name := prefix + 'albums1.key';
+albumIndex1 := INDEX(albumTable(Tracks<=4),{ Artist, Title, filepos }, albumIndex1Name);
+
+albumIndex2Name := prefix + 'albums2.key';
+albumIndex2 := INDEX(albumTable((Tracks>4)AND(Tracks<8)),{ Artist, Title, filepos }, albumIndex2Name);
+
+albumIndexName := prefix + 'albums.key';
+albumIndex  := INDEX(albumTable,{ Artist, Title, filepos }, albumIndexName);
+
+superFileName := prefix + 'superalbums';
+superFileCopyName := prefix + 'superalbums_copy';
+
+superIndexName := prefix + 'superalbums.key';
+superIndexCopyName := prefix + 'superalbums.key_copy';
+
+
+rec := RECORD
+  string sourceFileName;
+  string destFileName;
+  string destCluster;
+  boolean asSuperfile;
+  string result;
+  string msg;
+end;
+
+// Copy
+rec supercopy(rec l) := TRANSFORM
+  SELF.msg := FileServices.fCopy(
+                       sourceLogicalName := l.sourceFileName
+                      ,destinationGroup := l.destCluster
+                      ,destinationLogicalName := l.destFileName
+                      ,ASSUPERFILE := l.asSuperfile
+                      ,ALLOWOVERWRITE := True
+                      );
+  SELF.result := l.result + ' Pass';
+  SELF.sourceFileName := l.sourceFileName;
+  SELF.destFileName := l.destFileName;
+  SELF.destCluster := l.destCluster;
+  SELF.asSuperfile := l.asSuperfile;
+end;
+
+dst1 := NOFOLD(DATASET([{superFileName, superFileCopyName, 'mythor', False, 'Copy superfile as logical file:', ''}], rec));
+p1 := PROJECT(NOFOLD(dst1), supercopy(LEFT));
+c1 := CATCH(NOFOLD(p1), ONFAIL(TRANSFORM(rec,
+                                 SELF.result := 'Copy superfile as logical file: Fail',
+                                 SELF.msg := FAILMESSAGE,
+                                 SELF.sourceFileName := superFileName,
+                                 SELF.destFileName := superFileCopyName,
+                                 SELF.destCluster := 'mythor',
+                                 SELF.asSuperfile := False
+                                )));
+#if (VERBOSE = 1)
+   supercopyOut := output(c1);
+#else
+   supercopyOut := output(c1, {result});
+#end
+
+
+dst2 := NOFOLD(DATASET([{superFileName, superFileCopyName, 'mythor', True, 'Copy superfile as superfle:', ''}], rec));
+p2 := PROJECT(NOFOLD(dst2), supercopy(LEFT));
+c2 := CATCH(NOFOLD(p2), ONFAIL(TRANSFORM(rec,
+                                 SELF.result := 'Copy superfile as superfle: Fail',
+                                 SELF.msg := FAILMESSAGE,
+                                 SELF.sourceFileName := superFileName,
+                                 SELF.destFileName := superFileCopyName,
+                                 SELF.destCluster := 'mythor',
+                                 SELF.asSuperfile := True
+                                )));
+#if (VERBOSE = 1)
+   supercopyOut2 := output(c2);
+#else
+   supercopyOut2 := output(c2, {result});
+#end
+
+
+
+dst3 := NOFOLD(DATASET([{superIndexName, superIndexCopyName, 'myroxie', False, 'Copy superindex as logical file:', ''}], rec));
+p3 := PROJECT(NOFOLD(dst3), supercopy(LEFT));
+c3 := CATCH(NOFOLD(p3), ONFAIL(TRANSFORM(rec,
+                                 SELF.result := 'Copy superindex as logical file: Fail',
+                                 SELF.msg := FAILMESSAGE,
+                                 SELF.sourceFileName := superIndexName,
+                                 SELF.destFileName := superIndexName + '_copy',
+                                 SELF.destCluster := 'myroxie',
+                                 SELF.asSuperfile := False
+                                )));
+#if (VERBOSE = 1)
+   supercopyOut3 := output(c3);
+#else
+   supercopyOut3 := output(c3, {result});
+#end
+
+
+dst4 := NOFOLD(DATASET([{superIndexName, superIndexCopyName, 'mythor', True, 'Copy superindex as superfle:', ''}], rec));
+p4 := PROJECT(NOFOLD(dst4), supercopy(LEFT));
+c4 := CATCH(NOFOLD(p4), ONFAIL(TRANSFORM(rec,
+                                 SELF.result := 'Copy superindex as superfle: Fail',
+                                 SELF.msg := FAILMESSAGE,
+                                 SELF.sourceFileName := superIndexName,
+                                 SELF.destFileName := superIndexCopyName,
+                                 SELF.destCluster := 'mythor',
+                                 SELF.asSuperfile := True
+                                )));
+#if (VERBOSE = 1)
+   supercopyOut4 := output(c4);
+#else
+   supercopyOut4 := output(c4, {result});
+#end
+
+SEQUENTIAL(
+
+    // Create a logical file and add it to a Superfile
+    OUTPUT(ds,,albumTableName,OVERWRITE),
+
+    FileServices.CreateSuperFile(superFileName),
+    FileServices.StartSuperFileTransaction(),
+    FileServices.AddSuperFile(superFileName,albumTableName),
+    FileServices.FinishSuperFileTransaction(),
+
+    // Create a some index files and add them to a Superindex
+    BUILDINDEX(albumIndex1,OVERWRITE),
+    BUILDINDEX(albumIndex2,OVERWRITE),
+    BUILDINDEX(albumIndex,OVERWRITE),
+
+    FileServices.CreateSuperFile(superIndexName),
+    FileServices.StartSuperFileTransaction(),
+    FileServices.AddSuperFile(prefix + 'superalbums.key', albumIndex1Name),
+    FileServices.AddSuperFile(prefix + 'superalbums.key', albumIndex2Name),
+    FileServices.AddSuperFile(prefix + 'superalbums.key', albumIndexName),
+    FileServices.FinishSuperFileTransaction(),
+
+    // Copy Superfile as a logical file - should pass
+    supercopyOut;
+    // Copy Superfile as a superfile - should pass
+    supercopyOut2;
+
+    //Copy Superindex as a logical file - should fail
+    supercopyOut3;
+
+    // Copy Superindex as a Super file - should pass
+    supercopyOut4;
+
+    // Clean-up
+    FileServices.DeleteSuperFile(superFileName),
+    FileServices.DeleteSuperFile(superFileCopyName),
+    FileServices.DeleteSuperFile(superIndexName),
+    FileServices.DeleteSuperFile(superIndexCopyName),
+
+    FileServices.DeleteLogicalFile(albumIndex1Name),
+    FileServices.DeleteLogicalFile(albumIndex2Name),
+    FileServices.DeleteLogicalFile(albumIndexName),
+
+    FileServices.DeleteLogicalFile(albumTableName),
+);