Keep full model name when parsing expression with tag #16211
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
imtayadeway
approved these changes
Oct 16, 2017
lpichler
reviewed
Oct 16, 2017
lib/miq_expression/tag.rb
Outdated
| @@ -1,6 +1,6 @@ | |||
| class MiqExpression::Tag < MiqExpression::Target | |||
| REGEX = / | |||
| (?<model_name>([[:alnum:]]*(::)?)?) | |||
| (?<model_name>([[:alnum:]]*(::)?)*) | |||
There was a problem hiding this comment.
@yrudman you need probably limit count of ::
Problem is failing case like, it is too slow.
MiqExpression::Tag.parse('ManageIQ::Providers::CloudManagermanaged-se')There was a problem hiding this comment.
👍 nice catch @lpichler
parsing your example from console never finished and was interrupted after 2 minutes.
And parsing it on master returning nil. So, limiting :: looks like good idea.
…finity loop inside regex when parsing strings like 'ManageIQ::Providers::CloudManagermanaged-se'
yrudman
force-pushed
the
keep-full-model-name-when-parsing-expression-with-tag
branch
from
d6b9717
to
7822c56
Compare
yrudman
force-pushed
the
keep-full-model-name-when-parsing-expression-with-tag
branch
2 times, most recently
from
9a6f26d
to
1ef3fb0
Compare
|
Checked commits yrudman/manageiq@071592a~...1ef3fb0 with ruby 2.3.3, rubocop 0.47.1, and haml-lint 0.20.0 |
gtanzillo
approved these changes
Oct 19, 2017
yrudman
deleted the
keep-full-model-name-when-parsing-expression-with-tag
branch
jrafanie
added a commit
to jrafanie/manageiq
that referenced
this pull request
Dec 6, 2017
https://bugzilla.redhat.com/show_bug.cgi?id=1519809 This will prevent an infinite loop inside regex when parsing strings like "ManageIQ::Providers::CloudManager::Vm.miq_custom_attributes-name" This is a manual partial backport of b19567a which was in PR: ManageIQ#16211 Before this change, this will spin at 100% cpu in rails console: MiqExpression::Tag.parse("ManageIQ::Providers::CloudManager::Vm.miq_custom_attributes-name") After this change, it returns nil nearly immediately.
d-m-u
pushed a commit
to d-m-u/manageiq
that referenced
this pull request
Jun 6, 2018
https://bugzilla.redhat.com/show_bug.cgi?id=1519809 This will prevent an infinite loop inside regex when parsing strings like "ManageIQ::Providers::CloudManager::Vm.miq_custom_attributes-name" This is a manual partial backport of b19567a which was in PR: ManageIQ#16211 Before this change, this will spin at 100% cpu in rails console: MiqExpression::Tag.parse("ManageIQ::Providers::CloudManager::Vm.miq_custom_attributes-name") After this change, it returns nil nearly immediately.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue:
MiqExpression::Tag.parsedoes not return full model name. Parsing tag likeManageIQ::Providers::CloudManager.managed-service_levelwill returnCloudManagerand>> "CloudManager".safe_constantize => nilhttps://bugzilla.redhat.com/show_bug.cgi?id=1501333
Solution:
keep full model name when parsing
Example: group has assigned filter expression
and tag was assigned to some instances of gce provider
BEFORE:
AFTER:
@miq-bot add-label bug, core
\cc @gtanzillo @imtayadeway