Skip to content

Commit

Permalink
Merge branch 'master' of git@github.com:rails/rails
Browse files Browse the repository at this point in the history
  • Loading branch information
jeremy committed Dec 3, 2008
2 parents 0b4858c + 1e1056f commit 99f2cb4
Show file tree
Hide file tree
Showing 64 changed files with 1,027 additions and 768 deletions.
6 changes: 0 additions & 6 deletions actionmailer/lib/action_mailer/base.rb
Expand Up @@ -420,12 +420,6 @@ def deliver(mail)
new.deliver!(mail)
end

def register_template_extension(extension)
ActiveSupport::Deprecation.warn(
"ActionMailer::Base.register_template_extension has been deprecated." +
"Use ActionView::Base.register_template_extension instead", caller)
end

def template_root
self.view_paths && self.view_paths.first
end
Expand Down
54 changes: 54 additions & 0 deletions actionmailer/test/asset_host_test.rb
@@ -0,0 +1,54 @@
require 'abstract_unit'

class AssetHostMailer < ActionMailer::Base
def email_with_asset(recipient)
recipients recipient
subject "testing email containing asset path while asset_host is set"
from "tester@example.com"
end
end

class AssetHostTest < Test::Unit::TestCase
def setup
set_delivery_method :test
ActionMailer::Base.perform_deliveries = true
ActionMailer::Base.deliveries = []

@recipient = 'test@localhost'
end

def teardown
restore_delivery_method
end

def test_asset_host_as_string
ActionController::Base.asset_host = "http://www.example.com"
mail = AssetHostMailer.deliver_email_with_asset(@recipient)
assert_equal "<img alt=\"Somelogo\" src=\"http://www.example.com/images/somelogo.png\" />", mail.body.strip
end

def test_asset_host_as_one_arguement_proc
ActionController::Base.asset_host = Proc.new { |source|
if source.starts_with?('/images')
"http://images.example.com"
else
"http://assets.example.com"
end
}
mail = AssetHostMailer.deliver_email_with_asset(@recipient)
assert_equal "<img alt=\"Somelogo\" src=\"http://images.example.com/images/somelogo.png\" />", mail.body.strip
end

def test_asset_host_as_two_arguement_proc
ActionController::Base.asset_host = Proc.new {|source,request|
if request && request.ssl?
"https://www.example.com"
else
"http://www.example.com"
end
}
mail = nil
assert_nothing_raised { mail = AssetHostMailer.deliver_email_with_asset(@recipient) }
assert_equal "<img alt=\"Somelogo\" src=\"http://www.example.com/images/somelogo.png\" />", mail.body.strip
end
end
@@ -0,0 +1 @@
<%= image_tag "somelogo.png" %>
2 changes: 2 additions & 0 deletions actionpack/CHANGELOG
@@ -1,5 +1,7 @@
*2.3.0 [Edge]*

* Allow users to opt out of the spoofing checks in Request#remote_ip. Useful for sites whose traffic regularly triggers false positives. [Darren Boyd]

* Deprecated formatted_polymorphic_url. [Jeremy Kemper]

* Added the option to declare an asset_host as an object that responds to call (see http://github.com/dhh/asset-hosting-with-minimum-ssl for an example) [DHH]
Expand Down
1 change: 1 addition & 0 deletions actionpack/lib/action_controller.rb
Expand Up @@ -57,6 +57,7 @@ def self.load_all!
autoload :Integration, 'action_controller/integration'
autoload :IntegrationTest, 'action_controller/integration'
autoload :Layout, 'action_controller/layout'
autoload :MiddlewareStack, 'action_controller/middleware_stack'
autoload :MimeResponds, 'action_controller/mime_responds'
autoload :PolymorphicRoutes, 'action_controller/polymorphic_routes'
autoload :RackRequest, 'action_controller/rack_process'
Expand Down
4 changes: 4 additions & 0 deletions actionpack/lib/action_controller/base.rb
Expand Up @@ -327,6 +327,10 @@ class Base
# sets it to <tt>:authenticity_token</tt> by default.
cattr_accessor :request_forgery_protection_token

# Controls the IP Spoofing check when determining the remote IP.
@@ip_spoofing_check = true
cattr_accessor :ip_spoofing_check

# Indicates whether or not optimise the generated named
# route helper methods
cattr_accessor :optimise_named_routes
Expand Down
8 changes: 8 additions & 0 deletions actionpack/lib/action_controller/dispatcher.rb
Expand Up @@ -85,6 +85,9 @@ def failsafe_logger
end
end

cattr_accessor :middleware
self.middleware = MiddlewareStack.new

cattr_accessor :error_file_path
self.error_file_path = Rails.public_path if defined?(Rails.public_path)

Expand All @@ -93,6 +96,7 @@ def failsafe_logger

def initialize(output = $stdout, request = nil, response = nil)
@output, @request, @response = output, request, response
@app = @@middleware.build(lambda { |env| self._call(env) })
end

def dispatch_unlocked
Expand Down Expand Up @@ -127,6 +131,10 @@ def dispatch_cgi(cgi, session_options)
end

def call(env)
@app.call(env)
end

def _call(env)
@request = RackRequest.new(env)
@response = RackResponse.new(@request)
dispatch
Expand Down
42 changes: 42 additions & 0 deletions actionpack/lib/action_controller/middleware_stack.rb
@@ -0,0 +1,42 @@
module ActionController
class MiddlewareStack < Array
class Middleware
attr_reader :klass, :args, :block

def initialize(klass, *args, &block)
@klass = klass.is_a?(Class) ? klass : klass.to_s.constantize
@args = args
@block = block
end

def ==(middleware)
case middleware
when Middleware
klass == middleware.klass
when Class
klass == middleware
else
klass == middleware.to_s.constantize
end
end

def inspect
str = @klass.to_s
@args.each { |arg| str += ", #{arg.inspect}" }
str
end

def build(app)
klass.new(app, *args, &block)
end
end

def use(*args, &block)
push(Middleware.new(*args, &block))
end

def build(app)
reverse.inject(app) { |a, e| e.build(a) }
end
end
end
2 changes: 1 addition & 1 deletion actionpack/lib/action_controller/request.rb
Expand Up @@ -218,7 +218,7 @@ def remote_ip
remote_ips = @env['HTTP_X_FORWARDED_FOR'] && @env['HTTP_X_FORWARDED_FOR'].split(',')

if @env.include? 'HTTP_CLIENT_IP'
if remote_ips && !remote_ips.include?(@env['HTTP_CLIENT_IP'])
if ActionController::Base.ip_spoofing_check && remote_ips && !remote_ips.include?(@env['HTTP_CLIENT_IP'])
# We don't know which came from the proxy, and which from the user
raise ActionControllerError.new(<<EOM)
IP spoofing attack?!
Expand Down
2 changes: 1 addition & 1 deletion actionpack/lib/action_view/helpers/asset_tag_helper.rb
Expand Up @@ -574,7 +574,7 @@ def mtime

private
def request
@controller.request
request? && @controller.request
end

def request?
Expand Down
8 changes: 4 additions & 4 deletions actionpack/lib/action_view/helpers/text_helper.rb
Expand Up @@ -370,8 +370,8 @@ def auto_link(text, *args, &block)#link = :all, href_options = {}, &block)
options.reverse_merge!(:link => :all, :html => {})

case options[:link].to_sym
when :all then auto_link_email_addresses(auto_link_urls(text, options[:html], &block), &block)
when :email_addresses then auto_link_email_addresses(text, &block)
when :all then auto_link_email_addresses(auto_link_urls(text, options[:html], &block), options[:html], &block)
when :email_addresses then auto_link_email_addresses(text, options[:html], &block)
when :urls then auto_link_urls(text, options[:html], &block)
end
end
Expand Down Expand Up @@ -559,7 +559,7 @@ def auto_link_urls(text, html_options = {})

# Turns all email addresses into clickable links. If a block is given,
# each email is yielded and the result is used as the link text.
def auto_link_email_addresses(text)
def auto_link_email_addresses(text, html_options = {})
body = text.dup
text.gsub(/([\w\.!#\$%\-+.]+@[A-Za-z0-9\-]+(\.[A-Za-z0-9\-]+)+)/) do
text = $1
Expand All @@ -568,7 +568,7 @@ def auto_link_email_addresses(text)
text
else
display_text = (block_given?) ? yield(text) : text
%{<a href="mailto:#{text}">#{display_text}</a>}
mail_to text, display_text, html_options
end
end
end
Expand Down
9 changes: 9 additions & 0 deletions actionpack/test/controller/request_test.rb
Expand Up @@ -66,6 +66,15 @@ def test_remote_ip
assert_match /HTTP_X_FORWARDED_FOR="9.9.9.9, 3.4.5.6, 10.0.0.1, 172.31.4.4"/, e.message
assert_match /HTTP_CLIENT_IP="8.8.8.8"/, e.message

# turn IP Spoofing detection off.
# This is useful for sites that are aimed at non-IP clients. The typical
# example is WAP. Since the cellular network is not IP based, it's a
# leap of faith to assume that their proxies are ever going to set the
# HTTP_CLIENT_IP/HTTP_X_FORWARDED_FOR headers properly.
ActionController::Base.ip_spoofing_check = false
assert_equal('8.8.8.8', @request.remote_ip(true))
ActionController::Base.ip_spoofing_check = true

@request.env['HTTP_X_FORWARDED_FOR'] = '8.8.8.8, 9.9.9.9'
assert_equal '8.8.8.8', @request.remote_ip(true)

Expand Down
7 changes: 6 additions & 1 deletion actionpack/test/template/text_helper_test.rb
Expand Up @@ -262,6 +262,11 @@ def test_auto_linking
email2_result = %{<a href="mailto:#{email2_raw}">#{email2_raw}</a>}
assert_equal email2_result, auto_link(email2_raw)

email3_raw = '+david@loudthinking.com'
email3_result = %{<a href="&#109;&#97;&#105;&#108;&#116;&#111;&#58;+%64%61%76%69%64@%6c%6f%75%64%74%68%69%6e%6b%69%6e%67.%63%6f%6d">#{email3_raw}</a>}
assert_equal email3_result, auto_link(email3_raw, :all, :encode => :hex)
assert_equal email3_result, auto_link(email3_raw, :email_addresses, :encode => :hex)

link2_raw = 'www.rubyonrails.com'
link2_result = generate_result(link2_raw, "http://#{link2_raw}")
assert_equal %(Go to #{link2_result}), auto_link("Go to #{link2_raw}", :urls)
Expand Down Expand Up @@ -362,7 +367,7 @@ def test_auto_link_with_block
end

def test_auto_link_with_options_hash
assert_dom_equal 'Welcome to my new blog at <a href="http://www.myblog.com/" class="menu" target="_blank">http://www.myblog.com/</a>. Please e-mail me at <a href="mailto:me@email.com">me@email.com</a>.',
assert_dom_equal 'Welcome to my new blog at <a href="http://www.myblog.com/" class="menu" target="_blank">http://www.myblog.com/</a>. Please e-mail me at <a href="mailto:me@email.com" class="menu" target="_blank">me@email.com</a>.',
auto_link("Welcome to my new blog at http://www.myblog.com/. Please e-mail me at me@email.com.",
:link => :all, :html => { :class => "menu", :target => "_blank" })
end
Expand Down
2 changes: 2 additions & 0 deletions activerecord/CHANGELOG
@@ -1,5 +1,7 @@
*2.3.0/3.0*

* Add :having as a key to find and the relevant associations. [miloops]

* Added default_scope to Base #1381 [Paweł Kondzior]. Example:

class Person < ActiveRecord::Base
Expand Down
2 changes: 1 addition & 1 deletion activerecord/lib/active_record/association_preload.rb
Expand Up @@ -185,7 +185,7 @@ def preload_has_and_belongs_to_many_association(records, reflection, preload_opt

associated_records = reflection.klass.find(:all, :conditions => [conditions, ids],
:include => options[:include],
:joins => "INNER JOIN #{connection.quote_table_name options[:join_table]} as t0 ON #{reflection.klass.quoted_table_name}.#{reflection.klass.primary_key} = t0.#{reflection.association_foreign_key}",
:joins => "INNER JOIN #{connection.quote_table_name options[:join_table]} t0 ON #{reflection.klass.quoted_table_name}.#{reflection.klass.primary_key} = t0.#{reflection.association_foreign_key}",
:select => "#{options[:select] || table_name+'.*'}, t0.#{reflection.primary_key_name} as the_parent_record_id",
:order => options[:order])

Expand Down
12 changes: 8 additions & 4 deletions activerecord/lib/active_record/associations.rb
Expand Up @@ -724,6 +724,8 @@ module ClassMethods
# Specify second-order associations that should be eager loaded when the collection is loaded.
# [:group]
# An attribute name by which the result should be grouped. Uses the <tt>GROUP BY</tt> SQL-clause.
# [:having]
# Combined with +:group+ this can be used to filter the records that a <tt>GROUP BY</tt> returns. Uses the <tt>HAVING</tt> SQL-clause.
# [:limit]
# An integer determining the limit on the number of rows that should be returned.
# [:offset]
Expand Down Expand Up @@ -1181,6 +1183,8 @@ def belongs_to(association_id, options = {})
# Specify second-order associations that should be eager loaded when the collection is loaded.
# [:group]
# An attribute name by which the result should be grouped. Uses the <tt>GROUP BY</tt> SQL-clause.
# [:having]
# Combined with +:group+ this can be used to filter the records that a <tt>GROUP BY</tt> returns. Uses the <tt>HAVING</tt> SQL-clause.
# [:limit]
# An integer determining the limit on the number of rows that should be returned.
# [:offset]
Expand Down Expand Up @@ -1553,7 +1557,7 @@ def nullify_has_many_dependencies(record, reflection_name, association_class, pr
@@valid_keys_for_has_many_association = [
:class_name, :table_name, :foreign_key, :primary_key,
:dependent,
:select, :conditions, :include, :order, :group, :limit, :offset,
:select, :conditions, :include, :order, :group, :having, :limit, :offset,
:as, :through, :source, :source_type,
:uniq,
:finder_sql, :counter_sql,
Expand Down Expand Up @@ -1609,7 +1613,7 @@ def create_belongs_to_reflection(association_id, options)
mattr_accessor :valid_keys_for_has_and_belongs_to_many_association
@@valid_keys_for_has_and_belongs_to_many_association = [
:class_name, :table_name, :join_table, :foreign_key, :association_foreign_key,
:select, :conditions, :include, :order, :group, :limit, :offset,
:select, :conditions, :include, :order, :group, :having, :limit, :offset,
:uniq,
:finder_sql, :counter_sql, :delete_sql, :insert_sql,
:before_add, :after_add, :before_remove, :after_remove,
Expand Down Expand Up @@ -1658,7 +1662,7 @@ def construct_finder_sql_with_included_associations(options, join_dependency)
add_conditions!(sql, options[:conditions], scope)
add_limited_ids_condition!(sql, options, join_dependency) if !using_limitable_reflections?(join_dependency.reflections) && ((scope && scope[:limit]) || options[:limit])

add_group!(sql, options[:group], scope)
add_group!(sql, options[:group], options[:having], scope)
add_order!(sql, options[:order], scope)
add_limit!(sql, options, scope) if using_limitable_reflections?(join_dependency.reflections)
add_lock!(sql, options, scope)
Expand Down Expand Up @@ -1714,7 +1718,7 @@ def construct_finder_sql_for_association_limiting(options, join_dependency)
end

add_conditions!(sql, options[:conditions], scope)
add_group!(sql, options[:group], scope)
add_group!(sql, options[:group], options[:having], scope)

if order && is_distinct
connection.add_order_by_for_association_limiting!(sql, :order => order)
Expand Down
Expand Up @@ -188,6 +188,7 @@ def set_belongs_to_association_for(record)
def merge_options_from_reflection!(options)
options.reverse_merge!(
:group => @reflection.options[:group],
:having => @reflection.options[:having],
:limit => @reflection.options[:limit],
:offset => @reflection.options[:offset],
:joins => @reflection.options[:joins],
Expand Down
9 changes: 6 additions & 3 deletions activerecord/lib/active_record/base.rb
Expand Up @@ -521,6 +521,7 @@ class << self # Class methods
# * <tt>:conditions</tt> - An SQL fragment like "administrator = 1", <tt>[ "user_name = ?", username ]</tt>, or <tt>["user_name = :user_name", { :user_name => user_name }]</tt>. See conditions in the intro.
# * <tt>:order</tt> - An SQL fragment like "created_at DESC, name".
# * <tt>:group</tt> - An attribute name by which the result should be grouped. Uses the <tt>GROUP BY</tt> SQL-clause.
# * <tt>:having</tt> - Combined with +:group+ this can be used to filter the records that a <tt>GROUP BY</tt> returns. Uses the <tt>HAVING</tt> SQL-clause.
# * <tt>:limit</tt> - An integer determining the limit on the number of rows that should be returned.
# * <tt>:offset</tt> - An integer determining the offset from where the rows should be fetched. So at 5, it would skip rows 0 through 4.
# * <tt>:joins</tt> - Either an SQL fragment for additional joins like "LEFT JOIN comments ON comments.post_id = id" (rarely needed)
Expand Down Expand Up @@ -1632,7 +1633,7 @@ def construct_finder_sql(options)
add_joins!(sql, options[:joins], scope)
add_conditions!(sql, options[:conditions], scope)

add_group!(sql, options[:group], scope)
add_group!(sql, options[:group], options[:having], scope)
add_order!(sql, options[:order], scope)
add_limit!(sql, options, scope)
add_lock!(sql, options, scope)
Expand Down Expand Up @@ -1688,13 +1689,15 @@ def add_order!(sql, order, scope = :auto)
end
end

def add_group!(sql, group, scope = :auto)
def add_group!(sql, group, having, scope = :auto)
if group
sql << " GROUP BY #{group}"
sql << " HAVING #{having}" if having
else
scope = scope(:find) if :auto == scope
if scope && (scoped_group = scope[:group])
sql << " GROUP BY #{scoped_group}"
sql << " HAVING #{scoped_having}" if (scoped_having = scope[:having])
end
end
end
Expand Down Expand Up @@ -2259,7 +2262,7 @@ def raise_if_bind_arity_mismatch(statement, expected, provided) #:nodoc:
end

VALID_FIND_OPTIONS = [ :conditions, :include, :joins, :limit, :offset,
:order, :select, :readonly, :group, :from, :lock ]
:order, :select, :readonly, :group, :having, :from, :lock ]

def validate_find_options(options) #:nodoc:
options.assert_valid_keys(VALID_FIND_OPTIONS)
Expand Down
Expand Up @@ -658,6 +658,11 @@ def test_find_scoped_grouped
assert_equal 1, categories(:technology).posts_gruoped_by_title.size
end

def test_find_scoped_grouped_having
assert_equal 2, projects(:active_record).well_payed_salary_groups.size
assert projects(:active_record).well_payed_salary_groups.all? { |g| g.salary > 10000 }
end

def test_get_ids
assert_equal projects(:active_record, :action_controller).map(&:id).sort, developers(:david).project_ids.sort
assert_equal [projects(:active_record).id], developers(:jamis).project_ids
Expand Down

0 comments on commit 99f2cb4

Please sign in to comment.