Merge branch '5-0-beta-sec'

* 5-0-beta-sec:
  bumping version
  fix version update task to deal with .beta1.1
  Eliminate instance level writers for class accessors
  allow :file to be outside rails root, but anything else must be inside the rails view directory
  Don't short-circuit reject_if proc
  stop caching mime types globally
  use secure string comparisons for basic auth username / password
This commit is contained in:
Aaron Patterson 2016-01-25 11:25:11 -08:00
commit 6dfab475ca
32 changed files with 168 additions and 41 deletions

View File

@ -1 +1 @@
5.0.0.beta1
5.0.0.beta1.1

View File

@ -8,7 +8,7 @@ module ActionCable
MAJOR = 5
MINOR = 0
TINY = 0
PRE = "beta1"
PRE = "beta1.1"
STRING = [MAJOR, MINOR, TINY, PRE].compact.join(".")
end

View File

@ -8,7 +8,7 @@ module ActionMailer
MAJOR = 5
MINOR = 0
TINY = 0
PRE = "beta1"
PRE = "beta1.1"
STRING = [MAJOR, MINOR, TINY, PRE].compact.join(".")
end

View File

@ -82,7 +82,13 @@ module AbstractController
# <tt>render :file => "foo/bar"</tt>.
# :api: plugin
def _normalize_args(action=nil, options={})
if action.is_a? Hash
case action
when ActionController::Parameters
unless action.permitted?
raise ArgumentError, "render parameters are not permitted"
end
action
when Hash
action
else
options

View File

@ -1,4 +1,5 @@
require 'base64'
require 'active_support/security_utils'
module ActionController
# Makes it dead easy to do HTTP Basic, Digest and Token authentication.
@ -68,7 +69,11 @@ module ActionController
def http_basic_authenticate_with(options = {})
before_action(options.except(:name, :password, :realm)) do
authenticate_or_request_with_http_basic(options[:realm] || "Application") do |name, password|
name == options[:name] && password == options[:password]
# This comparison uses & so that it doesn't short circuit and
# uses `variable_size_secure_compare` so that length information
# isn't leaked.
ActiveSupport::SecurityUtils.variable_size_secure_compare(name, options[:name]) &
ActiveSupport::SecurityUtils.variable_size_secure_compare(password, options[:password])
end
end
end

View File

@ -31,7 +31,7 @@ module Mime
SET = Mimes.new
EXTENSION_LOOKUP = {}
LOOKUP = Hash.new { |h, k| h[k] = Type.new(k) unless k.blank? }
LOOKUP = {}
class << self
def [](type)
@ -177,7 +177,7 @@ module Mime
end
def lookup(string)
LOOKUP[string]
LOOKUP[string] || Type.new(string)
end
def lookup_by_extension(extension)
@ -255,9 +255,12 @@ module Mime
end
end
attr_reader :hash
def initialize(string, symbol = nil, synonyms = [])
@symbol, @synonyms = symbol, synonyms
@string = string
@hash = [@string, @synonyms, @symbol].hash
end
def to_s
@ -291,6 +294,13 @@ module Mime
end
end
def eql?(other)
super || (self.class == other.class &&
@string == other.string &&
@synonyms == other.synonyms &&
@symbol == other.symbol)
end
def =~(mime_type)
return false unless mime_type
regexp = Regexp.new(Regexp.quote(mime_type.to_s))
@ -303,6 +313,10 @@ module Mime
def all?; false; end
protected
attr_reader :string, :synonyms
private
def to_ary; end

View File

@ -8,7 +8,7 @@ module ActionPack
MAJOR = 5
MINOR = 0
TINY = 0
PRE = "beta1"
PRE = "beta1.1"
STRING = [MAJOR, MINOR, TINY, PRE].compact.join(".")
end

View File

@ -62,6 +62,16 @@ class TestController < ActionController::Base
end
end
def dynamic_render
render params[:id] # => String, AC:Params
end
def dynamic_render_with_file
# This is extremely bad, but should be possible to do.
file = params[:id] # => String, AC:Params
render file: file
end
class Collection
def initialize(records)
@records = records
@ -243,6 +253,27 @@ end
class ExpiresInRenderTest < ActionController::TestCase
tests TestController
def test_dynamic_render_with_file
# This is extremely bad, but should be possible to do.
assert File.exist?(File.join(File.dirname(__FILE__), '../../test/abstract_unit.rb'))
response = get :dynamic_render_with_file, params: { id: '../\\../test/abstract_unit.rb' }
assert_equal File.read(File.join(File.dirname(__FILE__), '../../test/abstract_unit.rb')),
response.body
end
def test_dynamic_render
assert File.exist?(File.join(File.dirname(__FILE__), '../../test/abstract_unit.rb'))
assert_raises ActionView::MissingTemplate do
get :dynamic_render, params: { id: '../\\../test/abstract_unit.rb' }
end
end
def test_dynamic_render_file_hash
assert_raises ArgumentError do
get :dynamic_render, params: { id: { file: '../\\../test/abstract_unit.rb' } }
end
end
def test_expires_in_header
get :conditional_hello_with_expires_in
assert_equal "max-age=60, private", @response.headers["Cache-Control"]

View File

@ -8,7 +8,7 @@ module ActionView
MAJOR = 5
MINOR = 0
TINY = 0
PRE = "beta1"
PRE = "beta1.1"
STRING = [MAJOR, MINOR, TINY, PRE].compact.join(".")
end

View File

@ -123,6 +123,10 @@ module ActionView
end
alias :find_template :find
def find_file(name, prefixes = [], partial = false, keys = [], options = {})
@view_paths.find_file(*args_for_lookup(name, prefixes, partial, keys, options))
end
def find_all(name, prefixes = [], partial = false, keys = [], options = {})
@view_paths.find_all(*args_for_lookup(name, prefixes, partial, keys, options))
end

View File

@ -46,15 +46,12 @@ module ActionView #:nodoc:
find_all(*args).first || raise(MissingTemplate.new(self, *args))
end
def find_file(path, prefixes = [], *args)
_find_all(path, prefixes, args, true).first || raise(MissingTemplate.new(self, path, prefixes, *args))
end
def find_all(path, prefixes = [], *args)
prefixes = [prefixes] if String === prefixes
prefixes.each do |prefix|
paths.each do |resolver|
templates = resolver.find_all(path, prefix, *args)
return templates unless templates.empty?
end
end
[]
_find_all path, prefixes, args, false
end
def exists?(path, prefixes, *args)
@ -72,6 +69,21 @@ module ActionView #:nodoc:
private
def _find_all(path, prefixes, args, outside_app)
prefixes = [prefixes] if String === prefixes
prefixes.each do |prefix|
paths.each do |resolver|
if outside_app
templates = resolver.find_all_anywhere(path, prefix, *args)
else
templates = resolver.find_all(path, prefix, *args)
end
return templates unless templates.empty?
end
end
[]
end
def typecast(paths)
paths.map do |path|
case path

View File

@ -15,7 +15,7 @@ module ActionView
# that new object is called in turn. This abstracts the setup and rendering
# into a separate classes for partials and templates.
class AbstractRenderer #:nodoc:
delegate :find_template, :template_exists?, :with_fallbacks, :with_layout_format, :formats, :to => :@lookup_context
delegate :find_template, :find_file, :template_exists?, :with_fallbacks, :with_layout_format, :formats, :to => :@lookup_context
def initialize(lookup_context)
@lookup_context = lookup_context

View File

@ -29,7 +29,7 @@ module ActionView
elsif options.key?(:html)
Template::HTML.new(options[:html], formats.first)
elsif options.key?(:file)
with_fallbacks { find_template(options[:file], nil, false, keys, @details) }
with_fallbacks { find_file(options[:file], nil, false, keys, @details) }
elsif options.key?(:inline)
handler = Template.handler_for_extension(options[:type] || "erb")
Template.new(options[:inline], "inline template", handler, :locals => keys)

View File

@ -126,6 +126,12 @@ module ActionView
end
end
def find_all_anywhere(name, prefix, partial=false, details={}, key=nil, locals=[])
cached(key, [name, prefix, partial], details, locals) do
find_templates(name, prefix, partial, details, true)
end
end
def find_all_with_query(query) # :nodoc:
@cache.cache_query(query) { find_template_paths(File.join(@path, query)) }
end
@ -187,15 +193,16 @@ module ActionView
private
def find_templates(name, prefix, partial, details)
def find_templates(name, prefix, partial, details, outside_app_allowed = false)
path = Path.build(name, prefix, partial)
query(path, details, details[:formats])
query(path, details, details[:formats], outside_app_allowed)
end
def query(path, details, formats)
def query(path, details, formats, outside_app_allowed)
query = build_query(path, details)
template_paths = find_template_paths(query)
template_paths = reject_files_external_to_app(template_paths) unless outside_app_allowed
template_paths.map do |template|
handler, format, variant = extract_handler_and_format_and_variant(template, formats)
@ -210,6 +217,10 @@ module ActionView
end
end
def reject_files_external_to_app(files)
files.reject { |filename| !inside_path?(@path, filename) }
end
def find_template_paths(query)
Dir[query].reject do |filename|
File.directory?(filename) ||
@ -218,6 +229,12 @@ module ActionView
end
end
def inside_path?(path, filename)
filename = File.expand_path(filename)
path = File.join(path, '')
filename.start_with?(path)
end
# Helper for building query glob string based on resolver's pattern.
def build_query(path, details)
query = @pattern.dup

View File

@ -19,7 +19,7 @@ module ActionView #:nodoc:
private
def query(path, exts, formats)
def query(path, exts, formats, _)
query = ""
EXTENSIONS.each_key do |ext|
query << '(' << exts[ext].map {|e| e && Regexp.escape(".#{e}") }.join('|') << '|)'
@ -44,7 +44,7 @@ module ActionView #:nodoc:
end
class NullResolver < PathResolver
def query(path, exts, formats)
def query(path, exts, formats, _)
handler, format, variant = extract_handler_and_format_and_variant(path, formats)
[ActionView::Template.new("Template generated by Null Resolver", path.virtual, handler, :virtual_path => path.virtual, :format => format, :variant => variant)]
end

View File

@ -148,6 +148,13 @@ module RenderTestCases
assert_equal "only partial", @view.render("test/partial_only")
end
def test_render_outside_path
assert File.exist?(File.join(File.dirname(__FILE__), '../../test/abstract_unit.rb'))
assert_raises ActionView::MissingTemplate do
@view.render(:template => "../\\../test/abstract_unit.rb")
end
end
def test_render_partial
assert_equal "only partial", @view.render(:partial => "test/partial_only")
end

View File

@ -8,7 +8,7 @@ module ActiveJob
MAJOR = 5
MINOR = 0
TINY = 0
PRE = "beta1"
PRE = "beta1.1"
STRING = [MAJOR, MINOR, TINY, PRE].compact.join(".")
end

View File

@ -8,7 +8,7 @@ module ActiveModel
MAJOR = 5
MINOR = 0
TINY = 0
PRE = "beta1"
PRE = "beta1.1"
STRING = [MAJOR, MINOR, TINY, PRE].compact.join(".")
end

View File

@ -10,7 +10,7 @@ module ActiveModel
included do
extend ActiveModel::Naming
class_attribute :include_root_in_json
class_attribute :include_root_in_json, instance_writer: false
self.include_root_in_json = false
end

View File

@ -47,9 +47,10 @@ module ActiveModel
include HelperMethods
attr_accessor :validation_context
private :validation_context=
define_callbacks :validate, scope: :name
class_attribute :_validators
class_attribute :_validators, instance_writer: false
self._validators = Hash.new { |h,k| h[k] = [] }
end

View File

@ -95,7 +95,7 @@ module ActiveRecord
module Enum
def self.extended(base) # :nodoc:
base.class_attribute(:defined_enums)
base.class_attribute(:defined_enums, instance_writer: false)
base.defined_enums = {}
end

View File

@ -8,7 +8,7 @@ module ActiveRecord
MAJOR = 5
MINOR = 0
TINY = 0
PRE = "beta1"
PRE = "beta1.1"
STRING = [MAJOR, MINOR, TINY, PRE].compact.join(".")
end

View File

@ -542,7 +542,7 @@ module ActiveRecord
# has_destroy_flag? or if a <tt>:reject_if</tt> proc exists for this
# association and evaluates to +true+.
def reject_new_record?(association_name, attributes)
has_destroy_flag?(attributes) || call_reject_if(association_name, attributes)
will_be_destroyed?(association_name, attributes) || call_reject_if(association_name, attributes)
end
# Determines if a record with the particular +attributes+ should be
@ -551,7 +551,8 @@ module ActiveRecord
#
# Returns false if there is a +destroy_flag+ on the attributes.
def call_reject_if(association_name, attributes)
return false if has_destroy_flag?(attributes)
return false if will_be_destroyed?(association_name, attributes)
case callback = self.nested_attributes_options[association_name][:reject_if]
when Symbol
method(callback).arity == 0 ? send(callback) : send(callback, attributes)
@ -560,6 +561,15 @@ module ActiveRecord
end
end
# Only take into account the destroy flag if <tt>:allow_destroy</tt> is true
def will_be_destroyed?(association_name, attributes)
allow_destroy?(association_name) && has_destroy_flag?(attributes)
end
def allow_destroy?(association_name)
self.nested_attributes_options[association_name][:allow_destroy]
end
def raise_nested_attributes_record_not_found!(association_name, record_id)
model = self.class._reflect_on_association(association_name).klass.name
raise RecordNotFound.new("Couldn't find #{model} with ID=#{record_id} for #{self.class.name} with ID=#{id}",

View File

@ -7,8 +7,8 @@ module ActiveRecord
extend ActiveSupport::Concern
included do
class_attribute :_reflections
class_attribute :aggregate_reflections
class_attribute :_reflections, instance_writer: false
class_attribute :aggregate_reflections, instance_writer: false
self._reflections = {}
self.aggregate_reflections = {}
end

View File

@ -6,7 +6,7 @@ module ActiveRecord
included do
# Stores the default scope for the class.
class_attribute :default_scopes, instance_writer: false, instance_predicate: false
class_attribute :default_scope_override, instance_predicate: false
class_attribute :default_scope_override, instance_writer: false, instance_predicate: false
self.default_scopes = []
self.default_scope_override = nil

View File

@ -146,6 +146,19 @@ class TestNestedAttributesInGeneral < ActiveRecord::TestCase
assert man.reload.interests.empty?
end
def test_reject_if_is_not_short_circuited_if_allow_destroy_is_false
Pirate.accepts_nested_attributes_for :ship, reject_if: ->(a) { a[:name] == "The Golden Hind" }, allow_destroy: false
pirate = Pirate.create!(catchphrase: "Stop wastin' me time", ship_attributes: { name: "White Pearl", _destroy: "1" })
assert_equal "White Pearl", pirate.reload.ship.name
pirate.update!(ship_attributes: { id: pirate.ship.id, name: "The Golden Hind", _destroy: "1" })
assert_equal "White Pearl", pirate.reload.ship.name
pirate.update!(ship_attributes: { id: pirate.ship.id, name: "Black Pearl", _destroy: "1" })
assert_equal "Black Pearl", pirate.reload.ship.name
end
def test_has_many_association_updating_a_single_record
Man.accepts_nested_attributes_for(:interests)
man = Man.create(name: 'John')

View File

@ -71,7 +71,7 @@ module ActiveSupport
# halt the entire callback chain and display a deprecation message.
# If false, callback chains will only be halted by calling +throw :abort+.
# Defaults to +true+.
mattr_accessor(:halt_and_display_warning_on_return_false) { true }
mattr_accessor(:halt_and_display_warning_on_return_false, instance_writer: false) { true }
# Runs the callbacks for the given event.
#
@ -742,7 +742,7 @@ module ActiveSupport
options = names.extract_options!
names.each do |name|
class_attribute "_#{name}_callbacks"
class_attribute "_#{name}_callbacks", instance_writer: false
set_callbacks name, CallbackChain.new(name, options)
module_eval <<-RUBY, __FILE__, __LINE__ + 1

View File

@ -8,7 +8,7 @@ module ActiveSupport
MAJOR = 5
MINOR = 0
TINY = 0
PRE = "beta1"
PRE = "beta1.1"
STRING = [MAJOR, MINOR, TINY, PRE].compact.join(".")
end

View File

@ -1,3 +1,5 @@
require 'digest'
module ActiveSupport
module SecurityUtils
# Constant time string comparison.
@ -16,5 +18,10 @@ module ActiveSupport
res == 0
end
module_function :secure_compare
def variable_size_secure_compare(a, b) # :nodoc:
secure_compare(::Digest::SHA256.hexdigest(a), ::Digest::SHA256.hexdigest(b))
end
module_function :variable_size_secure_compare
end
end

View File

@ -8,7 +8,7 @@ module Rails
MAJOR = 5
MINOR = 0
TINY = 0
PRE = "beta1"
PRE = "beta1.1"
STRING = [MAJOR, MINOR, TINY, PRE].compact.join(".")
end

View File

@ -27,7 +27,7 @@ directory "pkg"
file = Dir[glob].first
ruby = File.read(file)
major, minor, tiny, pre = version.split('.')
major, minor, tiny, pre = version.split('.', 4)
pre = pre ? pre.inspect : "nil"
ruby.gsub!(/^(\s*)MAJOR(\s*)= .*?$/, "\\1MAJOR = #{major}")

View File

@ -8,7 +8,7 @@ module Rails
MAJOR = 5
MINOR = 0
TINY = 0
PRE = "beta1"
PRE = "beta1.1"
STRING = [MAJOR, MINOR, TINY, PRE].compact.join(".")
end