Fixes #8631 request post? detection should remain unaffected by local inflections such as POS for Point Of Sale #8632

Merged
merged 1 commit into from Jan 16, 2013

2 participants

@asanghi

request post? detection should remain unaffected by local inflections such as POS for PointOfSale

@pixeltrix pixeltrix was assigned Dec 27, 2012
@asanghi

@pixeltrix Andrew, just a gentle nudge to address this please? I understand that underscore is used in a number of places but this particular use is working on a very limited set of inputs to underscore. What do you think?

@pixeltrix
Ruby on Rails member

@asanghi this needs to be against master doesn't it - problem appears there as well

@asanghi
@pixeltrix
Ruby on Rails member

Sorry, I forgot you mentioned that in #8631. I actually prefer the fix in 872d8c3 - can you update this PR to match that and squash the commits. Once that's done I'll merge it.

@asanghi

@pixeltrix squashed with solution from master and added changelog entry and test.

@pixeltrix pixeltrix commented on an outdated diff Jan 16, 2013
actionpack/lib/action_dispatch/http/request.rb
@@ -56,7 +56,13 @@ def key?(key)
RFC5789 = %w(PATCH)
HTTP_METHODS = RFC2616 + RFC2518 + RFC3253 + RFC3648 + RFC3744 + RFC5323 + RFC5789
- HTTP_METHOD_LOOKUP = Hash.new { |h, m| h[m] = m.underscore.to_sym if HTTP_METHODS.include?(m) }
+ HTTP_METHOD_LOOKUP = {}
+
+ # Populate the HTTP method lookup cache
+ HTTP_METHODS.each do |method|
+ HTTP_METHOD_LOOKUP[method] = method.underscore.to_sym
+ HTTP_METHOD_LOOKUP[method.to_sym] = method.underscore.to_sym
@pixeltrix
Ruby on Rails member
pixeltrix added a line comment Jan 16, 2013

Why the double assignment?

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

Good catch @pixeltrix . I was mucking around with tests earlier and it was left behind. Removed and squashed again.

@pixeltrix pixeltrix merged commit d3dcb4b into rails:3-2-stable Jan 16, 2013
@pixeltrix
Ruby on Rails member

Thanks @arunagw - I think we should add the test to master as well. Can you create a PR?

@asanghi

I'm sure you meant @asanghi and I will. :)

@pixeltrix
Ruby on Rails member

Yes, the autocomplete jumped to the wrong one and I didn't notice - thanks.

@asanghi

@pixeltrix .. see #8963 for pull on master

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