firely-net-sdk icon indicating copy to clipboard operation
firely-net-sdk copied to clipboard

fhirpath repeats implementation bug with string data

Open brianpos opened this issue 2 years ago • 2 comments

Describe the bug If the repeat fhirpath function is provided a string literal as a parameter rather than a property name, the engine enters into an infinite loop (till it runs out of memory eventually - not a stack overflow) Further discussion has been had on here. https://chat.fhir.org/#narrow/stream/179266-fhirpath/topic/repeat.28'someString'.29

Offending code is here: https://github.com/FirelyTeam/firely-net-common/blob/ba31e20d00dd0f2c5e0d56ceefa4fff4d686b309/src/Hl7.FhirPath/FhirPath/Expressions/SymbolTableInit.cs#L350

To Reproduce Steps to reproduce the behavior:

[TestMethod]
public void TestFhirPathRepeatsStringLiteral()
{
        var patient = new Patient() { Active = false };
        patient.Name.Add(new HumanName() {Text = "name1" });
        patient.Name.Add(new HumanName() { Text = "name2" });
        var nav = patient.ToTypedElement();
        
        var result = nav.Select("name.repeat('teststring')");
        Assert.IsNotNull(result.FirstOrDefault());
        Assert.AreEqual("teststring", result.First().Value);
}

Expected behavior The repeat function should terminate if no "new" items are discovered (based on the equals operator), not just if there there is a value returned.

Version used:

  • FHIR Version: R4 (but would be in all versions)
  • Version: 3.8.0

brianpos avatar Mar 24 '22 23:03 brianpos

This is the code snippit that I've tried to fix the problem, resolves the stack change, but I'm not sure it's 100% right either

                    newNodes.AddRange(lambda(newContext, InvokeeFactory.EmptyArgs));
                }
                // Ensure that there are new nodes being included by the repeat
                if (current.Count() >= current.DistinctUnion(newNodes).Count())
                    newNodes.Clear();
                fullResult.AddRange(newNodes);

brianpos avatar Mar 24 '22 23:03 brianpos

Further discussion on chat.fhir.org before we come to a conclusion.

brianpos avatar Mar 24 '22 23:03 brianpos

This is fixed, see https://github.com/FirelyTeam/firely-net-sdk/pull/2180

ewoutkramer avatar Mar 03 '23 10:03 ewoutkramer