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

Fix get version gemfile regexp and add tests #370

Merged

Conversation

themetric
Copy link
Contributor

This hopefully fixes #359 by matching both end of line characters \r\n and \n. Also added a fix for matching variable length version numbers like 1.2 instead of just ones of length three like 1.2.3.

Do we need to support formats like these?

haml (>= 3.0, < 5.0)
mime-types (>= 1.16)
ruby_parser (~> 3.1)

@presidentbeef
Copy link
Owner

Hi Paul,

We are only looking at Gemfile.lock, all versions should be exact.

This actually matches almost anything inside the parentheses:

1.9.3p392 :001 > name = "hi"
 => "hi"
1.9.3p392 :002 > r = /\s#{name} \((\w+.+)\)(?:\n|\r\n)/
 => /\shi \((\w+.+)\)(?:\n|\r\n)/
1.9.3p392 :003 > r.match(" hi (blah)\n")
 => #<MatchData " hi (blah)\n" 1:"blah">

It doesn't really matter though (in fact, the original regex isn't that great either, since it doesn't escape .).

Can you rebase this and squash the commits to make it a little cleaner? Feel free to use a new branch if that's easier.

Thanks!

@themetric
Copy link
Contributor Author

Good catch, I just updated the expression to be a little more picky about that. I'll rebase and make everything more organized.... 😃

tests for gem processor regexp get_version

revert namespacing in brakeman test oops

try new end of line regex match and refactor tests

update regexp and test to match \r\n or \n

add another test for matching short gem versions

make regexp match only dot seperated characters of any length
@themetric
Copy link
Contributor Author

Just squashed it all together into a single commit, much cleaner, I like that! Should be ready to merge. One test failing test_basic_auth_with_password but not sure if it's related to the changes I made. Thanks for looking into it...

presidentbeef added a commit that referenced this pull request Jul 17, 2013
Fix get version gemfile regexp and add tests
@presidentbeef presidentbeef merged commit b378274 into presidentbeef:master Jul 17, 2013
@presidentbeef
Copy link
Owner

That test failure is just because you need to update ruby_parser :)

Thanks!

@themetric themetric deleted the fix_get_version_gemfile branch July 17, 2013 16:25
Repository owner locked and limited conversation to collaborators Feb 16, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

get_version() regexp fails when Gemfile.lock has CRLF line terminators
2 participants