HPCC-31411 Make work unit IDs case insensitive

- Accept either 'w' or 'W' at the start of WUID filter patterns. - Improve reliability of ignoring leading and trailing whitespace. - Introduce a WUID pattern encapsulation that will grow to include more robust and standardized pattern analysis. Signed-off-by: Tim Klemm<[email protected]>
hpcc-systems · Jan 10, 2025 · 2cdf129 · 2cdf129
1 parent 134d1f3
commit 2cdf129
Show file tree

Hide file tree

Showing 7 changed files with 53 additions and 23 deletions.
diff --git a/common/workunit/workunit.cpp b/common/workunit/workunit.cpp
@@ -14985,3 +14985,12 @@ void recordTraceDebugOptions(IWorkUnit * target, const IProperties * source)
         }
     }
 }
+
+WuidPattern::WuidPattern(const char* _pattern)
+    : pattern(_pattern)
+{
+    if (!pattern.isEmpty())
+        pattern.trim();
+    if (!pattern.isEmpty() && islower(pattern.charAt(0)))
+        pattern.setCharAt(0, toupper(pattern.charAt(0)));
+}
diff --git a/common/workunit/workunit.hpp b/common/workunit/workunit.hpp
@@ -1843,4 +1843,17 @@ class WORKUNIT_API StatisticsAggregator : public CInterface
     const StatisticsMapping & mapping;
 };
 
+class WORKUNIT_API WuidPattern
+{
+private:
+    StringBuffer pattern;
+public:
+    WuidPattern(const char* _pattern);
+    inline bool isEmpty() const { return pattern.isEmpty(); }
+    inline const char* str() const { return pattern.str(); }
+    inline operator const char* () const { return pattern.str(); }
+    inline operator const StringBuffer& () const { return pattern; }
+    inline operator StringBuffer& () { return pattern; }
+};
+
 #endif
diff --git a/dali/sasha/saarch.cpp b/dali/sasha/saarch.cpp
@@ -1166,7 +1166,7 @@ class CDFUWorkUnitArchiver: public CBranchArchiver
             getWorkUnitCreateTime(wuid,time);
         }
         virtual ~cDFUWUBranchItem() {}
-        bool isempty() { return (wuid[0]!='D')||iserr; }
+        bool isempty() { return (toupper(wuid[0])!='D')||iserr; }
         bool qualifies() 
         { 
             if (isprotected)

diff --git a/esp/services/ws_fs/ws_fsService.cpp b/esp/services/ws_fs/ws_fsService.cpp
@@ -924,10 +924,9 @@ bool CFileSprayEx::onGetDFUWorkunits(IEspContext &context, IEspGetDFUWorkunits &
     {
         context.ensureFeatureAccess(DFU_WU_URL, SecAccess_Read, ECLWATCH_DFU_WU_ACCESS_DENIED, "Access to DFU workunit is denied.");
 
-        StringBuffer wuidStr(req.getWuid());
-        const char* wuid = wuidStr.trim().str();
-        if (wuid && *wuid && looksLikeAWuid(wuid, 'D'))
-            return getOneDFUWorkunit(context, wuid, resp);
+        WuidPattern wuidPattern(req.getWuid());
+        if (!wuidPattern.isEmpty() && looksLikeAWuid(wuidPattern, 'D'))
+            return getOneDFUWorkunit(context, wuidPattern, resp);
 
         double version = context.getClientVersion();
         if (version > 1.02)
@@ -1055,11 +1054,11 @@ bool CFileSprayEx::onGetDFUWorkunits(IEspContext &context, IEspGetDFUWorkunits &
                 filterbuf.append("");
         }
 
-        if(wuid && *wuid)
+        if(!isEmptyString(wuidPattern))
         {
             filters[filterCount] = DFUsf_wildwuid;
             filterCount++;
-            filterbuf.append(wuid);
+            filterbuf.append(wuidPattern);
         }
 
         if(clusterName && *clusterName)

diff --git a/esp/services/ws_smc/ws_smcService.cpp b/esp/services/ws_smc/ws_smcService.cpp
@@ -1237,7 +1237,7 @@ void CWsSMCEx::addWUsToResponse(IEspContext &context, const IArrayOf<IEspActiveW
     {
         IEspActiveWorkunit& wu = aws.item(i);
         const char* wuid = wu.getWuid();
-        if (wuid[0] == 'D')//DFU WU
+        if (toupper(wuid[0]) == 'D')//DFU WU
         {
             awsReturned.append(*LINK(&wu));
             continue;

diff --git a/esp/services/ws_workunits/ws_workunitsService.cpp b/esp/services/ws_workunits/ws_workunitsService.cpp
@@ -1883,6 +1883,8 @@ void doWUQueryWithSort(IEspContext &context, IEspWUQueryRequest & req, IEspWUQue
     unsigned short filterCount = 0;
     MemoryBuffer filterbuf;
 
+    WuidPattern wuidPattern(req.getWuid());
+
     // Query filters should be added in order of expected power - add the most restrictive filters first
 
     bool bDoubleCheckState = false;
@@ -1897,7 +1899,7 @@ void doWUQueryWithSort(IEspContext &context, IEspWUQueryRequest & req, IEspWUQue
             bDoubleCheckState = true;
     }
 
-    addWUQueryFilter(filters, filterCount, filterbuf, req.getWuid(), WUSFwildwuid);
+    addWUQueryFilter(filters, filterCount, filterbuf, wuidPattern.str(), WUSFwildwuid);
     addWUQueryFilter(filters, filterCount, filterbuf, req.getCluster(), WUSFcluster);
     addWUQueryFilter(filters, filterCount, filterbuf, req.getLogicalFile(), (WUSortField) (WUSFfileread | WUSFnocase));
     addWUQueryFilter(filters, filterCount, filterbuf, req.getOwner(), (WUSortField) (WUSFuser | WUSFnocase));
@@ -2098,6 +2100,8 @@ void doWULightWeightQueryWithSort(IEspContext &context, IEspWULightWeightQueryRe
     unsigned short filterCount = 0;
     MemoryBuffer filterbuf;
 
+    WuidPattern wuidPattern(req.getWuid());
+
     // Query filters should be added in order of expected power - add the most restrictive filters first
 
     bool bDoubleCheckState = false;
@@ -2112,7 +2116,7 @@ void doWULightWeightQueryWithSort(IEspContext &context, IEspWULightWeightQueryRe
             bDoubleCheckState = true;
     }
 
-    addWUQueryFilter(filters, filterCount, filterbuf, req.getWuid(), WUSFwildwuid);
+    addWUQueryFilter(filters, filterCount, filterbuf, wuidPattern.str(), WUSFwildwuid);
     addWUQueryFilter(filters, filterCount, filterbuf, req.getCluster(), WUSFcluster);
     addWUQueryFilter(filters, filterCount, filterbuf, req.getOwner(), (WUSortField) (WUSFuser | WUSFnocase));
     addWUQueryFilter(filters, filterCount, filterbuf, req.getJobName(), (WUSortField) (WUSFjob | WUSFnocase));
@@ -2586,15 +2590,14 @@ bool CWsWorkunitsEx::onWUQuery(IEspContext &context, IEspWUQueryRequest & req, I
 {
     try
     {
-        StringBuffer wuidStr(req.getWuid());
-        const char* wuid = wuidStr.trim().str();
+        WuidPattern pattern(req.getWuid());
 
         if (req.getType() && strieq(req.getType(), "archived workunits"))
             doWUQueryFromArchive(context, sashaServerIp.get(), sashaServerPort, *archivedWuCache, awusCacheMinutes, req, resp);
-        else if(notEmpty(wuid) && looksLikeAWuid(wuid, 'W'))
+        else if(notEmpty(pattern) && looksLikeAWuid(pattern, 'W'))
         {
-            ensureWsWorkunitAccess(context, wuid, SecAccess_Read);
-            doWUQueryBySingleWuid(context, wuid, resp);
+            ensureWsWorkunitAccess(context, pattern, SecAccess_Read);
+            doWUQueryBySingleWuid(context, pattern, resp);
         }
         else if (notEmpty(req.getLogicalFile()) && req.getLogicalFileSearchType() && strieq(req.getLogicalFileSearchType(), "Created"))
             doWUQueryByFile(context, req.getLogicalFile(), resp);
@@ -5179,16 +5182,16 @@ bool CWsWorkunitsEx::onWUGetStats(IEspContext &context, IEspWUGetStatsRequest &r
         if (!req.getCreateDescriptions_isNull())
             createDescriptions = req.getCreateDescriptions();
 
-        StringBuffer wuid(req.getWUID());
-        PROGLOG("WUGetStats: %s", wuid.str());
+        WuidPattern wuidPattern(req.getWUID());
+        PROGLOG("WUGetStats: %s", wuidPattern.str());
 
         IArrayOf<IEspWUStatisticItem> statistics;
-        if (strchr(wuid, '*'))
+        if (strchr(wuidPattern, '*'))
         {
             WUSortField filters[2];
             MemoryBuffer filterbuf;
             filters[0] = WUSFwildwuid;
-            filterbuf.append(wuid.str());
+            filterbuf.append(wuidPattern.str());
             filters[1] = WUSFterm;
             Owned<IWorkUnitFactory> factory = getWorkUnitFactory(context.querySecManager(), context.queryUser());
             Owned<IConstWorkUnitIterator> iter = factory->getWorkUnitsSorted((WUSortField) (WUSFwuid), filters, filterbuf.bufferBase(), 0, INT_MAX, NULL, NULL);
@@ -5205,14 +5208,14 @@ bool CWsWorkunitsEx::onWUGetStats(IEspContext &context, IEspWUGetStatsRequest &r
         }
         else
         {
-            WsWuHelpers::checkAndTrimWorkunit("WUInfo", wuid);
-            ensureWsWorkunitAccess(context, wuid, SecAccess_Read);
+            WsWuHelpers::checkAndTrimWorkunit("WUInfo", wuidPattern);
+            ensureWsWorkunitAccess(context, wuidPattern, SecAccess_Read);
 
-            WsWuInfo winfo(context, wuid);
+            WsWuInfo winfo(context, wuidPattern);
             winfo.getStats(filter, statsFilter, createDescriptions, statistics);
         }
         resp.setStatistics(statistics);
-        resp.setWUID(wuid.str());
+        resp.setWUID(wuidPattern.str());
     }
     catch(IException* e)
     {

diff --git a/testing/unittests/wutests.cpp b/testing/unittests/wutests.cpp
@@ -23,6 +23,7 @@ class wuTests : public CppUnit::TestFixture
 {
     CPPUNIT_TEST_SUITE( wuTests );
         CPPUNIT_TEST(testLooksLikeAWuid);
+        CPPUNIT_TEST(testWuidPattern);
     CPPUNIT_TEST_SUITE_END();
 
 public:
@@ -74,6 +75,11 @@ class wuTests : public CppUnit::TestFixture
         CPPUNIT_ASSERT_MESSAGE("looksLikeAWuid should fail", !looksLikeAWuid("W12345678-123456-", 'W'));
         CPPUNIT_ASSERT_MESSAGE("looksLikeAWuid should fail", !looksLikeAWuid("*", 'W'));
     }
+
+    void testWuidPattern()
+    {
+        CPPUNIT_ASSERT_MESSAGE("wuidPattern should pass", looksLikeAWuid(WuidPattern(" \t\rw12345678-123456 \t\r"), 'W'));
+    }
 };
 
 CPPUNIT_TEST_SUITE_REGISTRATION( wuTests );