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

Handle autoload error not caught by safe_constantize #16608

Merged

Conversation

imtayadeway
Copy link
Contributor

MiqExpression currently supports 'values' that are 'fields', i.e.:

{"=" => {"field" => "Vm-name", "value" => "Vm-description"}}

It will however trip up on a value/field if it looks like it
contains a constant that can be autoloaded, but can't:

{"=" => {"field" => "Vm-name", "value" => "VM-test"}}

...

"VM".safe_constantize # => LoadError: Unable to autoload constant VM, expected /home/tim/src/manageiq/app/models/vm.rb to define it

We need to explicitly catch this LoadError, which does not inherit
from StandardError, returning nil, to prevent MiqExpression from
trying to interpret this as a "field value".

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1521167

MiqExpression currently supports 'values' that are 'fields', i.e.:

```ruby
{"=" => {"field" => "Vm-name", "value" => "Vm-description"}}
```

It will however trip up on a value/field if it *looks* like it
contains a constant that can be autoloaded, but can't:

```ruby
{"=" => {"field" => "Vm-name", "value" => "VM-test"}}
```

...

```ruby
"VM".safe_constantize # => LoadError: Unable to autoload constant VM, expected /home/tim/src/manageiq/app/models/vm.rb to define it
```

We need to explicitly catch this `LoadError`, which does not inherit
from `StandardError`, returning `nil`, to prevent MiqExpression from
trying to interpret this as a "field value".

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1521167
@miq-bot
Copy link
Member

miq-bot commented Dec 6, 2017

Checked commit imtayadeway@54bd9d3 with ruby 2.3.3, rubocop 0.47.1, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 0 offenses detected
Everything looks fine. 🏆

@Fryguy Fryguy merged commit 7a5fd37 into ManageIQ:master Dec 6, 2017
@Fryguy Fryguy added this to the Sprint 75 Ending Dec 11, 2017 milestone Dec 6, 2017
simaishi pushed a commit that referenced this pull request Dec 11, 2017
…alues

Handle autoload error not caught by safe_constantize
(cherry picked from commit 7a5fd37)

https://bugzilla.redhat.com/show_bug.cgi?id=1522846
@simaishi
Copy link
Contributor

Gaprindashvili backport details:

$ git log -1
commit b175209421598ddb0f9743888f879d346f8d7e0e
Author: Jason Frey <fryguy9@gmail.com>
Date:   Wed Dec 6 13:30:30 2017 -0500

    Merge pull request #16608 from imtayadeway/bug/miq-expression-field-values
    
    Handle autoload error not caught by safe_constantize
    (cherry picked from commit 7a5fd377e48852964039bf9d674e9426cde34513)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1522846

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants