Project

General

Profile

Bug #11661

sprintf causes a KeyError instead of using a default value for hash substitution

Added by eddyluten (Eddy Luten) almost 4 years ago. Updated almost 4 years ago.

Status:
Closed
Priority:
Normal
Target version:
-
[ruby-core:71354]

Description

When using a format string that substitutes hash values (or using the % operator on a string), instead of using the Hash's default value if a key is not present, it causes a KeyError.

As an end-user, to get around this, my hash needs to know about all the possible keys ahead of time and pre-assign a value to them or handle the KeyError. Logically, I would assume that the sprintf implementation would use the default Hash value.

I wanted to open this issue to see what your collective thoughts were on this since I have a fork running locally that fixes this issue and was wondering if I could send a patch/PR for this.

This issue is reproducible using the following code:

my_hash = Hash.new('world')
puts "hello %{location}" % my_hash # expecting "hello world"
# "KeyError: key{location} not found"

Related issues

Related to Ruby master - Bug #11677: 52530の変更により、sprintfの引数のhashにkeyが存在していてもvalueがnilのときにKeyErrorがでるようになってしまった。ClosedActions

Associated revisions

Revision def4370a
Added by nobu (Nobuyoshi Nakada) almost 4 years ago

sprintf.c: hash default value

  • sprintf.c (rb_str_format): respect default value of a hash. no longer raises KeyError unless the default value of the hash is nil. [ruby-core:71354] [Bug #11661]

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@52530 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

Revision 52530
Added by nobu (Nobuyoshi Nakada) almost 4 years ago

sprintf.c: hash default value

  • sprintf.c (rb_str_format): respect default value of a hash. no longer raises KeyError unless the default value of the hash is nil. [ruby-core:71354] [Bug #11661]

Revision 52530
Added by nobu (Nobuyoshi Nakada) almost 4 years ago

sprintf.c: hash default value

  • sprintf.c (rb_str_format): respect default value of a hash. no longer raises KeyError unless the default value of the hash is nil. [ruby-core:71354] [Bug #11661]

Revision 52530
Added by nobu (Nobuyoshi Nakada) almost 4 years ago

sprintf.c: hash default value

  • sprintf.c (rb_str_format): respect default value of a hash. no longer raises KeyError unless the default value of the hash is nil. [ruby-core:71354] [Bug #11661]

Revision 52530
Added by nobu (Nobuyoshi Nakada) almost 4 years ago

sprintf.c: hash default value

  • sprintf.c (rb_str_format): respect default value of a hash. no longer raises KeyError unless the default value of the hash is nil. [ruby-core:71354] [Bug #11661]

Revision 52530
Added by nobu (Nobuyoshi Nakada) almost 4 years ago

sprintf.c: hash default value

  • sprintf.c (rb_str_format): respect default value of a hash. no longer raises KeyError unless the default value of the hash is nil. [ruby-core:71354] [Bug #11661]

History

Updated by nobu (Nobuyoshi Nakada) almost 4 years ago

  • Tracker changed from Bug to Feature
  • Status changed from Open to Feedback

GH-1087 doesn't work properly with a default proc expecting an argument, but segfaults.

My patch.

diff --git i/sprintf.c w/sprintf.c
index 705d3ce..eee5695 100644
--- i/sprintf.c
+++ w/sprintf.c
@@ -605,13 +605,10 @@ rb_str_format(int argc, const VALUE *argv, VALUE fmt)
        }
        CHECKNAMEARG(start, len, enc);
        get_hash(&hash, argc, argv);
-       sym = rb_check_symbol_cstr(start + 1,
-                      len - 2 /* without parenthesis */,
-                      enc);
-       if (sym != Qnil) nextvalue = rb_hash_lookup2(hash, sym, Qundef);
-       if (nextvalue == Qundef) {
-           rb_enc_raise(enc, rb_eKeyError, "key%.*s not found", len, start);
-       }
+       sym = rb_cstr_intern(start + 1,
+                    len - 2 /* without parenthesis */,
+                    enc);
+       nextvalue = rb_hash_aref(hash, sym);
        if (term == '}') goto format_s;
        p++;
        goto retry;

Updated by nobu (Nobuyoshi Nakada) almost 4 years ago

  • Status changed from Feedback to Assigned

Updated by nobu (Nobuyoshi Nakada) almost 4 years ago

  • Tracker changed from Feature to Bug
  • Backport set to 2.0.0: REQUIRED, 2.1: REQUIRED, 2.2: REQUIRED

Updated by Hanmac (Hans Mackowiak) almost 4 years ago

hm might it be possible that sprintf only uses default value/or default proc if the hash does has default,
but does still raise KeyError if it doesnt?

another idea would be that is does try to use hash[key] function if hash is a non-hash object.
(but that might reduce the calcing speed if not checked right)

Updated by eddyluten (Eddy Luten) almost 4 years ago

Hans Mackowiak wrote:

hm might it be possible that sprintf only uses default value/or default proc if the hash does has default,
but does still raise KeyError if it doesnt?

another idea would be that is does try to use hash[key] function if hash is a non-hash object.
(but that might reduce the calcing speed if not checked right)

Nobuyoshi Nakada: this was the change I initially made myself but couldn't get it to work properly with hash substitutions. Maybe I did something wrong in my testing, I will attempt to use your changes, thanks! :)

Hans Mackowiak: so raise KeyError is the default is nil? I think that could work, since if you wanted empty strings as a default, you could simply set an empty string as default value. However, logically, since "#{nil}" (nil.to_s as well for that case) works and returns an empty string, the modulus operator version of it should work the same. What do you think?

Also a general question, how would I go about making the Rubyspec tests pass for this particular pull request? Do I open another PR for the tests in Rubyspec? If so, how do I link these two PRs to work together in the Travis CI build acceptance test?

Updated by nobu (Nobuyoshi Nakada) almost 4 years ago

Unfortunately, this change conflict with rubyspec.
How should we do:

  1. backport the fix and remove the failed spec
  2. add version guard and fix only the trunk
  3. keep the current behavior
#7

Updated by nobu (Nobuyoshi Nakada) almost 4 years ago

  • Status changed from Assigned to Closed

Applied in changeset r52530.


sprintf.c: hash default value

  • sprintf.c (rb_str_format): respect default value of a hash. no longer raises KeyError unless the default value of the hash is nil. [ruby-core:71354] [Bug #11661]
#8

Updated by usa (Usaku NAKAMURA) almost 4 years ago

  • Related to Bug #11677: 52530の変更により、sprintfの引数のhashにkeyが存在していてもvalueがnilのときにKeyErrorがでるようになってしまった。 added

Also available in: Atom PDF