-
Notifications
You must be signed in to change notification settings - Fork 300
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-15589 ConfigMgr - Fix ambiguous xpath in EspBindings #8691
HPCC-15589 ConfigMgr - Fix ambiguous xpath in EspBindings #8691
Conversation
…ation Signed-off-by: Gleb Aronsky <gleb.aronsky@lexisnexis.com>
https://track.hpccsystems.com/browse/HPCC-15589 |
@rpastrana please review |
Automated Smoketest |
@@ -4665,7 +4665,13 @@ bool CWsDeployFileInfo::handleEspServiceBindings(IEspContext &context, IEspHandl | |||
if (resource) | |||
xpath.appendf("[@resource='%s']", resource); | |||
|
|||
IPropertyTree* pSubType = pEnvRoot->queryPropTree(xpath.str()); | |||
IPropertyTree* pSubType = nullptr; | |||
Owned<IPropertyTreeIterator> iterSubType = pEnvRoot->getElements(xpath.str()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we only care about the first entry, why are users allowed to enter more than 1 value. Isn't this fixing the symptom of the problem and not the problem? If more than 1 entry is allowed, but this piece of code always works on just the first entry, then you may want to just get the first entry and not create an iterator. i.e -
if (iterSubType->first() && iterSubType->isValid())
pSubType = &(iterSubType->query()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason for using an iterator, even though I only want the first element, is because queryPropTree() returns an ‘ambiguous xpath exception” when provided a valid xpath using an index. I do a hasProp call to verify that the element is there. The queryProp() call then throws an exception even though the index is provided and the xpath is valid and not ambiguous. This is the output from the debugger:
(gdb) p pEnvRoot->hasProp("./Software/EspProcess[@name='myesp']/EspBinding[@defaultForPort='true'][@name='EspBinding'][@port='8010'][@protocol='http'][@resourcesBasedn='ou=SMC,ou=EspServices,ou=ecl'][@service='EclWatch'][@workunitsBasedn='ou=workunits,ou=ecl']/Authenticate[@resource=''][1]")
$16 = true
(gdb) p pEnvRoot->queryProp("./Software/EspProcess[@name='myesp']/EspBinding[@defaultForPort='true'][@name='EspBinding'][@port='8010'][@protocol='http'][@resourcesBasedn='ou=SMC,ou=EspServices,ou=ecl'][@service='EclWatch'][@workunitsBasedn='ou=workunits,ou=ecl']/Authenticate[@resource=''][1]")
The program being debugged entered a std::terminate call, most likely caused by an unhandled C++ exception. GDB blocked this call in order to prevent the program from being terminated, and has restored the context to its original state before the call.
To change this behaviour use "set unwind-on-terminating-exception off".
Evaluation of the expression containing the function (PTree::queryProp(char const*) const)
will be abandoned.
I can replace the ForEach macro, but I think the iterator needs to stay, and some of the ugliness.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suspect we really need to fix both the symptom (crashing if there is more than one element matching) and the cause (whatever allowed the duplicate entries to be created in the first place). Either fix by itself is insufficient...
@garonsky reviewed. Left a minor comment. |
Signed-off-by: Gleb Aronsky gleb.aronsky@lexisnexis.com