CVE-2013-6417 #30

Merged
merged 2 commits into from Dec 3, 2013

3 participants

@jbarnette jbarnette commented on the diff Dec 3, 2013
actionpack/lib/action_controller/request.rb
@@ -469,6 +469,22 @@ def named_host?(host)
!(host.nil? || /\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}$/.match(host))
end
+ # Remove nils from the params hash
+ def deep_munge(hash)
+ hash.each do |k, v|
+ case v
+ when Array
+ v.grep(Hash) { |x| deep_munge(x) }
+ v.compact!
+ hash[k] = nil if v.empty?
+ when Hash
+ deep_munge(v)
+ end
+ end
+
+ hash
@jbarnette
GitHub member

each returns the receiver if you feel like ditching this.

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

👍

@charliesome
GitHub member
@mastahyeti mastahyeti commented on the diff Dec 3, 2013
actionpack/lib/action_controller/request.rb
@@ -469,6 +469,22 @@ def named_host?(host)
!(host.nil? || /\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}$/.match(host))
end
+ # Remove nils from the params hash
+ def deep_munge(hash)
+ hash.each do |k, v|
+ case v
+ when Array
+ v.grep(Hash) { |x| deep_munge(x) }
+ v.compact!
+ hash[k] = nil if v.empty?
@mastahyeti
GitHub member

This seems bad. Isn't the intent here to remove nil values from the hash? It seems that it would be better to delete the key instead.

@mastahyeti
GitHub member

I'm obviously missing something though.

@mastahyeti
GitHub member

Yes, we're trying to avoid [nil], not nil. Don't mind me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@tnm tnm merged commit d13866d into 2-3-github Dec 3, 2013

1 check passed

Details default Build #810748 succeeded in 62s
@tnm tnm deleted the CVE-2013-6417 branch Dec 3, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment