From 2e9ac8f4489021093f3f9fde63ae58d64d2b47f3 Mon Sep 17 00:00:00 2001 From: dearblue Date: Sat, 15 Apr 2023 21:25:56 +0900 Subject: [PATCH 1/6] Returns `self` for `String#bytesplice` --- src/string.c | 4 ++-- test/t/string.rb | 36 ++++++++++++------------------------ 2 files changed, 14 insertions(+), 26 deletions(-) diff --git a/src/string.c b/src/string.c index de8a671ac..b29a298c1 100644 --- a/src/string.c +++ b/src/string.c @@ -2956,7 +2956,7 @@ str_bytesplice(mrb_state *mrb, mrb_value str, mrb_int idx1, mrb_int len1, mrb_va if (RSTRING_LEN(replace) <= idx2+len2) { len2 = RSTRING_LEN(replace) - idx2; } - if (len2 == 0) return replace; + if (len2 == 0) return str; mrb_str_modify(mrb, s); if (len1 >= len2) { memmove(RSTR_PTR(s)+idx1, RSTRING_PTR(replace)+idx2, len2); @@ -2971,7 +2971,7 @@ str_bytesplice(mrb_state *mrb, mrb_value str, mrb_int idx1, mrb_int len1, mrb_va memmove(RSTR_PTR(s)+idx1+len2, RSTR_PTR(s)+idx1+len1, slen-(idx1+len1)); memmove(RSTR_PTR(s)+idx1, RSTRING_PTR(replace)+idx2, len2); } - return replace; + return str; } /* diff --git a/test/t/string.rb b/test/t/string.rb index 892899346..caba51b7c 100644 --- a/test/t/string.rb +++ b/test/t/string.rb @@ -902,62 +902,50 @@ end assert('String#bytesplice') do # range, replace (len1=len2) a = "0123456789" - assert_equal "ab", a.bytesplice(1..2, "ab") - assert_equal "0ab3456789", a + assert_equal "0ab3456789", a.bytesplice(1..2, "ab") # range, replace (len1>len2) a = "0123456789" - assert_equal "ab", a.bytesplice(1..3, "ab") - assert_equal "0ab456789", a + assert_equal "0ab456789", a.bytesplice(1..3, "ab") # range, replace (len1len2) a = "0123456789" - assert_equal "ab", a.bytesplice(1, 3, "ab") - assert_equal "0ab456789", a + assert_equal "0ab456789", a.bytesplice(1, 3, "ab") # idx, len, replace (len1len2) a = "0123456789" - assert_equal b, a.bytesplice(1..3, b, 1..2) - assert_equal "0bc456789", a + assert_equal "0bc456789", a.bytesplice(1..3, b, 1..2) # range, replace, range (len1len2) a = "0123456789" - assert_equal b, a.bytesplice(1, 3, b, 1, 2) - assert_equal "0bc456789", a + assert_equal "0bc456789", a.bytesplice(1, 3, b, 1, 2) # idx, len, replace, idx, len (len1 Date: Sat, 15 Apr 2023 21:25:57 +0900 Subject: [PATCH 2/6] Need type check for `replace` by `String#bytesplice` --- src/string.c | 1 + test/t/string.rb | 3 +++ 2 files changed, 4 insertions(+) diff --git a/src/string.c b/src/string.c index b29a298c1..1bd3f6f8f 100644 --- a/src/string.c +++ b/src/string.c @@ -3006,6 +3006,7 @@ mrb_str_bytesplice(mrb_state *mrb, mrb_value str) mrb_get_args(mrb, "iiS", &idx1, &len1, &replace); return str_bytesplice(mrb, str, idx1, len1, replace, 0, RSTRING_LEN(replace)); } + mrb_ensure_string_type(mrb, replace); if (mrb_range_beg_len(mrb, range1, &idx1, &len1, RSTRING_LEN(str), FALSE) != MRB_RANGE_OK) break; if (mrb_range_beg_len(mrb, range2, &idx2, &len2, RSTRING_LEN(replace), FALSE) != MRB_RANGE_OK) break; return str_bytesplice(mrb, str, idx1, len1, replace, idx2, len2); diff --git a/test/t/string.rb b/test/t/string.rb index caba51b7c..7dbd5bca8 100644 --- a/test/t/string.rb +++ b/test/t/string.rb @@ -948,4 +948,7 @@ assert('String#bytesplice') do # idx, len, replace, idx, len (len1 Date: Sat, 15 Apr 2023 21:25:58 +0900 Subject: [PATCH 3/6] Countermeasure to overflow for `String#bytesplice` --- src/string.c | 5 +++-- test/t/string.rb | 4 ++++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/string.c b/src/string.c index 1bd3f6f8f..966d7c8c9 100644 --- a/src/string.c +++ b/src/string.c @@ -2950,10 +2950,11 @@ str_bytesplice(mrb_state *mrb, mrb_value str, mrb_int idx1, mrb_int len1, mrb_va if (RSTR_LEN(s) < idx1 || RSTRING_LEN(replace) < idx2) { mrb_raise(mrb, E_INDEX_ERROR, "index out of string"); } - if (RSTR_LEN(s) <= idx1+len1) { + mrb_int n; + if (mrb_int_add_overflow(idx1, len1, &n) || RSTR_LEN(s) < n) { len1 = RSTR_LEN(s) - idx1; } - if (RSTRING_LEN(replace) <= idx2+len2) { + if (mrb_int_add_overflow(idx2, len2, &n) || RSTRING_LEN(replace) < n) { len2 = RSTRING_LEN(replace) - idx2; } if (len2 == 0) return str; diff --git a/test/t/string.rb b/test/t/string.rb index 7dbd5bca8..5fe0761f6 100644 --- a/test/t/string.rb +++ b/test/t/string.rb @@ -951,4 +951,8 @@ assert('String#bytesplice') do # check the object type to replace assert_raise(TypeError) { "0123456789".bytesplice(1, 1, Object.new) } + + # check the overflow to index and length (to be pass without crash) + assert_nothing_raised { "0123456789".bytesplice(8, ~(-1 << 31), "ab") } # for MRB_INT32 + assert_nothing_raised { begin; "0123456789".bytesplice(8, ~(-1 << 63), "ab"); rescue ArgumentError, RangeError; end } # for MRB_INT64 end From 5bc3a48ecbe02a4a8249f0aeef1d2cfd99a27701 Mon Sep 17 00:00:00 2001 From: dearblue Date: Sat, 15 Apr 2023 21:25:59 +0900 Subject: [PATCH 4/6] Handling negative indices in `String#bytesplice` --- src/string.c | 8 +++++++- test/t/string.rb | 5 +++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/src/string.c b/src/string.c index 966d7c8c9..c191fcbb7 100644 --- a/src/string.c +++ b/src/string.c @@ -2947,7 +2947,13 @@ static mrb_value str_bytesplice(mrb_state *mrb, mrb_value str, mrb_int idx1, mrb_int len1, mrb_value replace, mrb_int idx2, mrb_int len2) { struct RString *s = RSTRING(str); - if (RSTR_LEN(s) < idx1 || RSTRING_LEN(replace) < idx2) { + if (idx1 < 0) { + idx1 += RSTR_LEN(s); + } + if (idx2 < 0) { + idx2 += RSTRING_LEN(replace); + } + if (RSTR_LEN(s) < idx1 || idx1 < 0 || RSTRING_LEN(replace) < idx2 || idx2 < 0) { mrb_raise(mrb, E_INDEX_ERROR, "index out of string"); } mrb_int n; diff --git a/test/t/string.rb b/test/t/string.rb index 5fe0761f6..0b9916b59 100644 --- a/test/t/string.rb +++ b/test/t/string.rb @@ -955,4 +955,9 @@ assert('String#bytesplice') do # check the overflow to index and length (to be pass without crash) assert_nothing_raised { "0123456789".bytesplice(8, ~(-1 << 31), "ab") } # for MRB_INT32 assert_nothing_raised { begin; "0123456789".bytesplice(8, ~(-1 << 63), "ab"); rescue ArgumentError, RangeError; end } # for MRB_INT64 + + # check the negative index + assert_equal "0ab3456789", "0123456789".bytesplice(-9, 2, "ab") + assert_equal "ab23456789", "0123456789".bytesplice(-10, 2, "ab") + assert_raise(IndexError) { "0123456789".bytesplice(-11, 2, "ab") } end From 59931e95c47a9b1e5b0d814d925ec7409c997f9b Mon Sep 17 00:00:00 2001 From: dearblue Date: Sat, 15 Apr 2023 21:26:00 +0900 Subject: [PATCH 5/6] Prohibit string lengths less than 0 in `String#bytesplice` --- src/string.c | 3 +++ test/t/string.rb | 3 +++ 2 files changed, 6 insertions(+) diff --git a/src/string.c b/src/string.c index c191fcbb7..d9b973866 100644 --- a/src/string.c +++ b/src/string.c @@ -2956,6 +2956,9 @@ str_bytesplice(mrb_state *mrb, mrb_value str, mrb_int idx1, mrb_int len1, mrb_va if (RSTR_LEN(s) < idx1 || idx1 < 0 || RSTRING_LEN(replace) < idx2 || idx2 < 0) { mrb_raise(mrb, E_INDEX_ERROR, "index out of string"); } + if (len1 < 0 || len2 < 0) { + mrb_raise(mrb, E_INDEX_ERROR, "negative length"); + } mrb_int n; if (mrb_int_add_overflow(idx1, len1, &n) || RSTR_LEN(s) < n) { len1 = RSTR_LEN(s) - idx1; diff --git a/test/t/string.rb b/test/t/string.rb index 0b9916b59..cb06be629 100644 --- a/test/t/string.rb +++ b/test/t/string.rb @@ -960,4 +960,7 @@ assert('String#bytesplice') do assert_equal "0ab3456789", "0123456789".bytesplice(-9, 2, "ab") assert_equal "ab23456789", "0123456789".bytesplice(-10, 2, "ab") assert_raise(IndexError) { "0123456789".bytesplice(-11, 2, "ab") } + + # check the negative length + assert_raise(IndexError) { "0123456789".bytesplice(3, -4, "ab") } end From 8665f887915d21b070b97c01f21e7f9d931009c8 Mon Sep 17 00:00:00 2001 From: dearblue Date: Sat, 15 Apr 2023 21:26:01 +0900 Subject: [PATCH 6/6] Allow an empty string for `String#bytesplice` --- src/string.c | 1 - test/t/string.rb | 3 +++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/string.c b/src/string.c index d9b973866..31cf47c04 100644 --- a/src/string.c +++ b/src/string.c @@ -2966,7 +2966,6 @@ str_bytesplice(mrb_state *mrb, mrb_value str, mrb_int idx1, mrb_int len1, mrb_va if (mrb_int_add_overflow(idx2, len2, &n) || RSTRING_LEN(replace) < n) { len2 = RSTRING_LEN(replace) - idx2; } - if (len2 == 0) return str; mrb_str_modify(mrb, s); if (len1 >= len2) { memmove(RSTR_PTR(s)+idx1, RSTRING_PTR(replace)+idx2, len2); diff --git a/test/t/string.rb b/test/t/string.rb index cb06be629..0bb9acfb3 100644 --- a/test/t/string.rb +++ b/test/t/string.rb @@ -963,4 +963,7 @@ assert('String#bytesplice') do # check the negative length assert_raise(IndexError) { "0123456789".bytesplice(3, -4, "ab") } + + # with an empty string + assert_equal "012789", "0123456789".bytesplice(3, 4, "") end