Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multiparameter POLA (principle of least authority) with respect to time_select fixes. See LH4346 #396

Merged
merged 1 commit into from May 6, 2011

Conversation

asanghi
Copy link
Contributor

@asanghi asanghi commented May 5, 2011

@asanghi
Copy link
Contributor Author

asanghi commented May 6, 2011

Context for this ticket.

The ticket deals with errors relating to multiparamter attributes.

  1. time_select allows for an option :ignore_date => true. This option was added here. It was added for design reasons where the developer wanted to style date_select and time_select differently and not have one overwrite the other.
  2. :ignore_date => true usage is not documented clearly enough. Leading to bugs as reported here.
  3. If you use :ignore_date => true with time_select and dont have a date_select on the same attribute, funny things will happen. Time attributes would shift to date and silently work (if the time attributes can be turned into a date) or throw up if the time attributes do not form into a valid date.
  4. @asanghi with the massive input from (@andreacampi) have addressed various issues with Multiparameter attribute handling. Some of those issues include
    • Handling Date, Time distinctly from other multiparameter classes separately.
    • Handling DOS type attack by providing very high parameter indexes.
    • Handling empty parameters for Date and Time.
    • Handling nil parameters for Date and Time.
    • Raising appropriate exceptions
    • Refactoring Multiparameter attribute handling.
    • Adding many more tests.

I believe this fix should be applied to 3.0.x and possibly even to 2.3.x branch.

@fxn
Copy link
Member

fxn commented May 6, 2011

OK, good to go.

It is going to be applied to master for 3.1. The 2.3 branch is practically frozen, only security fixes and the-universe-is-going-to-collapse bug fixes. As per 3.0, while the fix could be interesting there, we think that it is safer not to introduce something like this in a minor version update. It could break existing extensions dealing with multi value attributes as https://github.com/philcrissman/multiparams_for_strings.

Thanks for keeping this ticket alive, the rebases, and for the summary above :).

fxn added a commit that referenced this pull request May 6, 2011
Multiparameter POLA (principle of least authority) with respect to time_select fixes. See LH4346
@fxn fxn merged commit 29dbccf into rails:master May 6, 2011
@asanghi
Copy link
Contributor Author

asanghi commented May 6, 2011

@fxn there are 3 warnings in test cases added to base_test.rb

`rails/activesupport/lib/active_support/core_ext/time/calculations.rb:24: warning: 2 digits year is used``

Can you suggest a way to hush them up? I was at a loss.

matthewd pushed a commit that referenced this pull request Apr 24, 2018
Rename MIT-LICENSE to MIT-LICENSE.txt
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants