dotenv icon indicating copy to clipboard operation
dotenv copied to clipboard

`Dotenv::Railtie.overload` violates policy about `env` files priority

Open domcermak opened this issue 4 years ago • 2 comments

Expected behavior

.env.development should have higher priority then .env while using Dotenv::Railtie.overload

Actual behavior

#overload method violates policy about env files priority.

Let's say RAILS_ENV=development and there are files .env.development and .env in the rails root directory.

Case of #load method

Dotenv::Railtie.load

calls

def self.load
  instance.load
end

which calls

def load
   Dotenv.load(*dotenv_files)
end

which calls

def load(*filenames)
  with(*filenames) do |f|
    ignoring_nonexistent_files do
      env = Environment.new(f, true)
      instrument("dotenv.load", env: env) { env.apply }
    end
  end
end

which calls

def apply
  each { |k, v| ENV[k] ||= v }
end

which uses the ||= operator, so when it is used with files in order given by

def dotenv_files
  [
    root.join(".env.#{Rails.env}.local"),
    (root.join(".env.local") unless Rails.env.test?),
    root.join(".env.#{Rails.env}"),
    root.join(".env")
  ].compact
end

then the #load method fulfills the priority policy mentioned in readme. So variables in .env.development have higher priority then .env. This is correct behaviour. Unlike the #overloadmethod, which I am going to describe now.

Case of #overload method

Dotenv::Railtie.overload

calls

def overload
  Dotenv.overload(*dotenv_files)
end

which calls

def overload(*filenames)
  with(*filenames) do |f|
    ignoring_nonexistent_files do
      env = Environment.new(f, false)
      instrument("dotenv.overload", env: env) { env.apply! }
    end
  end
end

which calls

def apply!
  each { |k, v| ENV[k] = v }
end

which uses the = operator, so the ENV values are really overriden. But with the given dotenv_files from

def dotenv_files
  [
    root.join(".env.#{Rails.env}.local"),
    (root.join(".env.local") unless Rails.env.test?),
    root.join(".env.#{Rails.env}"),
    root.join(".env")
  ].compact
end

this means, that variables from .env file have the highest priority, because they are processed after all other env files. This violates the policy about env files priority. It also violates the comment by the #overload method definition "Same as load, but will override existing values in ENV".

There is a test for the #overload method in rails_spec.rb on line 116.

it "overloads .env.test with .env" do
   expect(ENV["DOTENV"]).to eql("true")
end

which confirms the priority policy is violated, because "overloads .env.test with .env". Which leaves me confused, because documentation says there is a priority policy, and then the test tests that the policy is being violated intentionally.

Expected solution

Change #overload method in Dotenv::Railtie from

def overload
  Dotenv.overload(*dotenv_files)
end

to

def overload
  Dotenv.overload(*dotenv_files.reverse)
end

or a change of documentation.

System configuration

dotenv version: 2.7.6

Rails version: 5.0.7.2

Ruby version: 2.6.6

domcermak avatar Jan 30 '21 21:01 domcermak

Guys, I read the source code and identified the same problem. This problem is without solution since January, 2021. Is there any preview about you will change that? I didn't see any Opened Pull Request about that.

feihokpai avatar Jun 21 '22 18:06 feihokpai

@feihokpai you can always go with a monkey patch just like we did in our project but I created a PR with a fix I would prefer so we can hopefully get it fixed.

domcermak avatar Jun 25 '22 11:06 domcermak