Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Test fix for 74e59ea #9942

Closed
wants to merge 1,522 commits into
from

Conversation

Projects
None yet
Contributor

fredwu commented Mar 27, 2013

This fixes the backport PR #9899 causing test failures on 1.8.7.

claudiob and others added some commits Nov 13, 2012

Add release date of Rails 3.2.9 to documentation
Conflicts:
	actionpack/CHANGELOG.md
	activerecord/CHANGELOG.md
	activesupport/CHANGELOG.md
Merge branch '3-2-sec' into 3-2-secmerge
* 3-2-sec:
  CVE-2012-5664 options hashes should only be extracted if there are extra parameters
  updating changelog
  updating the changelogs
  updating the changelog for the CVE
  Add release date of Rails 3.2.9 to documentation

Conflicts:
	actionmailer/CHANGELOG.md
	actionpack/CHANGELOG.md
	activemodel/CHANGELOG.md
	activerecord/CHANGELOG.md
	activeresource/CHANGELOG.md
	activesupport/CHANGELOG.md
	railties/CHANGELOG.md
backport #8616, quote column names in generated fixture files
Conflicts:

	railties/CHANGELOG.md
	railties/lib/rails/generators/test_unit/model/model_generator.rb
	railties/lib/rails/generators/test_unit/model/templates/fixtures.yml
	railties/test/generators/model_generator_test.rb
Merge pull request #8618 from senny/backport_8612
backport #8616, quote column names in generated fixture files
Revert "Fix `validates_presence_of` with `:allow_nil` or `:allow_blan…
…k` options."

This reverts commit 93366c7.

REASON: This is backward incompatible. Also this behavior is documented
on the guides.
backport #8662, charset should not be appended for `head` responses
      1) Failure:
    test_head_created_with_image_png_content_type(RenderTest) [test/controller/render_test.rb:1238]:
    Expected: "image/png"
      Actual: "image/png; charset=utf-8"
Merge pull request #8665 from senny/8661_should_not_append_charset_if…
…_already_present

backport #8662, charset should not be appended for `head` responses
Revert "Merge pull request #8665 from senny/8661_should_not_append_ch…
…arset_if_already_present"

This reverts commit e48dc19, reversing
changes made to d38c8ca.
removes the Ajax on Rails early draft
There was a few attempts at writing this guide, but we
never passed from the work in progress stage. In spite
of not being included in the table of contents, this
draft was still indexed by bots and showed up in searches.

Steve Klabnik has written "Working with JavaScript in Rails"
which is going to be released with Rails 4. So better get
rid of this altogether.
Merge pull request #8640 from jasl/3-2-stable
fix block.arity will raise nil error
Merge branch '3-2-stable' into 3-2-secmerge
* 3-2-stable:
  fix block.arity raise nil error when not given a block to "content_tag_for"
  removes the Ajax on Rails early draft
  Revert "Merge pull request #8665 from senny/8661_should_not_append_charset_if_already_present"
  backport #8662, charset should not be appended for `head` responses
  Revert "Fix `validates_presence_of` with `:allow_nil` or `:allow_blank` options."
  Fix `validates_presence_of` with `:allow_nil` or `:allow_blank` options.
  backport #8616, quote column names in generated fixture files
Add release date of 3.2.10
Fix format and wrong changelog entry
Merge pull request #8703 from senny/backport_8700
Backport #8701, do not append a second slash with `trailing_slash: true`

Closes #8700
Merge tag 'v3.2.10' into 3-2-stable
Latest released tag was not fully merged into the stable branch (missed version bumping)
Merge pull request #8719 from pcasaretto/fix-actionview-doc-typo
Fix typo on form_tag_helper.rb [ci skip]
Merge pull request #8716 from miguelff/3-2-stable
Merged latest released tag (v3.2.10) into the stable branch (3-2-stable)
Merge pull request #8733 from amatsuda/rails32_readme_dir_tree
update directory tree in the generated README in Rails 3.2

[ci skip]
Fix undefined method `to_i' introduced since 3.2.8
This commit fixes a bug introduced in 96a13fc which breaks behaviour of
integer fields in 3.2.8.

In 3.2.8, setting the value of an integer field to a non-integer (eg.
Array, Hash, etc.) would default to 1 (true) :

    # 3.2.8
    p = Post.new
    p.category_id = [ 1, 2 ]
    p.category_id # => 1
    p.category_id = { 3 => 4 }
    p.category_id # => 1

In 3.2.9 and above, this will raise a NoMethodError :

    # 3.2.9
    p = Post.new
    p.category_id = [ 1, 2 ]

    NoMethodError: undefined method `to_i' for [1, 2]:Array

Whilst at first blush this appear to be sensible, it combines in bad
ways with scoping.

For example, it is common to use scopes to control access to data :

    @collection = Posts.where(:category_id => [ 1, 2 ])
    @new_post = @collection.new

In 3.2.8, this would work as expected, creating a new Post object
(albeit with @new_post.category_id = 1). However, in 3.2.9 this will
cause the NoMethodError to be raised as above.

It is difficult to avoid triggering this error without descoping before
calling .new, breaking any apps running on 3.2.8 that rely on this
behaviour.

This patch deviates from 3.2.8 in that it does not retain the somewhat
spurious behaviour of setting the attribute to 1. Instead, it explicitly
sets these invalid values to nil :

    p = Post.new
    p.category_id = [ 1, 2 ]
    p.category_id # => nil

This also fixes the situation where a scope using an array will
"pollute" any newly instantiated records.

    @new_post = @collection.new
    @new_post.category_id # => nil

Finally, 3.2.8 exhibited a behaviour where setting an object to an
integer field caused it to be coerced to "1". This has not been
retained, as it is spurious and surprising in the same way that setting
Arrays and Heshes was :

    c = Category.find(6)
    p = Post.new

    # 3.2.8
    p.category_id = c
    p.category_id # => 1

    # This patch
    p.category_id = c
    p.category_id # => nil

This commit includes explicit test cases that expose the original issue
with calling new on a scope that uses an Array. As this is a common
situation, an explicit test case is the best way to prevent regressions
in the future.

It also updates and separates existing tests to be explicit about the
situation that is being tested (eg. AR objects vs. other objects vs.
non-integers)
Merge pull request #8718 from jstirk/column_writer_to_i_errors
Fix undefined method `to_i' introduced since 3.2.8
backport #4f0f1b5 into 3-2-stable. because 1.9.3-p362 warned unused v…
…ariables.

Conflicts:
	actionpack/test/controller/render_test.rb
Merge pull request #8763 from hsbt/backport-to-hide-warning
Backport 4f0f1b5 into 3-2-stable.

When running the test with warnings enabled, it fails without this change.
Remove suggestion that Procs can be used as session secrets.
(cherry picked from commit 6500d79)

[ci skip]

Signed-off-by: Andrew White <andyw@pixeltrix.co.uk>
Do not call fields_for from form_for, to avoid instantiating two buil…
…ders

Conflicts:
	actionpack/lib/action_view/helpers/form_helper.rb
	actionpack/test/template/form_helper_test.rb
Fix error when assigning NaN to an integer column
Also covers any non-castable case by returning nil, which
is in-line with the intention of the former implementation,
but covers the odd cases which respond to to_i but raise
an error when it's called, such as NaN, Infinity and -Infinity.

Fixes #8757

Backport of #8781

Conflicts:
	activerecord/CHANGELOG.md
	activerecord/test/cases/column_test.rb
Fix named scope + class method example
Closes #8804 [ci skip]

Conflicts:
	activerecord/lib/active_record/scoping/named.rb
Fix javascript_include_tag when no js runtime is available
In a production environment where the assets have been precompiled, we
don't want an assets compile step to happen on the application server at
all. To ensure this, a js runtime may not be available on the app
servers. In this environment, pages using javascript_include_tag for
assets with non-standard or chained extensions were throwing 500 errors.
For instance, `javascript_include_tag('jquery.min')` would blow up.

Sprockets was attempting to build the assets being included during the
rewrite_extension step (responsible for appending a '.js' extension to
assets being included by the basename rather than a fully qualified
name). This was happening as a step to resolve #6310, which required
checking for the presence of an asset with a non-standard extension
before appending the extension.

We can check for the presence of an asset without invoking the asset
build step by using Sprockets' resolve method, which will search for the
base file without building it (and is the method that find_asset uses
internally to get the path to the asset before attempting to build it).

When rewriting the extension on an asset, these are the steps:
- If the source does not have an extension, assume that the default
  extension is desired and append it.
- If there is an extension and it doesn't match the default extension,
  check to see if a file with the precise name specified exists amongst
  the assets; if it is present, do not append the default extension.
  (This is the step that resolves #6310).
Refactor write attribute logic to convert number column value
This is an improvement for issue #8673:
    "Comparing a BigDecimal to true/false on write_attribute is slow"

It seems to be an issue with Ruby itself, related to the "coerce" method
being called in TrueClass/FalseClass due to the == condition, triggering
method_missing, then raising a NameError that's later catched.

This issue was also opened in Ruby tracker:
    https://bugs.ruby-lang.org/issues/7645.

This refactoring avoid the coerce call by using a case statement, which
gives us better readability as well. A simple benchmark:

----------

require 'benchmark/ips'
require 'bigdecimal'

Benchmark.ips do |x|
  x.report("== true")   { BigDecimal('3') == true }
  x.report("TrueClass") { TrueClass === BigDecimal('3') }
  x.report("== 0")      { BigDecimal('3') == 0 }
  x.report("Numeric")   { Numeric === BigDecimal('3') }
end

Calculating -------------------------------------
             == true      6427 i/100ms
           TrueClass     47297 i/100ms
                == 0     35923 i/100ms
             Numeric     55530 i/100ms
-------------------------------------------------
             == true    75878.5 (±21.6%) i/s -     359912 in   5.004392s
           TrueClass  1249547.0 (±13.1%) i/s -    6148610 in   5.035964s
                == 0   666856.3 (±13.3%) i/s -    3268993 in   5.013789s
             Numeric  1269300.9 (±11.3%) i/s -    6274890 in   5.028458s

----------

Master has a very different implementation, and there are apparently no
similar conversions at this point, it's mainly delegated to the column
type cast, but I'll check if something needs to be changed there as well.

Closes #8673.
Avoid Rack security warning no secret provided
This avoids "SECURITY WARNING: No secret option provided to Rack::Session::Cookie."
Wrap time ranges with timezones, closes #8807
(cherry picked from commit e2e5136)
(cherry picked from commit dcdde7d)

Backport of #6183, original issue was #6179

Conflicts:
	activesupport/lib/active_support/core_ext/time/calculations.rb
	activesupport/test/core_ext/time_ext_test

Signed-off-by: Andrew White <andyw@pixeltrix.co.uk>

This rational object is constructed in every call, and in several methods. Would it make sense to give it a meaningful name and extract it to a constant?

Owner

pixeltrix replied Jan 8, 2013

Probably - but we'd need to fix master first 😄

What name do you think? end_of_second is the best I can think of

Owner

fxn replied Jan 8, 2013

end_of_second sounds good, a little ambiguous but simple enough. Maybe with a brief comment that mentions that time is discrete in microseconds or something like that. I don't know how portable is that indeed, should investigate.

carlosantoniodasilva added some commits Jan 8, 2013

Merge pull request #8812 from rubys/master
Eliminate Rack::File headers deprecation warning
Bump rack dependency to 1.4.3
It includes security bug fixes and changes the initialization of
Rack::File to accept a hash, otherwise generating warnings.

See 295806e for the warnings fix.

Conflicts:
	actionpack/actionpack.gemspec
Owner

fxn commented on f64be7d Jan 8, 2013

❤️

spastorino and others added some commits Jan 8, 2013

Avoid Rack security warning no secret provided
This avoids "SECURITY WARNING: No secret option provided to Rack::Session::Cookie."
* Strip nils from collections on JSON and XML posts. [CVE-2013-0155] …
…* dealing with empty hashes. Thanks Damien Mathieu
Merge branch '3-2-sec' into 3-2-secmerge
* 3-2-sec:
  bumping version
  CVE-2013-0156: Safe XML params parsing. Doesn't allow symbols or yaml.
  * Strip nils from collections on JSON and XML posts. [CVE-2013-0155] * dealing with empty hashes. Thanks Damien Mathieu
  Avoid Rack security warning no secret provided

Conflicts:
	actionpack/CHANGELOG.md
	activerecord/CHANGELOG.md
	activesupport/CHANGELOG.md
Ignore binds payload with nil column in AR log subscriber
Some tests were raising the following error:

    Could not log "sql.active_record" event. NoMethodError: undefined method
    `type' for nil:NilClass`

Due to the way binds were being logged, the column info was considered
always present, but that is not true for some of the tests listed in the
issue.

Closes #8806.

Conflicts:

	activerecord/lib/active_record/log_subscriber.rb
	activerecord/test/cases/log_subscriber_test.rb

Conflict resolution:
- Revert ruby 1.9 style hash to support ruby 1.8
- Do not include 8f59ffc into 3-2-stable
Remove test for XML YAML parsing
The support for YAML parsing in XML has been removed from Active Support
since it introduced an security risk. See 43109ec for more detail.

Perhaps prefer a whiltelist instead of a blacklist? ALLOWED_XML_TYPES vs DISALLOWED_XML_TYPES?

Owner

jeremy replied Jan 9, 2013

Yeah! Started with a whitelist, but we allow custom types. A blacklist of types is clearer than a whitelist of parseable types.

Merge pull request #8838 from yahonda/8806_3-2-stable
Ignore binds payload with nil column in AR log subscriber

larrylv and others added some commits Mar 7, 2013

@target might be nil when Identity Map is enabled.
* With Identity Map enabled, NameError might be raised and @target is
  nil. So we should always ensure `@target ||= find_target`.

* Only force reload target when it is stale.
Merge pull request #9600 from larrylv/fix-load-target-with-identity-m…
…ap-on


@target might be nil when Identity Map is enabled.
Using latest AR-JDBC for JRuby
1.2.5 and 1.2.6 are bad ones
Removing FIXME
As we did in master.
Merge pull request #9616 from exviva/multiple_select_name_double_squa…
…re_brackets

Fix incorrectly appended square brackets to a multiple select box

Before:

    select(:category, [], {}, {:multiple => true, :name => "post[category][]"})
    # => <select name="post[category][][]" ...>

After:

    select(:category, [], {}, {:multiple => true, :name => "post[category][]"})
    # => <select name="post[category][]" ...>

Conflicts:
	actionpack/CHANGELOG.md
	actionpack/lib/action_view/helpers/tags/base.rb
	actionpack/test/template/form_options_helper_test.rb
Merge pull request #9616 from exviva/multiple_select_name_double_squa…
…re_brackets

Fix incorrectly appended square brackets to a multiple select box

Before:

    select(:category, [], {}, {:multiple => true, :name => "post[category][]"})
    # => <select name="post[category][][]" ...>

After:

    select(:category, [], {}, {:multiple => true, :name => "post[category][]"})
    # => <select name="post[category][]" ...>

Conflicts:
	actionpack/CHANGELOG.md
	actionpack/lib/action_view/helpers/tags/base.rb
	actionpack/test/template/form_options_helper_test.rb
Fix `ActiveSupport::TimeZone.parse` when time is a local DST jump
The previous implementation `ActiveSupport::TimeZone.parse` used `Time.parse`
which applies the system time DST rules to the parsed time. Instead we now
use `Time.utc` and manually apply the offset.

Backport tests from:
005d910
c89b6c4
03becb1

Fixes #9678.
Fix ar:mysql2 build on Ruby 1.8.7
The `skip` method is a no-op on Ruby 1.8.7 so we still need to
force skip by returning early from each test.
JDOM XXE Protection [CVE-2013-1856]
Conflicts:
	activesupport/test/xml_mini/jdom_engine_test.rb
fix protocol checking in sanitization [CVE-2013-1857]
Conflicts:
	actionpack/lib/action_controller/vendor/html-scanner/html/sanitizer.rb
Merge branch '3-2-13' into 3-2-stable
* 3-2-13:
  bumping to 3.2.13
  fix protocol checking in sanitization [CVE-2013-1857]
  JDOM XXE Protection [CVE-2013-1856]
  fix incorrect ^$ usage leading to XSS in sanitize_css [CVE-2013-1855]
  stop calling to_sym when building arel nodes [CVE-2013-1854]
  Merge pull request #9616 from exviva/multiple_select_name_double_square_brackets
  bumping to rc2
  Revert "Merge pull request #8209 from senny/backport_8176"
  Freeze columns only once per Result
  Preparing for 3.2.13.rc1 release
  Update CHANGELOGs for 3.2.13 release.

Conflicts:
	actionmailer/CHANGELOG.md
	actionpack/CHANGELOG.md
	activemodel/CHANGELOG.md
	activeresource/CHANGELOG.md
	activesupport/CHANGELOG.md
	railties/CHANGELOG.md
Add release dates to documentation [ci skip]
Set "March 18, 2013" as the release date for 3.2.13
Merge pull request #9785 from claudiob/add-3-2-13-release-date-to-cha…
…ngelogs

Add release dates to documentation [ci skip]
Change @env_config to @app_env_config
Moral of the story: One must be careful about lazily initializing
instance variables when subclassing.

I would like to draw your attention to #4652 where
the reader will see that there appears to be some kind of initialization issue
in rails.

The source of this issue is that:
1) Engine#env_config contains "@env_config ||= ..."
2) Application#env_config contains "@env_config ||= ..."
3) Threads are in the picture
4) Thread A calls Application#env_config, which super's to Engine#env_config
5) After Engine#env_config returns but before Application#env_config sets @env_config again, Thread B begins running
6) Thread B calls Application#env_config
7) Thread B finds @env_config to contain a value (the one set by Engine#env_config) and returns it
8) Thread B blows up because key set by Application#env_config are there.
9) People report bugs with puma, thin, rainbows, webrick, etc
10) Evan becomes tired of seeing these bugs
11) Evan pours himself a stiff drink, puts on Top Gear(tm), and begins debugging
12) Evan finds the source of the bug
13) Evan authors a PR
14) RIGHT NOW.

The bug is fixed by simply using a different ivar name in the methods.
Alternately, Engine#env_config could just return a new Hash each time, not memoizing into @env_config.

I bid you adieu.
Merge pull request #9789 from puma/3-2-stable
Change @env_config to @app_env_config
Contributor

alup commented on actionmailer/CHANGELOG.md in 23434f6 Mar 19, 2013

Shouldn't you have mentioned that mail gem have been upgraded to 2.5.x?

hoffm and others added some commits Feb 20, 2013

Reset postgreSQL search path in db:test:clone_structure.
This patch resets the postgres search path in the structure.sql after
the structure is dumped in order to find schema_migrations table when
multiples schemas are used.

Fixes #945
Merge pull request #9796 from dembskoi/clone_structure
Reset postgreSQL search path in db:test:clone_structure.
Contributor

rsanheim replied Mar 19, 2013

Also, a possible regression related to this commit: #9813

fredwu and others added some commits Mar 20, 2013

Fixed assets loading performance in 3.2.13
The PR #8756 uses Sprockets for resolving files that already exists on disk, for those files their extensions don't need to be rewritten.

Fixes #9803
Don't crash exception translation w/ nil result attribute.
Exception.result is nil when attempting a query after PostgreSQL
disconnect, resulting in new exception:
NoMethodError: undefined method `error_field' for nil:NilClass
Merge pull request #9802 from newsline/fix-broken-action-missing
Fix missing action_missing

Conflicts:
	actionpack/CHANGELOG.md

Conflicts:
	actionpack/test/controller/base_test.rb

Fixes #9799
Merge branch '3-2-stable' into fredwu-slow_view_loading_fix
* 3-2-stable:
  Merge pull request #9802 from newsline/fix-broken-action-missing
  Remove bad changelog entry from AR [ci skip]
  Wrong exception is occured when raising no translatable exception
  Don't crash exception translation w/ nil result attribute.

Conflicts:
	actionpack/CHANGELOG.md
Fix test to Ruby 1.8
Pathname doesn't respond to to_path
Contributor

exviva replied Mar 21, 2013

@cthiel this commit actually makes the conditional more restrictive. I think 175ba04 introduced the bug (see #9616).

@exviva you could be right! Thanks for the pointer.

senny and others added some commits Mar 22, 2013

do not reset associations when preloading twice.
Closes #9806.

As the `through_options` always contained `{:order=>nil}` the second time,
the preloader ran, the association was always reset. This patch only
adds the `:order` to the `through_options` if it is set.
Merge pull request #9870 from senny/9806_eager_loading_regression
do not reset associations when preloading twice.
Merge pull request #9873 from senny/use_old_hash_syntax
use ruby 1.8 Hash syntax [ci skip]
Backport #5808
df36c5f - Fix assert_template assertion with :layout option
4bd05a7 - Fix assert_template :layout => nil assertion
0d19a08 - Improve assert_template layout checking
Merge pull request #9576 from fredwu/backport_fix_explicitly_inherait…
…ance_column_to_3_2_stable

[Backport to 3-2-stable] Don't reset inheritance_column when setting explicitly.
Contributor

fredwu commented Mar 27, 2013

Oops, wrong branch sorry.

@fredwu fredwu closed this Mar 27, 2013

Thank you for this patch. We tried to include this as a monkey patch in our code but still have issues with js loading slow. Anything we are missing?

Nice. That reduces my page loading times in development mode from 1 second to about 200 milliseconds.

(from @raggi) this is a bug, as activesupport::orderedoptions does not support this

Was not this fixed yet on Rails 3.2.x ? I am referring to the bug on OrderedOptions

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment