From 54b5a71c88cf90ed255475f00bdcd754024ed054 Mon Sep 17 00:00:00 2001 From: Akira Matsuda Date: Thu, 8 Oct 2026 11:17:16 +0900 Subject: [PATCH 1/2] An Array read boxed out of a Hash value takes its elements' in-place mutations `h.each { |k, vs| vs.each(&:strip!) }` (and each_value, `h[k].each`, a local bound to `h[k]`) iterates an Array the Hash holds as a boxed value, so the block parameter binds each element boxed. The pass that makes the Strings an iterator's block mutates in place into shared handles only looked at a local Array (typed or poly) or a literal, and passed over a boxed receiver: the Strings stored in the Array stayed plain, each strip!/upcase!/<< changed a copy, and the Hash printed them unchanged with no refusal. WEBrick's parse_header strips its header values this way, so a POST's Content-Length kept its leading space and was rejected. A boxed receiver that is a local (a block parameter included) or a call now demands the Strings stored into what it reads, as a poly Array does: the walk follows the block parameter, the index read or the local back to the Hash's stores and on into its Arrays. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/analyze.c | 19 ++++++++++++++ test/hash_value_array_element_bang.rb | 25 +++++++++++++++++++ .../hash_value_array_element_bang.rb.expected | 7 ++++++ 3 files changed, 51 insertions(+) create mode 100644 test/hash_value_array_element_bang.rb create mode 100644 test/hash_value_array_element_bang.rb.expected diff --git a/src/analyze.c b/src/analyze.c index acfd937e00..1d6d213b5c 100644 --- a/src/analyze.c +++ b/src/analyze.c @@ -18955,6 +18955,25 @@ static int promote_shared_stored_strings(Compiler *c) { nt_kind(nt, recv4) == NK_GlobalVariableReadNode ? gvar_array_plain_string(c, comp_resolve_gvar(c, nt_str(nt, recv4, "name") + 1)) : -1; if (gpl >= 0) refuse_global_array_element(c, w, recv4, gpl); + /* An Array read out of a boxed slot -- a Hash's value, through `h.each + { |k, vs| }`, `h[k]` or `vs = h[k]` -- is iterated as a boxed value, + so its elements bind the parameter boxed. Only a handle element takes + the mutation in place: demand the Strings stored there (the walk + follows the block parameter, the index read or the local back to the + stores into the Hash and on into its Arrays), as for a poly Array + below. Left alone, the element was a plain String and each mutation + changed a copy. */ + if (!lit4 && bpv4->type == TY_POLY && infer_type(c, recv4) == TY_POLY && + (nt_kind(nt, recv4) == NK_LocalVariableReadNode || nt_kind(nt, recv4) == NK_CallNode)) { + if (g_infer_optimistic) continue; + if (nt_kind(nt, recv4) == NK_LocalVariableReadNode) { + const char *pn4 = nt_str(nt, recv4, "name"); + Scope *ps4 = pn4 ? comp_scope_of(c, recv4) : NULL; + if (ps4) changed |= strbuf_demand_container_stores(c, pn4, ps4); + } + else changed |= strbuf_container_source_walk(c, recv4, 0, SB_DEMAND); + continue; + } if (!lit4 && nt_kind(nt, recv4) != NK_LocalVariableReadNode) continue; const char *contn4 = lit4 ? NULL : nt_str(nt, recv4, "name"); Scope *conts4 = contn4 ? comp_scope_of(c, recv4) : NULL; diff --git a/test/hash_value_array_element_bang.rb b/test/hash_value_array_element_bang.rb new file mode 100644 index 0000000000..d6862cc982 --- /dev/null +++ b/test/hash_value_array_element_bang.rb @@ -0,0 +1,25 @@ +# An Array held as a Hash's value is read back boxed, through `h.each { |k, +# vs| }`, `h.each_value`, `h[k]` or a local bound to `h[k]`. Iterating it +# and mutating each element in place (strip!, upcase!, <<) changed a copy: +# the iteration gate passed over a boxed receiver, so the Strings stored in +# the Array stayed plain and the Hash printed them unchanged. WEBrick's +# parse_header strips its header values this way. +h1 = { "k" => [+" 1"] } +h1.each { |k, vs| vs.each(&:strip!) } +h2 = { "k" => [+" 2"] } +h2.each { |k, vs| vs.each { |v| v.strip! } } +h3 = { "k" => [+" 3"] } +h3.each_value { |vs| vs.each(&:strip!) } +h4 = { "k" => [+" 4"] } +h4["k"].each(&:strip!) +h5 = { "k" => [+" 5"] } +vs = h5["k"] +vs.each(&:strip!) +h6 = { "k" => [+" 6"] } +h6.each { |k, vs| vs[0].strip! } +p h1, h2, h3, h4, h5, h6 + +h7 = { "a" => [+"x", +"y"], "b" => [+"z"] } +h7.each { |k, vs| vs.each { |v| v << "!" } } +h7.each_value { |vs| vs.each(&:upcase!) } +p h7 diff --git a/test/hash_value_array_element_bang.rb.expected b/test/hash_value_array_element_bang.rb.expected new file mode 100644 index 0000000000..bcc87a9237 --- /dev/null +++ b/test/hash_value_array_element_bang.rb.expected @@ -0,0 +1,7 @@ +{"k" => ["1"]} +{"k" => ["2"]} +{"k" => ["3"]} +{"k" => ["4"]} +{"k" => ["5"]} +{"k" => ["6"]} +{"a" => ["X!", "Y!"], "b" => ["Z!"]} From 3b5a8efefdedd0eb685dae4b815c527c9764d409 Mon Sep 17 00:00:00 2001 From: Akira Matsuda Date: Thu, 8 Oct 2026 11:18:05 +0900 Subject: [PATCH 2/2] A String pushed into an Array a Hash hands out is shared with its iteration `header[field] << value` (or `(h[k] ||= []) << s`) stores into the Array the Hash holds, not into the Hash. The walk that demands shared handles for Strings mutated a container level in (`h.each { |k, vs| vs.each(&:strip!) }`) followed only the stores into the Hash itself -- its literal and `h[k] = []`, an empty Array -- so the pushed Strings stayed plain and strip! changed a copy, silently. That is how WEBrick's parse_header builds and then strips its header values, so a POST's Content-Length kept its leading space. A store site that reads an element out of the container (`h[k]`, `fetch`, `h[k] ||= v`) now has the push it receives walked one level in. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/analyze.c | 16 +++++++++- test/hash_element_push_each_bang.rb | 33 ++++++++++++++++++++ test/hash_element_push_each_bang.rb.expected | 4 +++ 3 files changed, 52 insertions(+), 1 deletion(-) create mode 100644 test/hash_element_push_each_bang.rb create mode 100644 test/hash_element_push_each_bang.rb.expected diff --git a/src/analyze.c b/src/analyze.c index 1d6d213b5c..68742eae93 100644 --- a/src/analyze.c +++ b/src/analyze.c @@ -15310,7 +15310,8 @@ static const int *sb_store_nodes(Compiler *c, const char *nm, Scope *sc, int *n) const char *wn = nt_str(nt, id, "name"); if (wn) sb_store_add(wn, comp_scope_of(c, id), id); } - else if (k == NK_CallNode) { + /* `(h[k] ||= []) << s` hands out the element it reads or stores */ + else if (k == NK_CallNode || k == NK_IndexOrWriteNode) { int r = nt_ref(nt, id, "receiver"); if (r < 0 || nt_kind(nt, r) != NK_LocalVariableReadNode) continue; const char *rn = nt_str(nt, r, "name"); @@ -15601,6 +15602,19 @@ static int strbuf_demand_container_stores_here(Compiler *c, const char *contn, S int nst = strbuf_container_store_values(c, sns0[si], contn, conts, 1, &stores); for (int e3 = 0; e3 < nst; e3++) changed |= strbuf_store_leaf(c, stores.v[e3], depth, mode); + /* the strings sit a level in (`h.each { |k, vs| vs.each(&:strip!) }`) + and a push lands in an element the container hands out (`h[k] << + s`): the push stores into that inner container */ + int orw = nt_kind(nt, sns0[si]) == NK_IndexOrWriteNode; + if (orw) changed |= strbuf_store_leaf(c, nt_ref(nt, sns0[si], "value"), depth, mode); + if (mode >= SB_NEST1 && nst == 0 && (orw || container_elem_read_p(nt, sns0[si]))) { + int p = comp_recv_parent(c, sns0[si]); + StoreVals inner = {0}; + int ni = p >= 0 && nt_kind(nt, p) == NK_CallNode ? array_call_store_values(c, p, &inner) : 0; + for (int e3 = 0; e3 < ni; e3++) + changed |= strbuf_store_leaf(c, inner.v[e3], depth, mode - SB_NEST1); + free(inner.v); + } /* `a.insert(0, s)`: a store this walk does not take (sa_refuse) */ for (int k = 0, e; mode == SB_DEMAND && nst == 0 && (e = sa_unseen_element(c, sns0[si], k)) != -2; k++) if (e >= 0) sa_refuse_element(c, e, sns0[si]); diff --git a/test/hash_element_push_each_bang.rb b/test/hash_element_push_each_bang.rb new file mode 100644 index 0000000000..aca1a43641 --- /dev/null +++ b/test/hash_element_push_each_bang.rb @@ -0,0 +1,33 @@ +# Strings pushed into an Array the Hash hands out (`h[k] << s`, as WEBrick's +# parse_header builds its header) and then mutated in place through an +# iteration of the Hash's values: the walk that makes those Strings shared +# handles followed only the stores into the Hash itself (its literal and +# `h[k] = []`), not the pushes into the Arrays it holds, so strip! changed a +# copy and the header kept its leading space. +def parse_header(raw) + header = Hash.new([].freeze) + field = nil + raw.each_line { |line| + case line + when /^([A-Za-z0-9\-]+):([^\r\n\0]*?)\r\n\z/m + field, value = $1, $2 + field.downcase! + header[field] = [] unless header.has_key?(field) + header[field] << value + when /^[ \t]+([^\r\n\0]*?)\r\n/m + header[field][-1] << " " << $1 + end + } + header.each { |key, values| values.each(&:strip!) } + header +end + +h = parse_header("Content-Length: 12\r\nX-A: a\r\n b\r\nX-A: c \r\n") +p h +p h["content-length"][0] +p Integer(h["content-length"][0]) + +tags = {} +%w[a b a].each { |k| (tags[k] ||= []) << +" #{k} " } +tags.each_value { |vs| vs.each(&:strip!) } +p tags diff --git a/test/hash_element_push_each_bang.rb.expected b/test/hash_element_push_each_bang.rb.expected new file mode 100644 index 0000000000..345ff124f9 --- /dev/null +++ b/test/hash_element_push_each_bang.rb.expected @@ -0,0 +1,4 @@ +{"content-length" => ["12"], "x-a" => ["a b", "c"]} +"12" +12 +{"a" => ["a", "a"], "b" => ["b"]}