Repository: NREL/OpenStudio (language: python)

Issue:
runner.getPastStepValuesForMeasure can cause segfault
Issue overview
--------------
I believe that certain values (i.e., containing special characters?) stored in "value" of a workflow step can cause a segfault. I have not investigated/narrowed this down, however.

## Current Behavior
<!--- Tell us what happens instead of the expected behavior -->

## Expected Behavior
<!--- Tell us what should happen -->

## Steps to Reproduce
Execute `runner.getPastStepValuesForMeasure('apply_upgrade')` from resstock's UpgradeCosts measure. Values can contain "(", "|", etc.

## Possible Solution
<!--- Optional, but if you can, suggest a fix/reason for the bug, -->

## Details

### Environment

Some additional details about your environment for this issue (if relevant):
 - Platform (Operating system, version):
 - Version of OpenStudio (if using an intermediate build, include SHA):

### Context
<!--- How has this issue affected you? What are you trying to accomplish? -->



Discussion / hints:
I think it's an encoding issue when converting from Json::Value to a string to be used as a key


I can reproduce using this

# Python: it works

```
from pathlib import Path
import sys
sys.path.insert(0, str(Path('./Products/python/').absolute()))
import openstudio


workflow = openstudio.WorkflowJSON()
runner = openstudio.measure.OSRunner(workflow)

step1 = openstudio.MeasureStep("MeasureName1")
step1.setArgument("Argument1", 100)

workflow_step_result1 = openstudio.WorkflowStepResult()
workflow_step_result1.setMeasureName("measure_name_1")

stepValue1 = openstudio.WorkflowStepValue("(|)'*$£*µ%é", True)
workflow_step_result1.addStepValue(stepValue1)

workflow_step_result1.setStepResult(openstudio.StepResult('Success'))
workflow_step_result1.setCompletedAt(openstudio.DateTime.nowUTC())
step1.setResult(workflow_step_result1)

workflow.setWorkflowSteps([step1])

workflow.saveAs("OSRunner_getPastStepValues_special_chars.osw")

print(runner.getPastStepValuesForMeasure("MeasureName1"))
```

```
{"(|)'*$£*µ%é": True}
```

# Ruby

```
require 'openstudio'

workflow = OpenStudio::WorkflowJSON.new()
runner = OpenStudio::Measure::OSRunner.new(workflow)

step1 = OpenStudio::MeasureStep.new("MeasureName1")
step1.setArgument("Argument1", 100)

workflow_step_result1 = OpenStudio::WorkflowStepResult.new()
workflow_step_result1.setMeasureName("measure_name_1")

stepValue1 = OpenStudio::WorkflowStepValue.new("(|)'*$£*µ%é", true)
workflow_step_result1.addStepValue(stepValue1)

workflow_step_result1.setStepResult('Success'.to_StepResult)
workflow_step_result1.setCompletedAt(OpenStudio::DateTime.nowUTC())
step1.setResult(workflow_step_result1)

workflow.setWorkflowSteps([step1])

workflow.saveAs("OSRunner_getPastStepValues_special_chars.osw")

puts runner.getPastStepValuesForMeasure("MeasureName1")
```

```
$ Products/openstudio test.rb

EncodingError: invalid symbol in encoding US-ASCII :"(|)'*$\xC2\xA3*\xC2\xB5%\xC3\xA9"
Backtrace:
	/home/julien/Software/Others/OS-build-release/test.rb:23:in `getPastStepValuesForMeasure'
	/home/julien/Software/Others/OS-build-release/test.rb:23:in `<top (required)>'
	eval:182:in `require'
	eval:182:in `require'
	<internal::/ruby/3.2.0/rubygems/core_ext/kernel_require.rb>:85:in `require'
	eval:4:in `<main>'
Failed to execute '/home/julien/Software/Others/OS-build-release/test.rb'
```

I try to make it a symbol... and that's the issue

https://github.com/NREL/OpenStudio/blob/53c1d69a81a6a833244d193a39a36cee601dd3be/src/utilities/core/jsoncpp.i#L111

rb_intern uses us_ascii encoding. There's a rb_intern3 that takes an encoding, but that isn't public by default and I can't get swig to find it yet

I don't know how to fix it.

Fix patch:
diff --git a/src/measure/OSRunner.cpp b/src/measure/OSRunner.cpp
index c770dda33be..77ce472a095 100644
--- a/src/measure/OSRunner.cpp
+++ b/src/measure/OSRunner.cpp
@@ -825,10 +825,11 @@ namespace measure {
       // TODO: should we match on any of these three?
       if (istringEqual(measureName, step.measureDirName())) {  // The directory name, eg `IncreaseWallRValue`
         LOG(Trace, "Step matches on measureDirName");
-      } else if (auto s_ = step.name(); istringEqual(measureName, *s_)) {  // An optional, abritrary one
+      } else if (auto s_ = step.name(); s_.is_initialized() && istringEqual(measureName, *s_)) {  // An optional, abritrary one
         LOG(Trace, "Step matches on name");
       } else if (auto s_ = stepResult_->measureName();
-                 istringEqual(measureName, *s_)) {  // The xml one, eg `increase_insulation_r_value_for_exterior_walls_by_percentage`
+                 s_.is_initialized()
+                 && istringEqual(measureName, *s_)) {  // The xml one, eg `increase_insulation_r_value_for_exterior_walls_by_percentage`
         LOG(Trace, "Step matches on Step Result's measureName");
       } else {
         continue;


Test patch:
diff --git a/src/measure/test/OSRunner_GTest.cpp b/src/measure/test/OSRunner_GTest.cpp
index 0d9729def90..0653b3d46fa 100644
--- a/src/measure/test/OSRunner_GTest.cpp
+++ b/src/measure/test/OSRunner_GTest.cpp
@@ -385,3 +385,51 @@ TEST_F(MeasureFixture, OSRunner_getPastStepValues) {
     EXPECT_EQ(20, stepValues["MeasureName3"].asInt());
   }
 }
+
+TEST_F(MeasureFixture, OSRunner_getPastStepValues_step_name_not_initialized) {
+  // #5192 - Produce a segfault on purpose
+  WorkflowJSON workflow;
+  OSRunner runner(workflow);
+
+  std::vector<WorkflowStep> workflow_steps;
+
+  {
+    MeasureStep step1("MeasureName1");
+    workflow_steps.push_back(step1);
+
+    step1.setArgument("Argument1", 100);
+    step1.setArgument("Argument2", 200);
+    EXPECT_EQ("MeasureName1", step1.measureDirName());
+    EXPECT_FALSE(step1.name());
+
+    {
+      WorkflowStepResult workflow_step_result1;
+      EXPECT_FALSE(workflow_step_result1.measureName());
+      workflow_step_result1.setMeasureName("measure_name_1");
+      ASSERT_TRUE(workflow_step_result1.measureName());
+      EXPECT_EQ("measure_name_1", workflow_step_result1.measureName().get());
+
+      const WorkflowStepValue stepValue1("StepValue1", true);
+      workflow_step_result1.addStepValue(stepValue1);
+
+      const WorkflowStepValue stepValue2("StepValue2", 5);
+      workflow_step_result1.addStepValue(stepValue2);
+
+      // Setting a Skip + (Completed At so it shows in the JSON)
+      workflow_step_result1.setStepResult(StepResult::Success);
+      workflow_step_result1.setCompletedAt(DateTime::nowUTC());
+      step1.setResult(workflow_step_result1);
+    }
+  }
+
+  EXPECT_TRUE(workflow.setWorkflowSteps(workflow_steps));
+  EXPECT_EQ(1, workflow.workflowSteps().size());
+  EXPECT_EQ(workflow.string(), runner.workflow().string());
+
+  workflow.saveAs(scratchDir / "OSRunner_getPastStepValues_step_name_not_initialized.osw");
+
+  {
+    const Json::Value stepValues = runner.getPastStepValuesForMeasure("non_existing");
+    EXPECT_EQ(0, stepValues.size());  // did not set step result as Success
+  }
+}