From 2cdf1293b5252de60fa26421b748ea982f5499d5 Mon Sep 17 00:00:00 2001 From: Tim Klemm Date: Wed, 20 Nov 2024 12:42:10 -0500 Subject: [PATCH] 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 --- common/workunit/workunit.cpp | 9 +++++ common/workunit/workunit.hpp | 13 ++++++++ dali/sasha/saarch.cpp | 2 +- esp/services/ws_fs/ws_fsService.cpp | 11 +++---- esp/services/ws_smc/ws_smcService.cpp | 2 +- .../ws_workunits/ws_workunitsService.cpp | 33 ++++++++++--------- testing/unittests/wutests.cpp | 6 ++++ 7 files changed, 53 insertions(+), 23 deletions(-) diff --git a/common/workunit/workunit.cpp b/common/workunit/workunit.cpp index bbe0159aede..7c3607f6e2b 100644 --- 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))); +} \ No newline at end of file diff --git a/common/workunit/workunit.hpp b/common/workunit/workunit.hpp index 0ae02fc3e6f..39e09b9b509 100644 --- 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 index 0d0d71efd10..730a96b6dab 100644 --- 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 index 43e500e94b2..4a3a9138719 100644 --- 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 index 89169dc29b9..97abd00ee0d 100644 --- 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 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 factory = getWorkUnitFactory(context.querySecManager(), context.queryUser()); Owned 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 index 7b509ef85fc..bdefd82641b 100644 --- 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 );