Use YAML.load to load classes beyond the basic types #6596
Conversation
https://bugzilla.redhat.com/show_bug.cgi?id=1789153 Followup to ManageIQ/manageiq#19701 At the very least, we know MiqExpression objects are in the YAML but other custom classes could be in the YAML so we need to use YAML.load to return to the prior behavior.
jrafanie
force-pushed
the
fix_invalid_usage_of_yaml_safe_load
branch
from
7675a05
to
00cfceb
Compare
|
@himdel it's green now, thanks for the good 👀 |
|
Checked commit jrafanie@00cfceb with ruby 2.5.5, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0 app/controllers/application_controller/miq_request_methods.rb
spec/controllers/application_controller/report_data_spec.rb
spec/support/report_helper.rb
|
| @@ -168,7 +168,7 @@ def set_pre_prov_vars | |||
|
|
|||
| unless %w[image_miq_request_new miq_template_miq_request_new].include?(params[:pressed]) | |||
| path_to_report = ManageIQ::UI::Classic::Engine.root.join("product", "views", provisioning_report).to_s | |||
There was a problem hiding this comment.
I'm assuming all of these are controlled by us? That is, a user (via the UI/API) can't get their own files into this directory?
There was a problem hiding this comment.
Yup, those are ours manageiq-ui-classic/product/views, and provisioning_report will always return either ProvisionCloudTemplates.yaml or ProvisionInfraTemplates.yaml
| @@ -168,7 +168,7 @@ def set_pre_prov_vars | |||
|
|
|||
| unless %w[image_miq_request_new miq_template_miq_request_new].include?(params[:pressed]) | |||
| path_to_report = ManageIQ::UI::Classic::Engine.root.join("product", "views", provisioning_report).to_s | |||
| @view = MiqReport.new(YAML.safe_load(File.open(path_to_report), [Symbol])) | |||
| @view = MiqReport.new(YAML.load(File.open(path_to_report))) | |||
There was a problem hiding this comment.
Maybe not for this PR, but this opens a file handle and then never closes it. Perhaps instead use YAML.load_file or MiqReport.new(File.open(path_to_report) { |f| YAML.load(f) })
There was a problem hiding this comment.
I'll use File.read in case we want to enumerate the possible classes and change this back to safe_load since there is no YAML.safe_file_load.
There was a problem hiding this comment.
Opened a PR... note, I found other instances of the same pattern:
#6597
https://bugzilla.redhat.com/show_bug.cgi?id=1789153
Followup to ManageIQ/manageiq#19701
At the very least, we know MiqExpression objects are in the YAML but other
custom classes could be in the YAML so we need to use YAML.load to return to the
prior behavior.