From 1f7545d2be0f2f39a8d1bae45a09d178506d6201 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Wed, 4 Oct 2017 08:49:09 -0700 Subject: [PATCH 01/16] Change StringValuePtr to StringValueCStr StringValuePtr doesn't guarantee a NULL byte at the end of the char * it returns. The for loop in the `parse_version_number` depends on a NULL byte in the string in order to stop the loop. Since `StringValuePtr` doesn't guarantee a NULL byte in the `char *`, it's possible the for loop could read past the end of the string, and `offset` would end up being larger than the number of bytes that are actually in the string. --- ext/version_sorter/version_sorter.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/version_sorter/version_sorter.c b/ext/version_sorter/version_sorter.c index 8506a31..416373e 100644 --- a/ext/version_sorter/version_sorter.c +++ b/ext/version_sorter/version_sorter.c @@ -186,7 +186,7 @@ rb_version_sort_1(VALUE rb_self, VALUE rb_version_array, compare_callback_t cmp) for (i = 0; i < length; ++i) { VALUE rb_version = rb_ary_entry(rb_version_array, i); - versions[i] = parse_version_number(StringValuePtr(rb_version)); + versions[i] = parse_version_number(StringValueCStr(rb_version_string)); versions[i]->rb_version = rb_version; } From 56d0cd096365a8a9e3abde3f147a49e9a23d76f8 Mon Sep 17 00:00:00 2001 From: Keith Cirkel Date: Wed, 21 Jun 2017 12:15:27 +0100 Subject: [PATCH 02/16] test: ensure older rubygems versions dont fail on semver tests --- test/version_sorter_test.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/version_sorter_test.rb b/test/version_sorter_test.rb index 84e0a66..2c797e3 100644 --- a/test/version_sorter_test.rb +++ b/test/version_sorter_test.rb @@ -13,6 +13,10 @@ def test_sorts_versions_correctly def test_sorts_versions_like_rubygems versions = %w(1.0.9.b 1.0.9 1.0.10 2.0 3.1.4.2 1.0.9a 2.0rc2 2.0-rc1) + if (Gem.rubygems_version < Gem::Version.new('2.1.0')) + # Old versions of RubyGems cannot parse semver versions like `2.0-rc1` + versions.pop() + end sorted_versions = versions.sort_by { |v| Gem::Version.new(v) } assert_equal sorted_versions, VersionSorter.sort(versions) From 43fd2a3477185a4678b35c16185f895bb6666af5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Fri, 13 Oct 2017 20:42:57 +0200 Subject: [PATCH 03/16] Support for old Rubygems in tests This fixes the test on Rubygems 1.8.23.2 --- test/version_sorter_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/version_sorter_test.rb b/test/version_sorter_test.rb index 2c797e3..654d430 100644 --- a/test/version_sorter_test.rb +++ b/test/version_sorter_test.rb @@ -13,7 +13,7 @@ def test_sorts_versions_correctly def test_sorts_versions_like_rubygems versions = %w(1.0.9.b 1.0.9 1.0.10 2.0 3.1.4.2 1.0.9a 2.0rc2 2.0-rc1) - if (Gem.rubygems_version < Gem::Version.new('2.1.0')) + if !Gem.respond_to?(:rubygems_version) || Gem.rubygems_version < Gem::Version.new('2.1.0') # Old versions of RubyGems cannot parse semver versions like `2.0-rc1` versions.pop() end From 086592cb66309ba6054b2cb8396afe4a63d8862f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Fri, 13 Oct 2017 20:43:58 +0200 Subject: [PATCH 04/16] Avoid Bundler bug on Travis --- .travis.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.travis.yml b/.travis.yml index 38862dc..ce29241 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,6 +1,8 @@ sudo: false language: ruby script: script/test +before_install: + - bundle --version 2>/dev/null | grep -q '1.7.6' && gem install bundler -v 1.11.2 || true rvm: - "1.8.7" - "1.9.3" From adb1793c2d2b364a261275744bfbcb01dce94f9e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Fri, 13 Oct 2017 20:45:17 +0200 Subject: [PATCH 05/16] Test against Ruby 2.3 and Ruby 2.4 in CI --- .travis.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.travis.yml b/.travis.yml index ce29241..0af306e 100644 --- a/.travis.yml +++ b/.travis.yml @@ -9,3 +9,5 @@ rvm: - "2.0" - "2.1" - "2.2" + - "2.3" + - "2.4" From 5351df08b848b7d30e2c9e7be4f0b09e31358b45 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Fri, 13 Oct 2017 20:50:07 +0200 Subject: [PATCH 06/16] Fix variable reference in backport to 2-0-stable --- ext/version_sorter/version_sorter.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/version_sorter/version_sorter.c b/ext/version_sorter/version_sorter.c index 416373e..84284c7 100644 --- a/ext/version_sorter/version_sorter.c +++ b/ext/version_sorter/version_sorter.c @@ -186,7 +186,7 @@ rb_version_sort_1(VALUE rb_self, VALUE rb_version_array, compare_callback_t cmp) for (i = 0; i < length; ++i) { VALUE rb_version = rb_ary_entry(rb_version_array, i); - versions[i] = parse_version_number(StringValueCStr(rb_version_string)); + versions[i] = parse_version_number(StringValueCStr(rb_version)); versions[i]->rb_version = rb_version; } From 9349b9aa67c108d96c6df58433e475906f4a7ec3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Fri, 13 Oct 2017 20:54:49 +0200 Subject: [PATCH 07/16] Update version in Gemfile.lock --- Gemfile.lock | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Gemfile.lock b/Gemfile.lock index 6de7930..5f5967b 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,7 +1,7 @@ PATH remote: . specs: - version_sorter (1.1.1) + version_sorter (2.0.0) GEM remote: https://rubygems.org/ @@ -19,3 +19,6 @@ DEPENDENCIES rake-compiler test-unit (~> 2.0.4) version_sorter! + +BUNDLED WITH + 1.15.4 From 9be73b1fd9fa1954120b7e685c1dac4c36d57085 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Fri, 13 Oct 2017 21:04:18 +0200 Subject: [PATCH 08/16] version_sorter 2.0.1 --- Gemfile.lock | 2 +- version_sorter.gemspec | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 5f5967b..d55e4f5 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,7 +1,7 @@ PATH remote: . specs: - version_sorter (2.0.0) + version_sorter (2.0.1) GEM remote: https://rubygems.org/ diff --git a/version_sorter.gemspec b/version_sorter.gemspec index 1a7f990..840f455 100644 --- a/version_sorter.gemspec +++ b/version_sorter.gemspec @@ -3,7 +3,7 @@ require 'rbconfig' Gem::Specification.new do |s| s.name = 'version_sorter' - s.version = '2.0.0' + s.version = '2.0.1' s.authors = ["Chris Wanstrath", "K. Adam Christensen"] s.email = 'chris@ozmm.org' s.homepage = 'https://github.com/defunkt/version_sorter' From 7df1df2bfb8d1742cd6771bdc9d3c9ab278b71e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Fri, 13 Oct 2017 21:11:07 +0200 Subject: [PATCH 09/16] Add `release` script --- script/release | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100755 script/release diff --git a/script/release b/script/release new file mode 100755 index 0000000..0a568e3 --- /dev/null +++ b/script/release @@ -0,0 +1,27 @@ +#!/bin/bash +# Edit the version string in the gemspec, then execute this script to: +# +# 1. Commit the version change +# 2. Tag the release +# 3. Push the tag and the current branch to GitHub +# 4. Publish the gem to RubyGems +# +set -eu + +fields=( $(gem build *.gemspec | awk '/Name:|Version:/ {print $2}') ) +name="${fields[0]}" +version="${fields[1]}" +gem="${name}-${version}.gem" +[ -n "$version" ] || exit 1 +trap "rm -f '$gem'" EXIT + +bundle install +script/test + +if ! git rev-parse --verify --quiet "refs/tags/v${version}" >/dev/null; then + git commit --allow-empty -a -m "$name $version" + git tag "v${version}" +fi + +git push origin HEAD "v${version}" +gem push "$gem" From ccc5f596181ff03c0debbae633df855cf44a9aea Mon Sep 17 00:00:00 2001 From: Ashe Connor Date: Mon, 29 Jan 2018 16:24:27 +1100 Subject: [PATCH 10/16] correctly shift 64 bit values --- ext/version_sorter/version_sorter.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/version_sorter/version_sorter.c b/ext/version_sorter/version_sorter.c index 84284c7..e761f73 100644 --- a/ext/version_sorter/version_sorter.c +++ b/ext/version_sorter/version_sorter.c @@ -138,7 +138,7 @@ parse_version_number(const char *string) version->comp[comp_n].string.len = offset - start; } else { version->comp[comp_n].number = number; - num_flags |= (1 << comp_n); + num_flags |= (1ull << comp_n); } comp_n++; continue; From 5f1c671006aeb0709fe12d5c39ebc915b631b8c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Mon, 29 Jan 2018 14:10:01 +0100 Subject: [PATCH 11/16] version_sorter 2.0.2 --- Gemfile.lock | 4 ++-- version_sorter.gemspec | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index d55e4f5..6249036 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,7 +1,7 @@ PATH remote: . specs: - version_sorter (2.0.1) + version_sorter (2.0.2) GEM remote: https://rubygems.org/ @@ -21,4 +21,4 @@ DEPENDENCIES version_sorter! BUNDLED WITH - 1.15.4 + 1.16.1 diff --git a/version_sorter.gemspec b/version_sorter.gemspec index 840f455..849c089 100644 --- a/version_sorter.gemspec +++ b/version_sorter.gemspec @@ -3,7 +3,7 @@ require 'rbconfig' Gem::Specification.new do |s| s.name = 'version_sorter' - s.version = '2.0.1' + s.version = '2.0.2' s.authors = ["Chris Wanstrath", "K. Adam Christensen"] s.email = 'chris@ozmm.org' s.homepage = 'https://github.com/defunkt/version_sorter' From 4f47c858163298571b9dbd527dd621a1887317b0 Mon Sep 17 00:00:00 2001 From: Ashe Connor Date: Thu, 1 Feb 2018 15:17:05 +1100 Subject: [PATCH 12/16] avoid signed overflow --- ext/version_sorter/version_sorter.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/ext/version_sorter/version_sorter.c b/ext/version_sorter/version_sorter.c index e761f73..e72f778 100644 --- a/ext/version_sorter/version_sorter.c +++ b/ext/version_sorter/version_sorter.c @@ -15,6 +15,7 @@ #include #define min(a, b) ((a) < (b) ? (a) : (b)) +#define max(a, b) ((a) > (b) ? (a) : (b)) typedef int compare_callback_t(const void *, const void *); struct version_number { @@ -56,7 +57,8 @@ compare_version_number(const struct version_number *a, int cmp = 0; if (num_a) { - cmp = (int)ca->number - (int)cb->number; + int64_t cmp64 = (int64_t)ca->number - (int64_t)cb->number; + cmp = (int)max(INT_MIN, min(INT_MAX, cmp64)); } else { cmp = strchunk_cmp( a->original, &ca->string, From ff48c442806ef5d777f8212c3770c8cfb227e417 Mon Sep 17 00:00:00 2001 From: Phil Turnbull Date: Mon, 5 Feb 2018 13:03:15 -0500 Subject: [PATCH 13/16] Clamp comparisons to -1, 0, 1 fca9ae5c "avoid signed overflow" can still cause undefined behavior because `compare_version_number` can return `INT_MIN` which `version_compare_cb_r` then tries to negate: ``` ../../../../ext/version_sorter/version_sorter.c:94:9: runtime error: negation of -2147483648 cannot be represented in type 'int'; cast to an unsigned type to negate this value to itself #0 0x7f9cde1fe1ba in version_compare_cb_r /tmp/x86_64-linux-gnu/version_sorter/2.3.1/../../../../ext/version_sorter/version_sorter.c:94:9 #1 0x7f9cde75c231 (/lib/x86_64-linux-gnu/libc.so.6+0x39231) #2 0x7f9cde75c69e in qsort_r (/lib/x86_64-linux-gnu/libc.so.6+0x3969e) #3 0x7f9cde1fd68c in rb_version_sort_1 /tmp/x86_64-linux-gnu/version_sorter/2.3.1/../../../../ext/version_sorter/version_sorter.c:202:2 ``` --- ext/version_sorter/version_sorter.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/version_sorter/version_sorter.c b/ext/version_sorter/version_sorter.c index e72f778..3ceac87 100644 --- a/ext/version_sorter/version_sorter.c +++ b/ext/version_sorter/version_sorter.c @@ -58,7 +58,7 @@ compare_version_number(const struct version_number *a, if (num_a) { int64_t cmp64 = (int64_t)ca->number - (int64_t)cb->number; - cmp = (int)max(INT_MIN, min(INT_MAX, cmp64)); + cmp = (int)max(-1, min(1, cmp64)); } else { cmp = strchunk_cmp( a->original, &ca->string, From 8d8b1fa86bf530a5f72f89a0a77f1cdc55e69153 Mon Sep 17 00:00:00 2001 From: Ashe Connor Date: Wed, 7 Feb 2018 13:45:53 +1100 Subject: [PATCH 14/16] add int negate test --- test/version_sorter_test.rb | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/version_sorter_test.rb b/test/version_sorter_test.rb index 654d430..5b81617 100644 --- a/test/version_sorter_test.rb +++ b/test/version_sorter_test.rb @@ -75,6 +75,12 @@ def test_rsort_bang assert_equal ["10.0", "2.0", "1.0"], versions end + def test_int_negate + a = ["0", "2147483648"] + assert_equal a, VersionSorter.sort(a) + assert_equal a.reverse, VersionSorter.rsort(a) + end + def shuffle(array) array, result = array.dup, [] result << array.delete_at(rand(array.size)) until array.size.zero? From 0faa512da9f8210e2889f96828d68ddf0aa422fb Mon Sep 17 00:00:00 2001 From: Ashe Connor Date: Wed, 7 Feb 2018 14:25:53 +1100 Subject: [PATCH 15/16] memory fixes from #17 --- ext/version_sorter/version_sorter.c | 60 ++++++++++++++++++++++------- 1 file changed, 46 insertions(+), 14 deletions(-) diff --git a/ext/version_sorter/version_sorter.c b/ext/version_sorter/version_sorter.c index 3ceac87..0e760b8 100644 --- a/ext/version_sorter/version_sorter.c +++ b/ext/version_sorter/version_sorter.c @@ -171,36 +171,68 @@ parse_version_number(const char *string) return version; } +struct sort_context { + VALUE rb_self; + VALUE rb_version_array; + compare_callback_t *cmp; + struct version_number **versions; +}; + static VALUE -rb_version_sort_1(VALUE rb_self, VALUE rb_version_array, compare_callback_t cmp) +rb_version_sort_1_cb(VALUE arg) { - struct version_number **versions; + struct sort_context *context = (struct sort_context *)arg; long length, i; VALUE *rb_version_ptr; + length = RARRAY_LEN(context->rb_version_array); + for (i = 0; i < length; ++i) { + VALUE rb_version = rb_ary_entry(context->rb_version_array, i); + context->versions[i] = parse_version_number(StringValueCStr(rb_version)); + context->versions[i]->rb_version = rb_version; + } + + qsort(context->versions, length, sizeof(struct version_number *), context->cmp); + rb_version_ptr = RARRAY_PTR(context->rb_version_array); + + for (i = 0; i < length; ++i) { + rb_version_ptr[i] = context->versions[i]->rb_version; + } + + return context->rb_version_array; +} + +static VALUE +rb_version_sort_1(VALUE rb_self, VALUE rb_version_array, compare_callback_t cmp) +{ + long length, i; + int exception; + Check_Type(rb_version_array, T_ARRAY); length = RARRAY_LEN(rb_version_array); if (!length) return rb_ary_new(); - versions = xcalloc(length, sizeof(struct version_number *)); + struct sort_context context = { + rb_self, + rb_version_array, + cmp, + xcalloc(length, sizeof(struct version_number *)), + }; + + VALUE result = rb_protect(rb_version_sort_1_cb, (VALUE)&context, &exception); for (i = 0; i < length; ++i) { - VALUE rb_version = rb_ary_entry(rb_version_array, i); - versions[i] = parse_version_number(StringValueCStr(rb_version)); - versions[i]->rb_version = rb_version; + xfree(context.versions[i]); } + xfree(context.versions); - qsort(versions, length, sizeof(struct version_number *), cmp); - rb_version_ptr = RARRAY_PTR(rb_version_array); - - for (i = 0; i < length; ++i) { - rb_version_ptr[i] = versions[i]->rb_version; - xfree(versions[i]); + if (exception) { + rb_jump_tag(exception); } - xfree(versions); - return rb_version_array; + + return result; } static VALUE From f27d85bb39867f61ed36f8267e980409c071f850 Mon Sep 17 00:00:00 2001 From: Ashe Connor Date: Mon, 12 Feb 2018 11:13:32 +1100 Subject: [PATCH 16/16] version_sorter 2.0.3 --- Gemfile.lock | 2 +- version_sorter.gemspec | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 6249036..8183e50 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,7 +1,7 @@ PATH remote: . specs: - version_sorter (2.0.2) + version_sorter (2.0.3) GEM remote: https://rubygems.org/ diff --git a/version_sorter.gemspec b/version_sorter.gemspec index 849c089..f360353 100644 --- a/version_sorter.gemspec +++ b/version_sorter.gemspec @@ -3,7 +3,7 @@ require 'rbconfig' Gem::Specification.new do |s| s.name = 'version_sorter' - s.version = '2.0.2' + s.version = '2.0.3' s.authors = ["Chris Wanstrath", "K. Adam Christensen"] s.email = 'chris@ozmm.org' s.homepage = 'https://github.com/defunkt/version_sorter'