fog / fog-google

Fog for Google Cloud Platform
MIT License
99 stars 146 forks source link

Double-splat (**options) operator fails on hash with string keys in put_object() call #535

Open jywarren opened 3 years ago

jywarren commented 3 years ago

We're seeing an error hash key "x-goog-acl" is not a Symbol in v1.15.0 (link to our PR):

https://sentry.io/share/issue/6fb1fbdfa23f417192acba7c65747e95/

It's related to the introduction of the ** "double splat" operator to this line 2 months ago in https://github.com/fog/fog-google/pull/523:

https://github.com/fog/fog-google/blob/0411601f78343cbb19874b082aef4225d5b7f038/lib/fog/storage/google_xml/requests/put_object.rb#L25

I believe it's redundant, in any case, since there's a default set in the function definition:

https://github.com/fog/fog-google/blob/0411601f78343cbb19874b082aef4225d5b7f038/lib/fog/storage/google_xml/requests/put_object.rb#L25

You can reproduce this behavior in the console with the same error occurring, just with dummy hashes:

2.6.6 :001 > puts {}

 => nil 
2.6.6 :002 > puts **{}

 => nil 
2.6.6 :003 > a = {}
 => {} 
2.6.6 :004 > a['one'] = 0
 => 0 
2.6.6 :005 > puts a
{"one"=>0}
 => nil 
2.6.6 :006 > puts **a
Traceback (most recent call last):
        4: from /home/warren/.rvm/rubies/ruby-2.6.6/bin/irb:23:in `<main>'
        3: from /home/warren/.rvm/rubies/ruby-2.6.6/bin/irb:23:in `load'
        2: from /home/warren/.rvm/rubies/ruby-2.6.6/lib/ruby/gems/2.6.0/gems/irb-1.0.0/exe/irb:11:in `<top (required)>'
        1: from (irb):6
TypeError (hash key "one" is not a Symbol)
2.6.6 :007 > b = {}
 => {} 
2.6.6 :008 > b[:one] = 0
 => 0 
2.6.6 :009 > puts b
{:one=>0}
 => nil 
2.6.6 :010 > puts **b
{:one=>0}
 => nil 

Thanks for the fog library, we really appreciate your hard work on it!!!!

Temikus commented 3 years ago

This was introduced due to Ruby 2.7/3.0 kwargs update, which doesn't work the same way, sadly (as I see you're reproducing on 2.6.6).

@jywarren looking into it - can you check if chucking on a .symbolize_keys on will help in your environment? E.g. **options.symbolize_keys

If it does I'll roll out a test version you can try.

jywarren commented 3 years ago

Ahh, hmm. That didn't seem to, was i doing it right?:

2.6.6 :002 > a = {}
 => {} 
2.6.6 :003 > a['one'] = 0
 => 0 
2.6.6 :004 > puts a
{"one"=>0}
 => nil 
2.6.6 :005 > puts **a
Traceback (most recent call last):
        4: from /Users/warren/.rvm/rubies/ruby-2.6.6/bin/irb:23:in `<main>'
        3: from /Users/warren/.rvm/rubies/ruby-2.6.6/bin/irb:23:in `load'
        2: from /Users/warren/.rvm/rubies/ruby-2.6.6/lib/ruby/gems/2.6.0/gems/irb-1.0.0/exe/irb:11:in `<top (required)>'
        1: from (irb):5
TypeError (hash key "one" is not a Symbol)
2.6.6 :006 > puts **a.symbolize_keys
Traceback (most recent call last):
        5: from /Users/warren/.rvm/rubies/ruby-2.6.6/bin/irb:23:in `<main>'
        4: from /Users/warren/.rvm/rubies/ruby-2.6.6/bin/irb:23:in `load'
        3: from /Users/warren/.rvm/rubies/ruby-2.6.6/lib/ruby/gems/2.6.0/gems/irb-1.0.0/exe/irb:11:in `<top (required)>'
        2: from (irb):6
        1: from (irb):6:in `rescue in irb_binding'
NoMethodError (undefined method `symbolize_keys' for {"one"=>0}:Hash)
2.6.6 :007 > 

Thank you for writing back! Is the double-splat completely necessary? It seems to be caught by the default arg set in:

def put_object(bucket_name, object_name, data, options = {}) isn't it?

Much appreciated!

Temikus commented 3 years ago

@jywarren Gah, my bad - forgot it's a rails method. .transform_keys(&:to_sym) should work starting ruby 2.5, can you check:

ruby: 2.6.5 go: 1.15.7 ~/ (master) [10:57:35]
temikus λ pry
[1] pry(main)> a = {}
=> {}
[2] pry(main)> a['one'] = 0
=> 0
[3] pry(main)> puts **a
TypeError: hash key "one" is not a Symbol
from (pry):3:in `__pry__'
[4] pry(main)> puts **a.transform_keys(&:to_sym)
{:one=>0}
=> nil
Temikus commented 3 years ago

And to clarify why I'm asking - I don't have a paperclip test app at the moment so I want to make sure this doesn't break further up the stack somewhere in your app.

jywarren commented 2 years ago
Somerville:~ warren$ rvm 2.6.6
Using /Users/warren/.rvm/gems/ruby-2.6.6
Somerville:~ warren$ irb
2.6.6 :001 > a = {}
 => {} 
2.6.6 :002 > a['one'] = 0
 => 0 
2.6.6 :003 > puts **a
Traceback (most recent call last):
        4: from /Users/warren/.rvm/rubies/ruby-2.6.6/bin/irb:23:in `<main>'
        3: from /Users/warren/.rvm/rubies/ruby-2.6.6/bin/irb:23:in `load'
        2: from /Users/warren/.rvm/rubies/ruby-2.6.6/lib/ruby/gems/2.6.0/gems/irb-1.0.0/exe/irb:11:in `<top (required)>'
        1: from (irb):3
TypeError (hash key "one" is not a Symbol)
2.6.6 :004 > puts **a.transform_keys(&:to_sym)
{:one=>0}
 => nil 

Looks good! Thanks! We skipped v1.15.0 but could this conceivably be part of v1.16.0 or later?

github-actions[bot] commented 2 years ago

This issue has been marked inactive and will be closed if no further activity occurs.

StarWar commented 2 years ago

I'm also encountering this same issue. #551

BrianHawley commented 1 year ago

The string keys in question are added by fog-google itself. From the Fog::Storage::GoogleXML::File code:

def save(options = {})
  requires :body, :directory, :key
  if options != {}
    Fog::Logger.deprecation("options param is deprecated, use acl= instead [light_black](#{caller.first})[/]")
  end
  options["x-goog-acl"] ||= @acl if @acl
  options["Cache-Control"] = cache_control if cache_control
  options["Content-Disposition"] = content_disposition if content_disposition
  options["Content-Encoding"] = content_encoding if content_encoding
  options["Content-MD5"] = content_md5 if content_md5
  options["Content-Type"] = content_type if content_type
  options["Expires"] = expires if expires
  options.merge!(metadata)

  data = service.put_object(directory.key, key, body, **options)
  merge_attributes(data.headers.reject { |key, _value| ["Content-Length", "Content-Type"].include?(key) })
  self.content_length = Fog::Storage.get_body_size(body)
  self.content_type ||= Fog::Storage.get_content_type(body)
  true
end

Note the string keys added to the options hash, which is then passed to service.put_object as **options. That won't work for Ruby versions less than 2.6. If you were to check the Ruby version and if it's less than 2.6, call this instead:

data = service.put_object(directory.key, key, body, options)

Then that would solve the issue.

Note that fog-google says it supports Ruby 2.0+, which is perfectly OK. Still, because of this issue, it stopped working on Ruby 2.5 or earlier starting with the 1.14.0 version. With this problem fixed, you could restore that support.