String#tr raises ArgumentError if given invalid range (like "z-a") #1665

Closed
wants to merge 6 commits into
from
View
@@ -1474,12 +1474,12 @@ def tr_trans(source, replacement, squeeze)
return if @num_bytes == 0
invert = source[0] == ?^ && source.length > 1
- expanded = source.tr_expand! nil, true
+ expanded = source.tr_expand! nil, Rubinius.ruby18?
size = source.size
src = source.__data__
if invert
- replacement.tr_expand! nil, false
+ replacement.tr_expand! nil, Rubinius.ruby18?
@dbussink

dbussink Apr 16, 2012

Owner

Looks like this changes behavior on 1.8? Is that intended?

@michalbugno

michalbugno Apr 16, 2012

Contributor

True, I'll try to cover it with some spec.

r = replacement.__data__[replacement.size-1]
table = Rubinius::Tuple.pattern 256, r
@@ -1491,12 +1491,17 @@ def tr_trans(source, replacement, squeeze)
else
table = Rubinius::Tuple.pattern 256, -1
- replacement.tr_expand! expanded, false
+ first_char = replacement[0]
+ replacement.tr_expand! expanded, Rubinius.ruby18?
repl = replacement.__data__
rsize = replacement.size
i = 0
while i < size
- r = repl[i] if i < rsize
+ if rsize == 0
+ r = first_char
+ elsif i < rsize
+ r = repl[i]
+ end
table[src[i]] = r
i += 1
end
@@ -1,4 +1,2 @@
fails:String#tr can replace a 7-bit ASCII character with a multibyte one
-fails:String#tr raises a ArgumentError a descending range in the replacement as containing just the start character
-fails:String#tr raises a ArgumentError a descending range in the source as empty
fails:String#tr supports non-injective replacements
View
@@ -1016,8 +1016,20 @@ namespace rubinius {
continue;
} else if(seq == '-') {
native_int max = ++i < bytes ? str[i] : -1;
- if(max >= 0 && chr > max && CBOOL(invalid_as_empty)) {
- i++;
+ if(max >= 0 && chr > max) {
+ if (CBOOL(invalid_as_empty)) {
+ i++;
+ } else {
+ std::stringstream ss;
+ if (isprint(chr) && isprint(max)) {
+ ss << "invalid range \"";
+ ss << (char)chr << "-" << (char)max;
+ ss << "\" in string transliteration";
+ } else {
+ ss << "invalid range in string transliteration";
+ }
+ Exception::argument_error(state, ss.str().c_str());
+ }
} else if(max >= 0) {
do {
if(tr_data.assign(chr)) return tr_replace(state, &tr_data);