Skip to content
This repository has been archived by the owner on Apr 14, 2021. It is now read-only.

namespace pollution in loading local gemspec #5958

Closed
shyouhei opened this issue Aug 20, 2017 · 2 comments · Fixed by #5960
Closed

namespace pollution in loading local gemspec #5958

shyouhei opened this issue Aug 20, 2017 · 2 comments · Fixed by #5960

Comments

@shyouhei
Copy link
Contributor

What you're trying to accomplish

Clean local variables

The command you ran

git clone https://github.com/shyouhei/bundler-pollution.git bundler-pollution
cd bundler-pollution
bundle install
bundle exec ruby -e 'p local_variables'

What you expected to happen

empty output at the last command

What actually happened

[:must_not_be_seen_from_other_files]

is shown.

The exception backtrace(s), if any

no.

Everything output by running bundle env

## Environment
Bundler   1.15.4
Rubygems  2.6.12
Ruby      2.5.0p-1 (2017-08-11 revision 59574) [x86_64-darwin15]
Git       2.14.1
Platform  x86_64-darwin-15
OpenSSL   LibreSSL 2.5.5

Gemfile

Gemfile

gemspec

Gemfile.lock

PATH
  remote: .
  specs:
    bundler-pollution (0)

GEM
  specs:

PLATFORMS
  ruby

DEPENDENCIES
  bundler-pollution!

BUNDLED WITH
   1.15.4

Gemspecs

bundler-pollution.gemspec

must_not_be_seen_from_other_files = 'bundler-pollution'

Gem::Specification.new {|s|
  s.version = 0;
  s.name = s.summary = s.author = must_not_be_seen_from_other_files
}
@shyouhei
Copy link
Contributor Author

From 0e0bfd7db38102f75308e01e25b937970b992e25 Mon Sep 17 00:00:00 2001
From: "Urabe, Shyouhei" <shyouhei@ruby-lang.org>
Date: Sun, 20 Aug 2017 15:16:18 +0900
Subject: [PATCH] proposed fix.

Signed-off-by: Urabe, Shyouhei <shyouhei@ruby-lang.org>
---
 lib/bundler.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/bundler.rb b/lib/bundler.rb
index c6d68c49d..aba63fb53 100644
--- a/lib/bundler.rb
+++ b/lib/bundler.rb
@@ -479,7 +479,7 @@ EOF
     end
 
     def eval_gemspec(path, contents)
-      eval(contents, TOPLEVEL_BINDING, path.expand_path.to_s)
+      eval(contents, TOPLEVEL_BINDING.dup, path.expand_path.to_s)
     rescue ScriptError, StandardError => e
       msg = "There was an error while loading `#{path.basename}`: #{e.message}"
 
-- 
2.14.1

@segiddins
Copy link
Member

If you could send in a PR with a test for this, that'd be awesome. Thanks!

bundlerbot added a commit that referenced this issue Aug 20, 2017
Avoid namespace pollution

fixes #5958.

### What was the end-user problem that led to this PR?

The problem was that local variables are magically introduced into the global toplevel, when there is a local gemspec that has such local variables.

### What was your diagnosis of the problem?

My diagnosis was that `TOPLEVEL_BINDING` is used with `eval`

### What is your fix for the problem, implemented in this PR?

My fix is to duplicate that binding.

### Why did you choose this fix out of the possible options?

I chose this fix because it is clean and concise.  Other possible options are like reinventions of wheel.
bundlerbot added a commit that referenced this issue Aug 21, 2017
Avoid namespace pollution

fixes #5958.

### What was the end-user problem that led to this PR?

The problem was that local variables are magically introduced into the global toplevel, when there is a local gemspec that has such local variables.

### What was your diagnosis of the problem?

My diagnosis was that `TOPLEVEL_BINDING` is used with `eval`

### What is your fix for the problem, implemented in this PR?

My fix is to duplicate that binding.

### Why did you choose this fix out of the possible options?

I chose this fix because it is clean and concise.  Other possible options are like reinventions of wheel.
bundlerbot added a commit that referenced this issue Aug 22, 2017
Avoid namespace pollution

fixes #5958.

### What was the end-user problem that led to this PR?

The problem was that local variables are magically introduced into the global toplevel, when there is a local gemspec that has such local variables.

### What was your diagnosis of the problem?

My diagnosis was that `TOPLEVEL_BINDING` is used with `eval`

### What is your fix for the problem, implemented in this PR?

My fix is to duplicate that binding.

### Why did you choose this fix out of the possible options?

I chose this fix because it is clean and concise.  Other possible options are like reinventions of wheel.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants