summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authordrbrain <drbrain@b2dd03c8-39d4-4d8f-98ff-823fe69b080e>2013-09-25 00:53:19 +0000
committerdrbrain <drbrain@b2dd03c8-39d4-4d8f-98ff-823fe69b080e>2013-09-25 00:53:19 +0000
commit8eb39185810a59ad8d3aa874ba8f6c9a7b0949ac (patch)
tree790b26abda56b06c1d25a7a0036d882c32df7609
parent61f3a787f6f12c794299871d5739cfdfa01ec617 (diff)
* lib/rubygems: Fix CVE-2013-4363. Miscellaneous minor improvements.
* test/rubygems: Tests for the above. git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@43039 b2dd03c8-39d4-4d8f-98ff-823fe69b080e
-rw-r--r--ChangeLog6
-rw-r--r--lib/rubygems/commands/specification_command.rb2
-rw-r--r--lib/rubygems/commands/unpack_command.rb2
-rw-r--r--lib/rubygems/commands/update_command.rb2
-rw-r--r--lib/rubygems/dependency_resolver.rb209
-rw-r--r--lib/rubygems/dependency_resolver/index_specification.rb7
-rw-r--r--lib/rubygems/remote_fetcher.rb2
-rw-r--r--lib/rubygems/source/local.rb2
-rw-r--r--lib/rubygems/specification.rb6
-rw-r--r--lib/rubygems/version.rb2
-rw-r--r--test/rubygems/test_gem_requirement.rb20
-rw-r--r--test/rubygems/test_gem_specification.rb20
-rw-r--r--test/rubygems/test_gem_version.rb11
13 files changed, 170 insertions, 121 deletions
diff --git a/ChangeLog b/ChangeLog
index 48d9b85..50fa3ba 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+Wed Sep 25 09:53:11 2013 Eric Hodel <drbrain@segment7.net>
+
+ * lib/rubygems: Fix CVE-2013-4363. Miscellaneous minor improvements.
+
+ * test/rubygems: Tests for the above.
+
Tue Sep 24 17:38:56 2013 Nobuyoshi Nakada <nobu@ruby-lang.org>
* string.c (rb_str_inspect): get rid of out-of-bound access.
diff --git a/lib/rubygems/commands/specification_command.rb b/lib/rubygems/commands/specification_command.rb
index d96c8b8..3bc02a9 100644
--- a/lib/rubygems/commands/specification_command.rb
+++ b/lib/rubygems/commands/specification_command.rb
@@ -127,7 +127,7 @@ Specific fields in the specification can be extracted in YAML format:
end
unless options[:all] then
- specs = [specs.sort_by { |s| s.version }.last]
+ specs = [specs.max_by { |s| s.version }]
end
specs.each do |s|
diff --git a/lib/rubygems/commands/unpack_command.rb b/lib/rubygems/commands/unpack_command.rb
index e60e7d9..5a05ad0 100644
--- a/lib/rubygems/commands/unpack_command.rb
+++ b/lib/rubygems/commands/unpack_command.rb
@@ -134,7 +134,7 @@ command help for an example.
specs = dependency.matching_specs
- selected = specs.sort_by { |s| s.version }.last # HACK: hunt last down
+ selected = specs.max_by { |s| s.version }
return Gem::RemoteFetcher.fetcher.download_to_cache(dependency) unless
selected
diff --git a/lib/rubygems/commands/update_command.rb b/lib/rubygems/commands/update_command.rb
index e53798d..4016981 100644
--- a/lib/rubygems/commands/update_command.rb
+++ b/lib/rubygems/commands/update_command.rb
@@ -134,7 +134,7 @@ command to remove old versions.
g.name == spec.name and g.match_platform?
end
- highest_remote_gem = matching_gems.sort_by { |g,_| g.version }.last
+ highest_remote_gem = matching_gems.max_by { |g,_| g.version }
highest_remote_gem ||= [Gem::NameTuple.null]
diff --git a/lib/rubygems/dependency_resolver.rb b/lib/rubygems/dependency_resolver.rb
index 9f3af38..bbb5408 100644
--- a/lib/rubygems/dependency_resolver.rb
+++ b/lib/rubygems/dependency_resolver.rb
@@ -92,6 +92,32 @@ class Gem::DependencyResolver
res.to_a
end
+ ##
+ # Finds the State in +states+ that matches the +conflict+ so that we can try
+ # other possible sets.
+
+ def find_conflict_state conflict, states # :nodoc:
+ until states.empty? do
+ if conflict.for_spec? states.last.spec
+ state = states.last
+ state.conflicts << [state.spec, conflict]
+ return state
+ else
+ states.pop
+ end
+ end
+
+ nil
+ end
+
+ ##
+ # Extracts the specifications that may be able to fulfill +dependency+
+
+ def find_possible dependency # :nodoc:
+ possible = @set.find_all dependency
+ select_local_platforms possible
+ end
+
def handle_conflict(dep, existing)
# There is a conflict! We return the conflict
# object which will be seen by the caller and be
@@ -144,116 +170,119 @@ class Gem::DependencyResolver
# If there is already a spec activated for the requested name...
if specs && existing = specs.find { |s| dep.name == s.name }
-
- # then we're done since this new dep matches the
- # existing spec.
+ # then we're done since this new dep matches the existing spec.
next if dep.matches_spec? existing
conflict = handle_conflict dep, existing
- # Look through the state array and pop State objects
- # until we get back to the State that matches the conflict
- # so that we can try other possible sets.
+ state = find_conflict_state conflict, states
- i = nil
+ return conflict unless state
- until states.empty?
- if conflict.for_spec? states.last.spec
- i = states.last
- i.conflicts << [i.spec, conflict]
- break
- else
- states.pop
- end
- end
+ needed, specs = resolve_for_conflict needed, specs, state
- if i
- # We exhausted the possibles so it's definitely not going to
- # work out, bail out.
+ next
+ end
- if i.possibles.empty?
- raise Gem::ImpossibleDependenciesError.new(i.dep, i.conflicts)
- end
+ possible = find_possible dep
- spec = i.possibles.pop
+ case possible.size
+ when 0
+ resolve_for_zero dep
+ when 1
+ needed, specs =
+ resolve_for_single needed, specs, dep, possible
+ else
+ needed, specs =
+ resolve_for_multiple needed, specs, states, dep, possible
+ end
+ end
- # Recursively call #resolve_for with this spec
- # and add it's dependencies into the picture...
+ specs
+ end
- act = Gem::DependencyResolver::ActivationRequest.new spec, i.dep
+ ##
+ # Rewinds +needed+ and +specs+ to a previous state in +state+ for a conflict
+ # between +dep+ and +existing+.
- needed = requests(spec, act, i.needed)
- specs = Gem::List.prepend(i.specs, act)
+ def resolve_for_conflict needed, specs, state # :nodoc:
+ # We exhausted the possibles so it's definitely not going to work out,
+ # bail out.
+ raise Gem::ImpossibleDependenciesError.new state.dep, state.conflicts if
+ state.possibles.empty?
- next
- else
- return conflict
- end
- end
+ spec = state.possibles.pop
- # Get a list of all specs that satisfy dep and platform
- possible = @set.find_all dep
- possible = select_local_platforms possible
+ # Retry resolution with this spec and add it's dependencies
+ act = Gem::DependencyResolver::ActivationRequest.new spec, state.dep
- case possible.size
- when 0
- @missing << dep
+ needed = requests spec, act, state.needed
+ specs = Gem::List.prepend state.specs, act
- unless @soft_missing
- # If there are none, then our work here is done.
- raise Gem::UnsatisfiableDependencyError, dep
- end
- when 1
- # If there is one, then we just add it to specs
- # and process the specs dependencies by adding
- # them to needed.
+ return needed, specs
+ end
- spec = possible.first
- act = Gem::DependencyResolver::ActivationRequest.new spec, dep, false
+ ##
+ # There are multiple +possible+ specifications for this +dep+. Updates
+ # +needed+, +specs+ and +states+ for further resolution of the +possible+
+ # choices.
+
+ def resolve_for_multiple needed, specs, states, dep, possible # :nodoc:
+ # Sort them so that we try the highest versions first.
+ possible = possible.sort_by do |s|
+ [s.source, s.version, s.platform == Gem::Platform::RUBY ? -1 : 1]
+ end
- specs = Gem::List.prepend specs, act
+ # To figure out which to pick, we keep resolving given each one being
+ # activated and if there isn't a conflict, we know we've found a full set.
+ #
+ # We use an until loop rather than reverse_each to keep the stack short
+ # since we're using a recursive algorithm.
+ spec = possible.pop
- # Put the deps for at the beginning of needed
- # rather than the end to match the depth first
- # searching done by the multiple case code below.
- #
- # This keeps the error messages consistent.
- needed = requests(spec, act, needed)
- else
- # There are multiple specs for this dep. This is
- # the case that this class is built to handle.
-
- # Sort them so that we try the highest versions
- # first.
- possible = possible.sort_by do |s|
- [s.source, s.version, s.platform == Gem::Platform::RUBY ? -1 : 1]
- end
-
- # To figure out which to pick, we keep resolving
- # given each one being activated and if there isn't
- # a conflict, we know we've found a full set.
- #
- # We use an until loop rather than #reverse_each
- # to keep the stack short since we're using a recursive
- # algorithm.
- #
- spec = possible.pop
-
- # We're may need to try all of +possible+, so we setup
- # state to unwind back to current +needed+ and +specs+
- # so we can try another. This is code is what makes the above
- # code in conflict resolution possible.
-
- act = Gem::DependencyResolver::ActivationRequest.new spec, dep
-
- states << State.new(needed, specs, dep, spec, possible, [])
-
- needed = requests(spec, act, needed)
- specs = Gem::List.prepend(specs, act)
- end
- end
+ # We may need to try all of +possible+, so we setup state to unwind back
+ # to current +needed+ and +specs+ so we can try another. This is code is
+ # what makes conflict resolution possible.
- specs
+ act = Gem::DependencyResolver::ActivationRequest.new spec, dep
+
+ states << State.new(needed, specs, dep, spec, possible, [])
+
+ needed = requests spec, act, needed
+ specs = Gem::List.prepend specs, act
+
+ return needed, specs
+ end
+
+ ##
+ # Add the spec from the +possible+ list to +specs+ and process the spec's
+ # dependencies by adding them to +needed+.
+
+ def resolve_for_single needed, specs, dep, possible # :nodoc:
+ spec = possible.first
+ act = Gem::DependencyResolver::ActivationRequest.new spec, dep, false
+
+ specs = Gem::List.prepend specs, act
+
+ # Put the deps for at the beginning of needed
+ # rather than the end to match the depth first
+ # searching done by the multiple case code below.
+ #
+ # This keeps the error messages consistent.
+ needed = requests spec, act, needed
+
+ return needed, specs
+ end
+
+ ##
+ # When there are no possible specifications for +dep+ our work is done.
+
+ def resolve_for_zero dep # :nodoc:
+ @missing << dep
+
+ unless @soft_missing
+ raise Gem::UnsatisfiableDependencyError, dep
+ end
end
##
diff --git a/lib/rubygems/dependency_resolver/index_specification.rb b/lib/rubygems/dependency_resolver/index_specification.rb
index fc40933..44a8438 100644
--- a/lib/rubygems/dependency_resolver/index_specification.rb
+++ b/lib/rubygems/dependency_resolver/index_specification.rb
@@ -1,8 +1,7 @@
##
-# Represents a possible Specification object returned
-# from IndexSet. Used to delay needed to download full
-# Specification objects when only the +name+ and +version+
-# are needed.
+# Represents a possible Specification object returned from IndexSet. Used to
+# delay needed to download full Specification objects when only the +name+
+# and +version+ are needed.
class Gem::DependencyResolver::IndexSpecification
diff --git a/lib/rubygems/remote_fetcher.rb b/lib/rubygems/remote_fetcher.rb
index 6abd6bd..bab96cb 100644
--- a/lib/rubygems/remote_fetcher.rb
+++ b/lib/rubygems/remote_fetcher.rb
@@ -107,7 +107,7 @@ class Gem::RemoteFetcher
return if found.empty?
- spec, source = found.sort_by { |(s,_)| s.version }.last
+ spec, source = found.max_by { |(s,_)| s.version }
download spec, source.uri.to_s
end
diff --git a/lib/rubygems/source/local.rb b/lib/rubygems/source/local.rb
index 642dda1..f7c70de 100644
--- a/lib/rubygems/source/local.rb
+++ b/lib/rubygems/source/local.rb
@@ -88,7 +88,7 @@ class Gem::Source::Local < Gem::Source
end
end
- found.sort_by { |s| s.version }.last
+ found.max_by { |s| s.version }
end
def fetch_spec(name)
diff --git a/lib/rubygems/specification.rb b/lib/rubygems/specification.rb
index 6de8c3b..0952cb2 100644
--- a/lib/rubygems/specification.rb
+++ b/lib/rubygems/specification.rb
@@ -1509,7 +1509,6 @@ class Gem::Specification < Gem::BasicSpecification
# [depending_gem, dependency, [list_of_gems_that_satisfy_dependency]]
def dependent_gems
- # REFACTOR: out = []; each; out; ? Really? No #collect love?
out = []
Gem::Specification.each do |spec|
spec.dependencies.each do |dep|
@@ -1937,7 +1936,6 @@ class Gem::Specification < Gem::BasicSpecification
q.group 2, 'Gem::Specification.new do |s|', 'end' do
q.breakable
- # REFACTOR: each_attr - use in to_yaml as well
@@attributes.each do |attr_name|
current_value = self.send attr_name
if current_value != default_value(attr_name) or
@@ -2186,10 +2184,6 @@ class Gem::Specification < Gem::BasicSpecification
# Returns a Ruby code representation of this specification, such that it can
# be eval'ed and reconstruct the same specification later. Attributes that
# still have their default values are omitted.
- #
- # REFACTOR: This, plus stuff like #ruby_code and #pretty_print, should
- # probably be extracted out into some sort of separate class. SRP, do you
- # speak it!??!
def to_ruby
mark_version
diff --git a/lib/rubygems/version.rb b/lib/rubygems/version.rb
index 2e54646..2ee887e 100644
--- a/lib/rubygems/version.rb
+++ b/lib/rubygems/version.rb
@@ -148,7 +148,7 @@ class Gem::Version
# FIX: These are only used once, in .correct?. Do they deserve to be
# constants?
VERSION_PATTERN = '[0-9]+(?>\.[0-9a-zA-Z]+)*(-[0-9A-Za-z-]+(\.[0-9A-Za-z-]+)*)?' # :nodoc:
- ANCHORED_VERSION_PATTERN = /\A\s*(#{VERSION_PATTERN})*\s*\z/ # :nodoc:
+ ANCHORED_VERSION_PATTERN = /\A\s*(#{VERSION_PATTERN})?\s*\z/ # :nodoc:
##
# A string representation of this Version.
diff --git a/test/rubygems/test_gem_requirement.rb b/test/rubygems/test_gem_requirement.rb
index 1de0f41..01db08e 100644
--- a/test/rubygems/test_gem_requirement.rb
+++ b/test/rubygems/test_gem_requirement.rb
@@ -47,18 +47,20 @@ class TestGemRequirement < Gem::TestCase
end
def test_parse_bad
- e = assert_raises Gem::Requirement::BadRequirementError do
- Gem::Requirement.parse nil
- end
-
- assert_equal 'Illformed requirement [nil]', e.message
+ [
+ nil,
+ '',
+ '! 1',
+ '= junk',
+ '1..2',
+ ].each do |bad|
+ e = assert_raises Gem::Requirement::BadRequirementError do
+ Gem::Requirement.parse bad
+ end
- e = assert_raises Gem::Requirement::BadRequirementError do
- Gem::Requirement.parse ""
+ assert_equal "Illformed requirement [#{bad.inspect}]", e.message
end
- assert_equal 'Illformed requirement [""]', e.message
-
assert_equal Gem::Requirement::BadRequirementError.superclass, ArgumentError
end
diff --git a/test/rubygems/test_gem_specification.rb b/test/rubygems/test_gem_specification.rb
index 51c7be8..2e2074f 100644
--- a/test/rubygems/test_gem_specification.rb
+++ b/test/rubygems/test_gem_specification.rb
@@ -1112,6 +1112,20 @@ dependencies: []
assert_equal [@bonobo, @monkey], @gem.dependencies
end
+ def test_dependent_gems
+ util_setup_deps
+
+ assert_empty @gem.dependent_gems
+
+ bonobo = quick_spec 'bonobo'
+
+ expected = [
+ [@gem, @bonobo, [bonobo]],
+ ]
+
+ assert_equal expected, bonobo.dependent_gems
+ end
+
def test_doc_dir
assert_equal File.join(@gemhome, 'doc', 'a-1'), @a1.doc_dir
end
@@ -1550,13 +1564,13 @@ dependencies: []
@a2.add_runtime_dependency 'b', '1'
@a2.dependencies.first.instance_variable_set :@type, nil
@a2.required_rubygems_version = Gem::Requirement.new '> 0'
- @a2.require_paths << "lib/a/ext"
+ @a2.require_paths << 'other'
ruby_code = @a2.to_ruby
expected = <<-SPEC
# -*- encoding: utf-8 -*-
-# stub: a 2 ruby lib\0lib/a/ext
+# stub: a 2 ruby lib\0other
Gem::Specification.new do |s|
s.name = "a"
@@ -1569,7 +1583,7 @@ Gem::Specification.new do |s|
s.email = "example@example.com"
s.files = ["lib/code.rb"]
s.homepage = "http://example.com"
- s.require_paths = ["lib", "lib/a/ext"]
+ s.require_paths = ["lib", "other"]
s.rubygems_version = "#{Gem::VERSION}"
s.summary = "this is a summary"
diff --git a/test/rubygems/test_gem_version.rb b/test/rubygems/test_gem_version.rb
index 2ba196e..2136034 100644
--- a/test/rubygems/test_gem_version.rb
+++ b/test/rubygems/test_gem_version.rb
@@ -67,12 +67,17 @@ class TestGemVersion < Gem::TestCase
end
def test_initialize_bad
- ["junk", "1.0\n2.0"].each do |bad|
- e = assert_raises ArgumentError do
+ %W[
+ junk
+ 1.0\n2.0
+ 1..2
+ 1.2\ 3.4
+ ].each do |bad|
+ e = assert_raises ArgumentError, bad do
Gem::Version.new bad
end
- assert_equal "Malformed version number string #{bad}", e.message
+ assert_equal "Malformed version number string #{bad}", e.message, bad
end
end