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

(IAC-1414) Throw error in range() function when step size invalid #1161

Merged
merged 1 commit into from
Feb 5, 2021
Merged

(IAC-1414) Throw error in range() function when step size invalid #1161

merged 1 commit into from
Feb 5, 2021

Conversation

sanfrancrisko
Copy link
Contributor

@sanfrancrisko sanfrancrisko commented Feb 5, 2021

Prior to this commit, on Ruby 2.7, the range() method would NOT
throw an ArgumentError exception if it was passed an invalid value as the
step size (3rd arg). Instead, it would return an array of size
1,000,000 with repeated integer values of the 2nd arg.

The function invokes Ruby's in built range method and step to
define a step size to increment / decrement by and then passes the
output to first, limiting to the first 1,000,000 values.

On Ruby < 2.7, step would throw an ArgumentError if the step
size passed was 0:

1..2).step(0).first(1_000_000).to_a
Traceback (most recent call last):
        5: from ~/.rvm/rubies/ruby-2.5.8/bin/irb:11:in `<main>'
        4: from (irb):7
        3: from (irb):7:in `first'
        2: from (irb):7:in `each'
        1: from (irb):7:in `step'
ArgumentError (step can't be 0)

However, on Ruby 2.7, the step method returns an Enumerator that
will then subsequently generate an Array of the size specified when
passed to first, made up of the 2nd arg value (last):

(1..2).step(0).first(10).to_a
 => [1, 1, 1, 1, 1, 1, 1, 1, 1, 1]

This is an issue, as the 3rd arg is passed to to_i, which then
becomes the step size. For any values that cannot be converted to
a positive / negative (or legit 0) Integer, a 0 is returned.

This commit will perform a check on the value of step and throw
an ArgumentError if it is not zero.

@puppet-community-rangefinder
Copy link

range is a function

Breaking changes to this file MAY impact these 22 modules (near match):

This module is declared in 319 of 576 indexed public Puppetfiles.


These results were generated with Rangefinder, a tool that helps predict the downstream impact of breaking changes to elements used in Puppet modules. You can run this on the command line to get a full report.

Exact matches are those that we can positively identify via namespace and the declaring modules' metadata. Non-namespaced items, such as Puppet 3.x functions, will always be reported as near matches only.

@sanfrancrisko sanfrancrisko changed the title (IAC-1228) Fix multiple spec test issues (IAC-1414) Throw error in range() function when step size invalid Feb 5, 2021
@codecov-io
Copy link

codecov-io commented Feb 5, 2021

Codecov Report

Merging #1161 (14e6d11) into main (9a8f097) will decrease coverage by 0.42%.
The diff coverage is 0.00%.

Impacted file tree graph

@@           Coverage Diff            @@
##            main   #1161      +/-   ##
========================================
- Coverage   3.47%   3.04%   -0.43%     
========================================
  Files        185     185              
  Lines       5184    5220      +36     
========================================
- Hits         180     159      -21     
- Misses      5004    5061      +57     
Impacted Files Coverage Δ
lib/puppet/parser/functions/range.rb 0.00% <0.00%> (ø)
lib/puppet/parser/functions/dig44.rb 0.00% <0.00%> (-80.00%) ⬇️
lib/facter/package_provider.rb 57.14% <0.00%> (-28.58%) ⬇️
lib/facter/root_home.rb 48.00% <0.00%> (-17.39%) ⬇️
lib/facter/pe_version.rb 62.50% <0.00%> (-6.25%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9a8f097...14e6d11. Read the comment docs.

Prior to this commit, on Ruby 2.7, the range() method would NOT
throw an ArgumentError exception if it was passed an invalid value as the
step size (3rd arg). Instead, it would return an array of size
1,000,000 with repeated integer values of the 2nd arg.

The function invokes Ruby's in built range method and `step` to
define a step size to increment / decrement by  and then passes the
output to `first`, limiting to the first 1,000,000 values.

On Ruby < 2.7, `step` would throw an `ArgumentError` if the step
size passed was `0`:

```
1..2).step(0).first(1_000_000).to_a
Traceback (most recent call last):
        5: from ~/.rvm/rubies/ruby-2.5.8/bin/irb:11:in `<main>'
        4: from (irb):7
        3: from (irb):7:in `first'
        2: from (irb):7:in `each'
        1: from (irb):7:in `step'
ArgumentError (step can't be 0)
```

However, on Ruby 2.7, the `step` method returns an Enumerator that
will then subsequently generate an Array of the size specified when
passed to `first`, made up of the 2nd arg value (last):

```
(1..2).step(0).first(10).to_a
 => [1, 1, 1, 1, 1, 1, 1, 1, 1, 1]
```

This is an issue, as the 3rd arg is passed to `to_i`, which then
becomes the step size. For any values that cannot be converted to
a positive / negative (or legit 0) Integer, a 0 is returned.

This commit will perform a check on the value of `step` and throw
an ArgumentError if it is not zero.
@michaeltlombardi michaeltlombardi merged commit 2dedbb9 into puppetlabs:main Feb 5, 2021
@sanfrancrisko sanfrancrisko deleted the IAC-1414/main/fix_range_invalid_step_arg branch February 8, 2021 09:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants