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

runner.getPastStepValuesForMeasure can cause segfault #5192

Closed
joseph-robertson opened this issue May 9, 2024 · 2 comments · Fixed by #5194 or #5199
Closed

runner.getPastStepValuesForMeasure can cause segfault #5192

joseph-robertson opened this issue May 9, 2024 · 2 comments · Fixed by #5194 or #5199

Comments

@joseph-robertson
Copy link
Collaborator

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

Expected Behavior

Steps to Reproduce

Execute runner.getPastStepValuesForMeasure('apply_upgrade') from resstock's UpgradeCosts measure. Values can contain "(", "|", etc.

Possible Solution

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

@jmarrec
Copy link
Collaborator

jmarrec commented May 13, 2024

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'

@jmarrec
Copy link
Collaborator

jmarrec commented May 13, 2024

I try to make it a symbol... and that's the issue

rb_hash_aset(result, ID2SYM(rb_intern(id.data())), SWIG_From_JsonValue(value[id]));

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.

jmarrec added a commit that referenced this issue May 13, 2024
```
The following tests FAILED:
	3705 - RubyTest-SwigJson_Test-swig_json_special_chars (Failed)
	3870 - CLITest-SwigJson_Test-swig_json_special_chars (Failed)
```
jmarrec added a commit that referenced this issue May 14, 2024
```
The following tests FAILED:
	3705 - RubyTest-SwigJson_Test-swig_json_special_chars (Failed)
	3870 - CLITest-SwigJson_Test-swig_json_special_chars (Failed)
```
kbenne added a commit that referenced this issue May 15, 2024
#5192 - Fix runner.getPastStepValuesForMeasure can cause segfault
@jmarrec jmarrec reopened this May 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants