ManageIQ / manageiq-automation_engine

Automation engine for ManageIQ
Apache License 2.0
11 stars 74 forks source link

Bug with handling of Arrays in Automate #480

Closed zigaSRC closed 3 years ago

zigaSRC commented 3 years ago

There's an issue when passing arrays of strings with commas between automate methods (indcluding output from multiselect dialog).

Current Behavior:

Saving ["test1,test2", "test3,test4"] (2 elements) under $evm (or selecting multiple strings with commas in a dialog) and then reading the value in another/different automate method returns ["\"test1", "test2\"", "\"test3", "test4\""] (4 elements)

Expected Behavior:

Saving an array of strings in one method should produce the exact same array when reading the value in another method.

Steps To Reproduce:

  1. Create two automate methods that are run one after another.
  2. Save the above array under $evm in the first method. $evm.root['test'] = ["test1,test2", "test3,test4"]
  3. Read the value in the second method. $evm.log('info', "Array: #{$evm.root['test']}")
  4. See a different array returned than was saved.

Environment:

ManageIQ Lasker-1

Anything else:

I suspect the split method here: https://github.com/ManageIQ/manageiq-automation_engine/blob/5be34d7779dbd7e968326076c39aacca197fc59f/lib/miq_automation_engine/engine/miq_ae_engine/miq_ae_object.rb#L491 splits the string (array) on every comma even if they are a part of a string and not a delimiter between elements.

Using eval() would solve this problem, such as many others (nested arrays or other comma issues). Though I must admit I don't know how the values are handled on the other end to judge if this would be a safe use of the eval method.

zigaSRC commented 3 years ago

Found this bit of code as well: https://github.com/ManageIQ/manageiq-automation_engine/blob/5be34d7779dbd7e968326076c39aacca197fc59f/lib/miq_automation_engine/engine/miq_ae_engine.rb#L246

As far as I found out looking at the structure under $evm there are a couple of different ways the arrays are stored in different dialog field/value locations. Standardizing the structure and serializing the array in one of the more standard ways (json, yaml or using Marshal) might be a good improvement as a comma is not that uncommon to have in a string. Someone with a bit more knowledge of the backend might need to take a look as I (just a user) don't have a good grasp of the whole backend.

Fryguy commented 3 years ago

cc @akhilkr128

akhilkr128 commented 3 years ago

This bug happens when we create option values like this.

Screenshot 2021-10-07 at 5 19 46 PM

Saving direct array values to $evm variable as provided in the steps is not producing any errors.

Screenshot 2021-10-07 at 5 19 57 PM

Working on the solution.

Fryguy commented 3 years ago

Fixed in #484