Project

General

Profile

Actions

Bug #18931

open

Inconsistent handling of invalid codepoints in String#lstrip and String#rstrip

Added by nirvdrum (Kevin Menard) 18 days ago. Updated 18 days ago.

Status:
Open
Priority:
Normal
Assignee:
-
Target version:
-
ruby -v:
ruby 3.1.2p20 (2022-04-12 revision 4491bb740a) [arm64-darwin21]
[ruby-core:109264]

Description

When attempting to strip a string, there are three basic options when an invalid code point is encountered:

  1. Ignore the code point
  2. Strip the code point
  3. Raise an exception

For background, Ruby does not consider the string's code range for lstrip or rstrip. It permits stripping strings with a ENC_CODERANGE_BROKEN so long as any invalid code points are not encountered while performing the loop to remove whitespace. What it does when such a code point is encountered, however, is not consistent between lstrip and rstrip.

String#lstrip will unconditionally raise an invalid byte sequence error:

> ruby -v
ruby 3.1.2p20 (2022-04-12 revision 4491bb740a) [arm64-darwin21]

> ruby -e 'p " \x80abc".lstrip'
-e:1:in `lstrip': invalid byte sequence in UTF-8 (ArgumentError)
	from -e:1:in `<main>'

> ruby -e 'p " \x80 abc".lstrip'
-e:1:in `lstrip': invalid byte sequence in UTF-8 (ArgumentError)
	from -e:1:in `<main>'

> ruby -e 'p "\x80 abc".lstrip'
-e:1:in `lstrip': invalid byte sequence in UTF-8 (ArgumentError)
	from -e:1:in `<main>'

> ruby -e 'p "\x80".lstrip'
-e:1:in `lstrip': invalid byte sequence in UTF-8 (ArgumentError)
	from -e:1:in `<main>'

> ruby -e ' p " a\x80bc".lstrip'
"a\x80bc"   # This one is okay because the broken code point appears after a non-whitespace code point.

Things get a lot messier with String#rstrip, however. Depending on context, rstrip may raise an exception, treat the broken code point as a non-whitespace boundary and stop processing, or treat the broken code point as if it were whitespace and remove it.

String#rstrip will ignore the invalid code point if it immediately follows a non-whitespace code point:

> ruby -v
ruby 3.1.2p20 (2022-04-12 revision 4491bb740a) [arm64-darwin21]

> ruby -e 'p "abc\x80 ".rstrip'
"abc\x80"

> ruby -e 'p "abc\x80".rstrip'
"abc\x80"

String#rstrip will remove the invalid code point if it is surround by whitespace:

> ruby -v
ruby 3.1.2p20 (2022-04-12 revision 4491bb740a) [arm64-darwin21]

> ruby -e 'p "abc \x80".rstrip'
"abc"

> ruby -e 'p "abc \x80 ".rstrip'
"abc"

> ruby -e 'p " \x80 ".rstrip'
""

String#rstrip will raise an exception if no valid, non-whitespace code points appear before it:

> ruby -v
ruby 3.1.2p20 (2022-04-12 revision 4491bb740a) [arm64-darwin21]

> ruby -e 'p "\x80 ".rstrip'
-e:1:in `rstrip': invalid byte sequence in UTF-8 (ArgumentError)
	from -e:1:in `<main>'

> ruby -e 'p "\x80".rstrip'
-e:1:in `rstrip': invalid byte sequence in UTF-8 (ArgumentError)
	from -e:1:in `<main>'

It looks to me like the current behavior is a byproduct of the functions chosen for finding code point boundaries, rather than something deliberately chosen. E.g., rb_str_lstrip will call rb_enc_codepoint_len, which raises on invalid code points, while rb_str_rstrip calls rb_enc_prev_char, which doesn't perform the same code point validation. I think it'd make for a better user experience if lstrip and rstrip behaved consistently with each other, which would then unify the behavior in rstrip. What that behavior should be needs to be decided and I'm hoping to reach consensus on the semantics in this issue.

Updated by nirvdrum (Kevin Menard) 18 days ago

My own take on three options, with no significance to the order, are:

Ignore the code point

The documentation for lstrip is "Returns a copy of the receiver with leading whitespace removed." It seems fairly straightforward and there's no mention of string validation; raising an exception might violate user expectations.
Treating broken code points the same as any other non-whitespace code point would be logically consistent. An additional benefit is the method could be implemented more efficiently as the whitespace check can be done without calculating code point boundaries. Only ASCII whitespace code points are stripped and those, by definition, are only one byte wide. However, if lstrip and rstrip ever evolve to handle non-ASCII whitespace we'll be back to calculating code point boundaries.

Strip the code point

Despite rstrip doing it in some cases, I don't think removing the invalid code points is what an end user would expect and runs counter to the method's documentation.

Raise an exception

Given lstrip's behavior, raising in all cases would be the most backward-compatible and is consistent with equivalent expressions (e.g., " \x80 abc".sub(/^\s+/, "") will raise an error on the invalid byte sequence). While the documentation makes no mention of string validation, encountering an invalid code point is arguably an exceptional condition.

Actions

Also available in: Atom PDF