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

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

Open
jywarren opened this issue Jun 29, 2021 · 8 comments
Labels

Comments

@jywarren
Copy link

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 #523:

def put_object(bucket_name, object_name, data, options = {})

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

def put_object(bucket_name, object_name, data, options = {})

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
Copy link
Member

Temikus commented Jul 3, 2021

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
Copy link
Author

jywarren commented Jul 9, 2021

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
Copy link
Member

Temikus commented Jul 10, 2021

@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
Copy link
Member

Temikus commented Jul 10, 2021

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
Copy link
Author

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
Copy link

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

@StarWar
Copy link

StarWar commented Oct 12, 2021

I'm also encountering this same issue. #551

@BrianHawley
Copy link

BrianHawley commented Sep 16, 2022

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
4 participants