September 2023 - ruby-core - ml.ruby-lang.org

[ruby-core:113819] [Ruby master Feature#19720] Warning for non-linear Regexps
by Eregon (Benoit Daloze) 14 Sep '23

14 Sep '23

Issue #19720 has been reported by Eregon (Benoit Daloze). ---------------------------------------- Feature #19720: Warning for non-linear Regexps https://bugs.ruby-lang.org/issues/19720 * Author: Eregon (Benoit Daloze) * Status: Open * Priority: Normal ---------------------------------------- I believe the best way to solve ReDoS is to ensure all Regexps used in the process are linear. Using `Regexp.timeout = 5.0` or so does not really prevent ReDoS, given enough requests causing that timeout the servers will still be very unresponsive. To this purpose, we should make it easy to identify non-linear Regexps and fix them. I suggest we either use 1. a performance warning (enabled with `Warning[:performance] = true`, #19538) or 2. a new regexp warning category (enabled with `Warning[:regexp] = true`). I think we should warn only once per non-linear Regexp, to avoid too many such warnings. We could warn as soon as the Regexp is created, or on first match. On first match might makes more sense for Ruby implementations which compile the Regexp lazily (since that is costly during startup), and also avoids warning for Regexps which are never used (which can be good or bad). OTOH, if the warning is enabled, we could always compile the Regexp eagerly (or at least checks whether it's linear), and that would then provide a better way to guarantee that all Regexps created so far are linear. Because warnings are easily customizable, it is also possible to e.g. `raise/abort` on such a warning, if one wants to ensure their application does not use a non-linear Regexp and so cannot be vulnerable to ReDoS: ```ruby Warning.extend Module.new { def warn(message, category: nil, **) raise message if category == :regexp super end } ``` A regexp warning category seems better for that as it makes it easy to filter by category, if a performance warning one would need to match the message which is less clean. As a note, TruffleRuby already has a similar warning, as a command-line option: ``` $ truffleruby --experimental-options --warn-truffle-regex-compile-fallback -e 'Gem' truffleruby-dev/lib/mri/rubygems/version.rb:176: warning: Regexp /\A\s*([0-9]+(?>\.[0-9a-zA-Z]+)*(-[0-9A-Za-z-]+(\.[0-9A-Za-z-]+)*)?)?\s*\z/ at_start=false encoding=US-ASCII requires backtracking and will not match in linear time truffleruby-dev/lib/mri/rubygems/requirement.rb:105: warning: Regexp /\A\s*(=|!=|>|<|>=|<=|~>)?\s*([0-9]+(?>\.[0-9a-zA-Z]+)*(-[0-9A-Za-z-]+(\.[0-9A-Za-z-]+)*)?)\s*\z/ at_start=false encoding=US-ASCII requires backtracking and will not match in linear time ``` So the warning message could be like `FILE:LINE: warning: Regexp /REGEXP/ requires backtracking and might not match in linear time and might cause ReDoS` or more concise: `FILE:LINE: warning: Regexp /REGEXP/ requires backtracking and might cause ReDoS` -- https://bugs.ruby-lang.org/

5 10

[ruby-core:114757] [Ruby master Bug#9115] Logger traps all exceptions; breaks Timeout
by Eregon (Benoit Daloze) 14 Sep '23

14 Sep '23

Issue #9115 has been updated by Eregon (Benoit Daloze). mame (Yusuke Endoh) wrote in #note-11: > As a better-than-nothing mitigation, it is proposed to enclose the entire `Logger::LogDevice#write` in `Thread.handle_interrupt(:never) { ... }`. > It could be a problem if the method takes a very long time because `Timeout.timeout` cannot interrupt the execution, but such a case will be rare (hopefully). This sounds problematic, especially since that is doing IO, possibly even network IO (e.g. NFS, or logging over some API). I believe all libraries should only catch specific errors they want to rescue and if that's hard to determine than StandardError at most, never Exception. Rescuing Exception without re-raise is always a bug (e.g. NoMemoryError/SystemStackError can be silent and that can cause pretty serious inconsistencies and indirectly what looks like memory corruption). ---------------------------------------- Bug #9115: Logger traps all exceptions; breaks Timeout https://bugs.ruby-lang.org/issues/9115#change-104594 * Author: cphoenix (Chris Phoenix) * Status: Assigned * Priority: Normal * Assignee: sonots (Naotoshi Seo) * ruby -v: ruby 2.0.0p247 (2013-06-27) [i386-mingw32] ---------------------------------------- Line 577-579 of logger.rb rescue Exception => ignored warn("log writing failed. #{ignored}") end Thus, when the system times out in the middle of writing a log message, it warns "log writing failed. execution expired" and just keeps right on running. This is true in 1.9.3 as well. I haven't looked at older versions. Pardon me while I go grep "rescue Exception" in the entire Ruby codebase, and see whether I can reliably use Timeout at all... OK, you might check out C:\Ruby200\lib\ruby\gems\2.0.0\gems\activerecord-3.2.13\lib\active_record\railties\databases.rake All the other "rescue Exception" seem to re-raise it, except maybe C:\Ruby200\lib\ruby\2.0.0\xmlrpc\server.rb and C:\Ruby200\lib\ruby\gems\2.0.0\gems\activesupport-3.2.13\lib\active_support\callbacks.rb -- https://bugs.ruby-lang.org/

1 0

[ruby-core:114501] [Ruby master Bug#19849] Requiring file with autoload results in confusing error if file doesn't exist
by p8 (Petrik de Heus) 14 Sep '23

14 Sep '23

Issue #19849 has been reported by p8 (Petrik de Heus). ---------------------------------------- Bug #19849: Requiring file with autoload results in confusing error if file doesn't exist https://bugs.ruby-lang.org/issues/19849 * Author: p8 (Petrik de Heus) * Status: Open * Priority: Normal * Backport: 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN ---------------------------------------- Given the following file: ```ruby # example.rb require_relative 'autoload_example.rb' Example.new ``` and the constant `Example` is defined using autoload, with an unknown path: ```ruby # autoload_example.rb autoload "Example", "path_unknown" ``` Running `ruby example.rb` results in the following error: ``` <internal:~/.rubies/ruby-3.2.2/lib/ruby/3.2.0/rubygems/core_ext/kernel_require.rb>:85:in `require': cannot load such file -- path_unknown (LoadError) from <internal:~/.rubies/ruby-3.2.2/lib/ruby/3.2.0/rubygems/core_ext/kernel_require.rb>:85:in `require' from example.rb:1:in `<main>' ``` The error is somewhat confusing as it doesn't show the location of the `autoload` which caused the error. -- https://bugs.ruby-lang.org/

3 2

[ruby-core:114517] [Ruby master Bug#19850] Get thread creation time
by ngan (Ngan Pham) 14 Sep '23

14 Sep '23

Issue #19850 has been reported by ngan (Ngan Pham). ---------------------------------------- Bug #19850: Get thread creation time https://bugs.ruby-lang.org/issues/19850 * Author: ngan (Ngan Pham) * Status: Open * Priority: Normal * Backport: 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN ---------------------------------------- I was debugging some threads hanging issue and thought it would be nice to know how long a thread has been around for. Would there be interest in a `Thread#creation_time` (or some other name) that returns the time when the thread was created? -- https://bugs.ruby-lang.org/

3 4

[ruby-core:114610] [Ruby master Bug#19859] Start & Finish, Begin & End
by thoran (thoran not blank) 14 Sep '23

14 Sep '23

Issue #19859 has been reported by thoran (thoran not blank). ---------------------------------------- Bug #19859: Start & Finish, Begin & End https://bugs.ruby-lang.org/issues/19859 * Author: thoran (thoran not blank) * Status: Open * Priority: Normal * Backport: 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN ---------------------------------------- In English "start" is paired with "finish" and "begin" is paired with "end". While "start" and "begin", and "finish" and "end" have similar meanings they are not the same. Hence it should be: String#begin_with? & String#end_with? or maybe String#start_with? & String#finish_with? or both; but not certainly not String#start_with? & String#end_with? as it is presently. Further, I would argue that it should be only String#begin_with? & String#end_with? because "start" and "finish" imply temporality and strings don't have that connotation. Hence the solution here for me would be to: /String#start_with?/String#begin_with?/. If however there is a desire to retain interface compatibility and to adhere to TMTOWTDI, then introduce both: String#begin_with? and String#finish_with? -- https://bugs.ruby-lang.org/

5 4

[ruby-core:114753] [Ruby master Bug#9115] Logger traps all exceptions; breaks Timeout
by mame (Yusuke Endoh) 14 Sep '23

14 Sep '23

Issue #9115 has been updated by mame (Yusuke Endoh). Discussed at the dev meeting. * It is clear that Logger has the intention not to interfere with the execution of the application on a log write failure. * On the other hand, it is also clear that Logger is not considering about asynchronous exceptions. As a better-than-nothing mitigation, it is proposed to enclose the entire `Logger::LogDevice#write` in `Thread.handle_interrupt(:never) { ... }`. It could be a problem if the method takes a very long time because `Timeout.timeout` cannot interrupt the execution, but such a case will be rare (hopefully). What exception `Timeout.timeout` raises is user-configurable, so it would be difficult to reraise only a specific exception class. It is ultimately up to the maintainer, @sonots, to decide. ---------------------------------------- Bug #9115: Logger traps all exceptions; breaks Timeout https://bugs.ruby-lang.org/issues/9115#change-104590 * Author: cphoenix (Chris Phoenix) * Status: Assigned * Priority: Normal * Assignee: sonots (Naotoshi Seo) * ruby -v: ruby 2.0.0p247 (2013-06-27) [i386-mingw32] ---------------------------------------- Line 577-579 of logger.rb rescue Exception => ignored warn("log writing failed. #{ignored}") end Thus, when the system times out in the middle of writing a log message, it warns "log writing failed. execution expired" and just keeps right on running. This is true in 1.9.3 as well. I haven't looked at older versions. Pardon me while I go grep "rescue Exception" in the entire Ruby codebase, and see whether I can reliably use Timeout at all... OK, you might check out C:\Ruby200\lib\ruby\gems\2.0.0\gems\activerecord-3.2.13\lib\active_record\railties\databases.rake All the other "rescue Exception" seem to re-raise it, except maybe C:\Ruby200\lib\ruby\2.0.0\xmlrpc\server.rb and C:\Ruby200\lib\ruby\gems\2.0.0\gems\activesupport-3.2.13\lib\active_support\callbacks.rb -- https://bugs.ruby-lang.org/

1 0

[ruby-core:114751] [Ruby master Bug#17146] Queue operations are allowed after it is frozen
by mame (Yusuke Endoh) 14 Sep '23

14 Sep '23

Issue #17146 has been updated by mame (Yusuke Endoh). I would like to add a point that came up during the dev meeting: if a thread is waiting for `Queue#pop`, and another thread freezes the Queue, what should the first thread do? Wait forever? Raise a FrozenError? To raise a FrozenError, it needs to be told that the Queue was asynchronously frozen by another thread. In other words, `Queue#freeze` needs to wake up the thread waiting for the Queue, which is likely to do more than `#freeze` would normally be expected to do. ---------------------------------------- Bug #17146: Queue operations are allowed after it is frozen https://bugs.ruby-lang.org/issues/17146#change-104587 * Author: Eregon (Benoit Daloze) * Status: Open * Priority: Normal * ruby -v: ruby 2.6.6p146 (2020-03-31 revision 67876) [x86_64-linux] * Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN ---------------------------------------- ``` [1] pry(main)> q = Queue.new => #<Thread::Queue:0x000056263683aee8> [2] pry(main)> q.freeze => #<Thread::Queue:0x000056263683aee8> [3] pry(main)> q << 1 => #<Thread::Queue:0x000056263683aee8> [4] pry(main)> q.pop => 1 [5] pry(main)> q.frozen? => true ``` Found by @ko1 in https://bugs.ruby-lang.org/issues/17100#note-28 I think it's a bug, since those are clear mutations. I guess old Thread::Queue implemented in Ruby did not have this bug. -- https://bugs.ruby-lang.org/

1 0

[ruby-core:114348] [Ruby master Feature#19832] Method#destructive?, UnboundMethod#destructive?
by sawa (Tsuyoshi Sawada) 14 Sep '23

14 Sep '23

Issue #19832 has been reported by sawa (Tsuyoshi Sawada). ---------------------------------------- Feature #19832: Method#destructive?, UnboundMethod#destructive? https://bugs.ruby-lang.org/issues/19832 * Author: sawa (Tsuyoshi Sawada) * Status: Open * Priority: Normal ---------------------------------------- I propose to add `destructive?` property to `Method` and `UnboundMethod` instances, which shall behave like: ```ruby String.instance_method(:<<).destructive? # => true String.instance_method(:+).destructive? # => false ``` One main purpose of using these classes is to inspect and make sure how a certain method behaves. Besides arity and owner, whether a method is destructive or not is one important piece of information, but currently, you cannot achieve that from `Method` or `UnboundMethod` instances. -- https://bugs.ruby-lang.org/

9 14

[ruby-core:114617] [Ruby master Misc#19860] Update license phrases to SPDX BSD-2-Clause
by nobu (Nobuyoshi Nakada) 14 Sep '23

14 Sep '23

Issue #19860 has been reported by nobu (Nobuyoshi Nakada). ---------------------------------------- Misc #19860: Update license phrases to SPDX BSD-2-Clause https://bugs.ruby-lang.org/issues/19860 * Author: nobu (Nobuyoshi Nakada) * Status: Assigned * Priority: Normal * Assignee: matz (Yukihiro Matsumoto) ---------------------------------------- There seem to be many variations on the "2-clause BSDL" these days. The current BSDL file seems to be the old style of the BSD 2 Clause "Simplified" license (BSD-2-Clause), so I think we should clarify this and update it to a more [recent version]. [recent version]: https://spdx.org/licenses/BSD-2-Clause.html -- https://bugs.ruby-lang.org/

2 2

[ruby-core:114740] [Ruby master Bug#17354] Module#const_source_location is misleading for constants awaiting autoload
by matz (Yukihiro Matsumoto) 14 Sep '23

14 Sep '23

Issue #17354 has been updated by matz (Yukihiro Matsumoto). Currently, it gives the place we called `autoload`. If you want the place where the loaded module defined (after autoload), you can explicitly reference the module before calling `const_source_location`. So I vote for keeping it as it is now. Matz. ---------------------------------------- Bug #17354: Module#const_source_location is misleading for constants awaiting autoload https://bugs.ruby-lang.org/issues/17354#change-104575 * Author: tomstuart (Tom Stuart) * Status: Open * Priority: Normal * ruby -v: ruby 2.7.2p137 (2020-10-01 revision 5445e04352) [x86_64-darwin20] * Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN ---------------------------------------- Feature #10771 added `Module#const_source_location` as a way to find the source location of a constant’s definition. Bug #16764 reported that it didn’t work correctly for autoloaded constants, instead giving the source location of the `autoload` call site. This was fixed in `v3_0_0_preview1` in commit:92730810 and backported to `v2_7_2` in commit:c65aae11. However, `#const_source_location` still returns the `autoload` call site for constants which have not yet been loaded: ``` % echo 'class Foo; end' > foo.rb % irb >> Module.const_defined?(:Foo) => false >> Module.const_source_location(:Foo) => nil >> autoload :Foo, './foo' => nil >> Module.const_defined?(:Foo) => true >> Module.const_source_location(:Foo) => ["(irb)", 3] >> Module.const_get(:Foo) => Foo >> Module.const_defined?(:Foo) => true >> Module.const_source_location(:Foo) => ["./foo.rb", 1] ``` This edge case is undocumented and surprising. It looks like a bug to the programmer who receives the `autoload` location instead of one of the documented return values of `#const_source_location` (`nil`, `[]`, or the definition’s source location). We could either: * change the behaviour of `#const_source_location` to return `[]` for constants awaiting autoload, which is consistent with the [return value of `Module#const_defined?`](https://docs.ruby-lang.org/en/2.7.0/Module.html#method-i-const_defined-3F) in this case (“if the constant is not present but there is an autoload for it, `true` is returned directly”), as well as the [return value of `#const_source_location`](https://docs.ruby-lang.org/en/2.7.0/Module.html#method-i-const_source_location) for other constants whose source location is unknown (“if the constant is found, but its source location can not be extracted (constant is defined in C code), empty array is returned”); or * document the current behaviour of `#const_source_location` to make it less surprising. I recommend the first option — although the current behaviour was recently specified in source:spec/ruby/core/module/const_source_location_spec.rb@6d059674#L209, it doesn’t seem intentional — but if that’s not feasible, simply documenting this edge case would also be an improvement. -- https://bugs.ruby-lang.org/

1 0