Skip to content

(#MODULES-865) validate_re cannot match numeric regex in all cases #424

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

Closed
wants to merge 1 commit into from

Conversation

ripzura
Copy link

@ripzura ripzura commented Mar 8, 2015

The validate_re function now tries to convert it's first argument to
a string. This will ensure that numeric, boolean, symbols, arrays and hashes can be matched
by a regex since regex matching can only happen on strings.

validate_re(1, '^\d+$') would previously fail because the first argument is a Fixnum.
This patch ensures the first argument is cast to a string so this would match as one would expect.

The validate_re function now tries to convert it's first argument to
a string. This will ensure that numeric, boolean, symbols, arrays and hashes can be matched
by a regex since regex matching can only happen on strings.

validate_re(1, '^\d+$') would previously fail because the first argument is a Fixnum.
This patch ensures the first argument is cast to a string so this would match as one would expect.
@ripzura
Copy link
Author

ripzura commented Mar 17, 2015

Test seems to fail for arrays and hashes on puppet 2.7.x. So my fix does not work for Arrays and Hashes on 2.7.x (function reverts to it's old behavior) It does work for symbols, integers and floats however.

Since I cannot test myslf on 2.7.x I would like some comments on how to proceed. Is this critical?
I thought the current stdlib wasn't supposed to be compatible with 2.7.x anyway so any pointers on how to get this patch merged are welcome.

@DavidS
Copy link
Contributor

DavidS commented Apr 9, 2015

I'm currently working on a major cleanup for the next major stdlib release. I'll see to it that this will be fixed there.

@puppet-community-ci
Copy link

The result of the test was: PASS
Details at http://planck.nibalizer.com/buildlogs/puppetlabs+puppetlabs-stdlib+424+1432894632+PASS

I am a beta ci bot. I am probably lying to you.
You can contact nibalizer for more details.

@puppet-community-ci
Copy link

The result of the test was: PASS
Details at http://ci.puppet.community/buildlogs/puppetlabs+puppetlabs-stdlib+424+1433065133+PASS

I am a beta ci bot. I am probably lying to you.
You can contact nibalizer for more details.

@DavidS
Copy link
Contributor

DavidS commented Sep 17, 2015

Actually looking through the code, this change would break backwards compatibility in exactly the case where it's needed the most: when the input is unexpected. I've replaced this with #526, clarifying the docs and the error message.

@DavidS DavidS closed this Sep 17, 2015
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.

6 participants