Fix SystemStackError with ActiveRecord #27

Merged
merged 1 commit into from May 12, 2016

Conversation

Projects
None yet
3 participants
@garethrees
Contributor

garethrees commented Oct 8, 2015

Fixes #26

If you use the default otp_counter column name, InstanceMethodsOnActivation#otp_counter calls otp_counter resulting in a stack overflow.

test/schema.rb
+ActiveRecord::Schema.define do
+ self.verbose = false
+
+ create_table :activerecord_users, :force => true do |t|

This comment has been minimized.

@houndci-bot

houndci-bot Oct 8, 2015

Use the new Ruby 1.9 hash syntax.

@houndci-bot

houndci-bot Oct 8, 2015

Use the new Ruby 1.9 hash syntax.

test/schema.rb
+ t.string :otp_secret_key
+ t.timestamps
+ end
+

This comment has been minimized.

@houndci-bot

houndci-bot Oct 8, 2015

Extra empty line detected at block body end.

@houndci-bot

houndci-bot Oct 8, 2015

Extra empty line detected at block body end.

@@ -13,6 +13,10 @@ def setup
@member = Member.new
@member.email = nil
@member.run_callbacks :create
+
+ @ar_user = ActiverecordUser.new
+ @ar_user.email = 'roberto@heapsource.com'

This comment has been minimized.

@houndci-bot

houndci-bot Oct 8, 2015

Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

@houndci-bot

houndci-bot Oct 8, 2015

Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

@@ -34,6 +38,17 @@ def test_counter_based_otp
assert code != @member.otp_code(auto_increment: true)
end
+ def test_counter_based_otp_active_record

This comment has been minimized.

@houndci-bot

houndci-bot Oct 8, 2015

Assignment Branch Condition size for test_counter_based_otp_active_record is too high. [17.12/15]

@houndci-bot

houndci-bot Oct 8, 2015

Assignment Branch Condition size for test_counter_based_otp_active_record is too high. [17.12/15]

lib/active_model/one_time_password.rb
end
def otp_counter=(attr)
- self.public_send("#{self.class.otp_counter_column_name}=", attr)
+ if self.class.otp_counter_column_name != 'otp_counter'

This comment has been minimized.

@houndci-bot

houndci-bot Oct 8, 2015

Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

@houndci-bot

houndci-bot Oct 8, 2015

Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

lib/active_model/one_time_password.rb
@@ -96,11 +96,19 @@ def otp_column=(attr)
end
def otp_counter
- self.public_send(self.class.otp_counter_column_name)
+ if self.class.otp_counter_column_name != 'otp_counter'

This comment has been minimized.

@houndci-bot

houndci-bot Oct 8, 2015

Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

@houndci-bot

houndci-bot Oct 8, 2015

Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

lib/active_model/one_time_password.rb
end
def otp_counter=(attr)
- self.public_send("#{self.class.otp_counter_column_name}=", attr)
+ if self.class.otp_counter_column_name != 'otp_counter'
+ self.public_send("#{self.class.otp_counter_column_name}=", attr)

This comment has been minimized.

@houndci-bot

houndci-bot Oct 8, 2015

Redundant self detected.

@houndci-bot

houndci-bot Oct 8, 2015

Redundant self detected.

lib/active_model/one_time_password.rb
@@ -96,11 +96,19 @@ def otp_column=(attr)
end
def otp_counter
- self.public_send(self.class.otp_counter_column_name)
+ if self.class.otp_counter_column_name != 'otp_counter'
+ self.public_send(self.class.otp_counter_column_name)

This comment has been minimized.

@houndci-bot

houndci-bot Oct 8, 2015

Redundant self detected.

@houndci-bot

houndci-bot Oct 8, 2015

Redundant self detected.

@garethrees

This comment has been minimized.

Show comment
Hide comment
@garethrees

garethrees Oct 8, 2015

Contributor

Cleared up some Hound warnings – remaining ones are just following existing conventions.

Contributor

garethrees commented Oct 8, 2015

Cleared up some Hound warnings – remaining ones are just following existing conventions.

ext/mkrf_conf.rb
+ end
+rescue => e
+ warn "#{$0}: #{e}"
+

This comment has been minimized.

@houndci-bot

houndci-bot Oct 8, 2015

Trailing whitespace detected.

@houndci-bot

houndci-bot Oct 8, 2015

Trailing whitespace detected.

ext/mkrf_conf.rb
+puts "Writing fake Rakefile"
+
+# Write fake Rakefile for rake since Makefile isn't used
+File.open(File.join(File.dirname(__FILE__), 'Rakefile'), 'w') do |f|

This comment has been minimized.

@houndci-bot

houndci-bot Oct 8, 2015

Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

@houndci-bot

houndci-bot Oct 8, 2015

Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

Fix SystemStackError with ActiveRecord
If you use the default `otp_counter` column name,
InstanceMethodsOnActivation#otp_counter calls `otp_counter` resulting in
a stack overflow.

garethrees referenced this pull request in mysociety/alaveteli Oct 9, 2015

Add has_one_time_passwords and support methods
- Adds forked gem for temporary fix; see commit ref for details
- Regenerates the secret and counter when enabled
@garethrees

This comment has been minimized.

Show comment
Hide comment
@garethrees

garethrees Oct 26, 2015

Contributor

Anything I can do to help get this merged? Cheers.

Contributor

garethrees commented Oct 26, 2015

Anything I can do to help get this merged? Cheers.

@robertomiranda

This comment has been minimized.

Show comment
Hide comment
@robertomiranda

robertomiranda Oct 26, 2015

Member

@garethrees sorry for the delay, I'm going to take a look today

Member

robertomiranda commented Oct 26, 2015

@garethrees sorry for the delay, I'm going to take a look today

@garethrees

This comment has been minimized.

Show comment
Hide comment
@garethrees

garethrees Oct 26, 2015

Contributor

@robertomiranda thanks, much appreciated! Feel free to get in touch if there's anything you need to clarify :) 🍻

Contributor

garethrees commented Oct 26, 2015

@robertomiranda thanks, much appreciated! Feel free to get in touch if there's anything you need to clarify :) 🍻

@robertomiranda robertomiranda merged commit c342283 into heapsource:master May 12, 2016

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
hound 4 violations found.

mysociety-pusher pushed a commit to mysociety/alaveteli that referenced this pull request May 17, 2016

@garethrees garethrees referenced this pull request in mysociety/alaveteli May 17, 2016

Merged

Return to source repo for active_model_otp #3254

@garethrees garethrees deleted the garethrees:stack-too-deep branch May 20, 2016

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