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.

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-31411 Make work unit IDs case insensitive #19399

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
9 changes: 9 additions & 0 deletions common/workunit/workunit.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)));
}
13 changes: 13 additions & 0 deletions common/workunit/workunit.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion dali/sasha/saarch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
11 changes: 5 additions & 6 deletions esp/services/ws_fs/ws_fsService.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion esp/services/ws_smc/ws_smcService.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
33 changes: 18 additions & 15 deletions esp/services/ws_workunits/ws_workunitsService.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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));
Expand Down Expand Up @@ -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;
Expand All @@ -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));
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand All @@ -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)
{
Expand Down
6 changes: 6 additions & 0 deletions testing/unittests/wutests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ class wuTests : public CppUnit::TestFixture
{
CPPUNIT_TEST_SUITE( wuTests );
CPPUNIT_TEST(testLooksLikeAWuid);
CPPUNIT_TEST(testWuidPattern);
CPPUNIT_TEST_SUITE_END();

public:
Expand Down Expand Up @@ -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 );
Expand Down
Loading