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

[ruby-core:113107] [Ruby master Bug#19576] Backport request: Gemfile.lock resolving is broken with bundler shipped with Ruby 3.1.4

by jprokop (Jarek Prokop)

Issue #19576 has been reported by jprokop (Jarek Prokop). ---------------------------------------- Bug #19576: Backport request: Gemfile.lock resolving is broken with bundler shipped with Ruby 3.1.4 https://bugs.ruby-lang.org/issues/19576 * Author: jprokop (Jarek Prokop) * Status: Open * Priority: Normal * ruby -v: ruby 3.1.3p185 (2022-11-24 revision 1a6b16756e) [x86_64-linux] * Backport: 2.7: UNKNOWN, 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN ---------------------------------------- This is a backport request for bundler, that regressed in 2.3.36 in a specific situation. Newer and older bundler versions that ship with Ruby are not problematic, only the version that ships with Ruby version >= 3.1.3. A few weeks ago, we discovered a bug in resolving in bundler shipped with Ruby 3.1.3 and 3.1.4: Bundler version: ``` $ bundler --version Bundler version 2.3.26 ``` Affected rubies `ruby -v`: First: ``` $ ruby -v ruby 3.1.4p223 (2023-03-30 revision 957bb7cb81) [x86_64-linux] ``` Second: ``` $ruby -v ruby 3.1.3p185 (2022-11-24 revision 1a6b16756e) [x86_64-linux] ``` Initial bug report with reproducer and more in-depth description can be found here: https://github.com/sclorg/s2i-ruby-container/issues/469 Using the following Gemfile for a rails app: https://github.com/sclorg/rails-ex/blob/67b7a61eae9efa1088ff3f634ae316e1022… bundler locks up in trying to resolve Nokogiri for Ruby 3.1, but keeps failing because it keeps using incompatible built binary gem instead of falling back to installing and building the binary extension of Nokogiri locally. We craft this Gemfile to be usable from Ruby 2.5 up to Ruby 3.1, as the app is used mainly for testing. I have created a patch to fix the situation, see the attached files. There are 2 of them, one contains the fix and the other one contains the test from the rubygems repo PR#6225. full commit available here: https://src.fedoraproject.org/fork/jackorp/rpms/ruby/c/5ef600a8f40b76de5636… The patches are created from the following upstream changes in bundler: https://github.com/rubygems/rubygems/pull/6225 and adapted: https://github.com/rubygems/rubygems/commit/7b64c64262a7a980c0eb23b96ea56cf… for the PR#6225. With the fix applied I no longer have issues doing `bundle install` with our Gemfile.lock. ---Files-------------------------------- rubygem-bundler-2.3.26-Tests-from-bundler-PR-6225.patch (1.82 KB) rubygem-bundler-2.3.26-Provide-fix-for-bundler-Gemfile-resolving-regression.patch (5.21 KB) -- https://bugs.ruby-lang.org/

6 months

5
7
0 0

[ruby-core:111953] [Ruby master Bug#19362] #dup on Proc doesn't call initialize_dup

by zverok (Victor Shepelev)

Issue #19362 has been reported by zverok (Victor Shepelev). ---------------------------------------- Bug #19362: #dup on Proc doesn't call initialize_dup https://bugs.ruby-lang.org/issues/19362 * Author: zverok (Victor Shepelev) * Status: Open * Priority: Normal * Backport: 2.7: UNKNOWN, 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN ---------------------------------------- In #17545, `#dup` had changed to create an instance of the subclass. It, though, doesn't invoke `initialize_dup` of the subclass, unlike other standard classes. ```ruby class MyAry < Array def initialize_dup(...) p(self.class, ...) super end end class MyString < String def initialize_dup(...) p(self.class, ...) super end end class MyProc < Proc def initialize_dup(...) p(self.class, ...) super end end MyString.new('test').dup # prints MyString, "test" MyAry.new(['test']).dup # prints MyAry, ["test"] MyProc.new { 'test' }.dup # doesn't print anything ``` This makes the change in #17545 useless: while inheriting from core classes is indeed marginal, one of author's intention might be carrying additional information with the Proc instance, and bypassing `#initialize_dup` makes it impossible to maintain this information. It seems that actually `#initialize_dup` is also invoked on the core classes themselves, but ignored on `Proc`. ```ruby class Array def initialize_dup(...) p(self.class, ...) super end end class String def initialize_dup(...) p(self.class, ...) super end end class Proc def initialize_dup(...) p(self.class, ...) super end end 'test'.dup # prints String, "test" ['test'].dup # prints Array, ["test"] Proc.new { 'test' }.dup # doesn't print anything ``` Which is an even more marginal problem but still an inconsistency. -- https://bugs.ruby-lang.org/

6 months, 2 weeks

3
7
0 0

[ruby-core:112944] [Ruby master Feature#19541] Proposal: Generate frame unwinding info for YJIT code

by kjtsanaktsidis (KJ Tsanaktsidis)

Issue #19541 has been reported by kjtsanaktsidis (KJ Tsanaktsidis). ---------------------------------------- Feature #19541: Proposal: Generate frame unwinding info for YJIT code https://bugs.ruby-lang.org/issues/19541 * Author: kjtsanaktsidis (KJ Tsanaktsidis) * Status: Open * Priority: Normal ---------------------------------------- ## What is being propsed? Currently, Ruby crashes with yjit generated code on the stack, `rb_print_backtrace()` is unable to actually show any frames underneath the yjit code. For example, if you send SIGSEGV to a Ruby process running yjit, this is what you see: ``` /ruby/miniruby(rb_print_backtrace+0xc) [0xaaaad0276884] /ruby/vm_dump.c:785 /ruby/miniruby(rb_vm_bugreport) /ruby/vm_dump.c:1093 /ruby/miniruby(rb_bug_for_fatal_signal+0xd0) [0xaaaad0075580] /ruby/error.c:813 /ruby/miniruby(sigsegv+0x5c) [0xaaaad01bedac] /ruby/signal.c:919 linux-vdso.so.1(__kernel_rt_sigreturn+0x0) [0xffff91a3e8bc] /ruby/miniruby(map<(usize, yjit::backend::ir::Insn), (usize, yjit::backend::ir::Insn), yjit::backend::ir::{impl#17}::next_mapped::{closure_env#0}>+0x8c) [0xaaaad03b8b00] /rustc/897e37553bba8b42751c67658967889d11ecd120/library/core/src/option.rs:929 /ruby/miniruby(next_mapped+0x3c) [0xaaaad0291dc0] src/backend/ir.rs:1225 /ruby/miniruby(arm64_split+0x114) [0xaaaad0287744] src/backend/arm64/mod.rs:359 /ruby/miniruby(compile_with_regs+0x80) [0xaaaad028bf84] src/backend/arm64/mod.rs:1106 /ruby/miniruby(compile+0xc4) [0xaaaad0291ae0] src/backend/ir.rs:1158 /ruby/miniruby(gen_single_block+0xe44) [0xaaaad02b1f88] src/codegen.rs:854 /ruby/miniruby(gen_block_series_body+0x9c) [0xaaaad03b0250] src/core.rs:1698 /ruby/miniruby(gen_block_series+0x50) [0xaaaad03b0100] src/core.rs:1676 /ruby/miniruby(branch_stub_hit_body+0x80c) [0xaaaad03b1f68] src/core.rs:2021 /ruby/miniruby({closure#0}+0x28) [0xaaaad02eb86c] src/core.rs:1924 /ruby/miniruby(do_call<yjit::core::branch_stub_hit::{closure_env#0}, *const u8>+0x98) [0xaaaad035ba3c] /rustc/897e37553bba8b42751c67658967889d11ecd120/library/std/src/panicking.rs:492 [0xaaaad035c9b4] ``` (n.b. - I compiled Ruby with `-fasynchronous-unwind-tables –rdynamic –g` in cflags to make sure gcc generates appropriate unwind info & keeps the symbol tables). Likewise, if you attach gdb to a Ruby process with yjit enabled, gdb can't show thread backtraces through yjit-generated code either. My proposal is that YJIT generate sufficient unwinding and debug information on all platforms to allow both `rb_print_backtrace()` and the platform's debugger (gdb/lldb/WinDbg) to show: * Full stack traces all the way back to `main`. That is, it should be possible to see frames _underneath_ `[0xaaaad035c9b4]` from the backtrace above. * Names for the dynamically generated yjit blocks (e.g. instead of `[0xaaaad035c9b4]`, we should see something like `yjit$$name_of_ruby_method`, where `name_of_ruby_method` is the `label` for the iseq this is JIT'd code for). ## Motivation I have a few motivations for wanting this. Firstly, I feel this functionality is independently useful. When Ruby crashes, the more information we can get, the more likely we are to find the root cause. Likewise, the same principle applies to debugging with gdb - you can get a fuller understanding of what the process is doing if you see the whole stack. I have often found attaching gdb to the Ruby interpreter helps in understanding problems in Ruby code or C extensions and is something I do relatively frequently; yjit breaking that will definitely be inconvenient for me! ## Implementation I have a draft implementation here on how I'd implement this: . It's currently missing tests & platform support (it only works on Linux aarch64). Also, it implements unwind info generation, so unwinding can work _through_ yjit code, but it does not currently emit symbols to give _names_ to those yjit frames. My PR contains a document which explains how the Linux interfaces for registering unwind info for JIT'd code work, so I won't duplicate that information here. The biggest implementation question I had is around the use of Rust crates. Currently, I prototyped my implementation using the gimli & object crates, for generating DWARF info and ELF binaries. However, the yjit build does purposefully does not use cargo & external crates for release builds. There are a few different ways we could go here: * Don't use the gimli & object crates; instead, re-implement all debug info & object file generation code in yjit. * Don't use the crates; instead, link againt C libraries to provide this functionality & call them from Rust (perhaps some combination of libelf, libdw, libbfd, or llvm might do what we need) * Use cargo after all for the release build & download the crates at build-time * Use cargo for the release build, but vendor everything, so the build doesn't need to download anything * Only make unwind info generation available in dev mode where cargo is used, and so mark the gimli/object dependencies as optional in Cargo.toml. We'd need to decide on one of these approaches for this proposal to work. I don't really have a strong sense of the pros/cons of each. (Side note - my PR actually depends on a _fork_ of gimli - I've been discussing adding the needed interfaces upstream here: https://github.com/gimli-rs/gimli/issues/648). ## Benchmarks I ran the yit-bench suite on my branch and compared it to Ruby master: * My branch: https://gist.github.com/KJTsanaktsidis/5741a9f64e5cd75cdf5fedd846091a4f * Ruby master: https://gist.github.com/KJTsanaktsidis/592d3ebcf98f6745dfa3efbd30a25acf This is a (very simple) comparison: ``` -------------- ------------ ------------ --------------- bench yjit (ms) branch (ms) branch/yjit (%) activerecord 97.5 98.5 101.03% hexapdf 2415.3 2458.2 101.78% liquid-c 61.9 63.1 101.94% liquid-render 135.3 135.0 99.78% mail 104.6 105.5 100.86% psych-load 1887.1 1922.0 101.85% railsbench 1544.4 1556.0 100.75% ruby-lsp 88.4 89.5 101.24% sequel 147.5 151.1 102.44% binarytrees 303 305.6 100.86% chunky_png 1075.8 1079.4 100.33% erubi 392.9 392.3 99.85% erubi_rails 14.7 14.7 100.00% etanni 792.3 791.4 99.89% fannkuchredux 3815.9 3813.6 99.94% lee 1030.2 1039.2 100.87% nbody 49.2 49.3 100.20% optcarrot 4142 4143.3 100.03% ruby-json 2860.7 2874.0 100.46% rubykon 7906.6 7904.2 99.97% 30k_ifelse 348.7 345.4 99.05% 30k_methods 828.6 831.8 100.39% cfunc_itself 28.8 28.9 100.35% fib 34.4 34.5 100.29% getivar 115.5 109.7 94.98% keyword_args 37.7 38.0 100.80% respond_to 26 26.1 100.38% setivar 33.8 33.5 99.11% setivar_object 208.7 194.3 93.10% str_concat 52.6 52.2 99.24% throw 23.8 24.1 101.26% -------------- ------------ ------------ --------------- ``` It seems like the performance impact of generating and registering the debug info is marginal. -- https://bugs.ruby-lang.org/

6 months, 3 weeks

7
12
0 0

[ruby-core:112326] [Ruby master Feature#19430] Contribution wanted: DNS lookup by c-ares library

by mame (Yusuke Endoh)

Issue #19430 has been reported by mame (Yusuke Endoh). ---------------------------------------- Feature #19430: Contribution wanted: DNS lookup by c-ares library https://bugs.ruby-lang.org/issues/19430 * Author: mame (Yusuke Endoh) * Status: Open * Priority: Normal ---------------------------------------- ## Problem At the present time, Ruby uses `getaddrinfo(3)` to resolve names. Because this function is synchronous, we cannot interrupt the thread performing name resolution until the DNS server returns a response. We can see this behavior by setting blackhole.webpagetest.org (72.66.115.13) as a DNS server, which swallows all packets, and resolving any name: ``` # cat /etc/resolv.conf nameserver 72.66.115.13 # ./local/bin/ruby -rsocket -e 'Addrinfo.getaddrinfo("www.ruby-lang.org", 80)' ^C^C^C^C ``` As we see, Ctrl+C does not stop ruby. The current workaround that users can take is to do name resolution in a Ruby thread. ```ruby Thread.new { Addrinfo.getaddrinfo("www.ruby-lang.org", 80) }.value ``` The thread that calls this code is interruptible. (Note that the newly created thread itself will be stuck until the DNS lookup exceeds the time out.) ## Proposal We can solve this problem by using c-ares, which is an asynchronous name resolver, as a backend of `Addrinfo.getaddrinfo`, etc. (@sorah told me about this library, thanks!) https://c-ares.org/ I have created a PoC patch. https://github.com/mame/ruby/commit/547806146993bbc25984011d423dcc0f913b211c By applying this patch, we can interrupt `Addrinfo.getaddrinfo` by Ctrl+C. ``` # cat /etc/resolv.conf nameserver 72.66.115.13 # ./local/bin/ruby -rsocket -e 'Addrinfo.getaddrinfo("www.ruby-lang.org", 80)' ^C-e:1:in `getaddrinfo': Interrupt from -e:1:in `<main>' ``` ## Discussion ### About c-ares According to the site of c-ares, some major tools including libcurl, Wireshark, and Apache Arrow are already using c-ares. In the language interpreter, node.js seems to be using c-ares. I am honestly not sure about the compatibility of c-ares with `getaddrinfo(3)`. I guess there is no major incompatibility because I have not experienced any name resolution problem of curl. @akr (who is the author and maintainer of Ruby's socket library) suggested to check if OS-specific name resolution, e.g., WINS on Windows, NIS on Solaris, etc., is supported. He also said that it may be acceptable even if they are not supported. Whether to bundle c-ares source code with ruby would require further discussion. If this proposal is accepted, then c-ares will become a de facto essential dependency for practical use, like gmp, in my opinion. Incidentally, node.js bundles c-ares: https://github.com/nodejs/node/tree/main/deps/cares ### Alternative approaches Recent glibc provides `getaddrinfo_a(3)` which performs asynchronous name resolution. However, this function has a fatal problem of being incompatible with `fork(2)`, which is heavily used in the Ruby ecosystem. In fact, the attempt to use `getaddrinfo_a(3)` (#17134) has been revert because it fails rails tests. (#17220) Another alternative is to have a worker pthread inside Ruby that calls getaddrinfo(3). Instead of calling getaddrinfo(3) directly, `Addrinfo.getaddrinfo` would ask the worker to resolve a name and wait for a response. This method should be able to implement cancellation. (Simply put, this means reimplementation of getaddrinfo_a(3) on our own, taking into account of `fork(2).) This has the advantages: not adding dependencies on external libraries and not having compatibility issues with `getaddrinfo(3)`. However, it is considerably more difficult to implement and maintain. An internal pthread may have a non-trivial impact on the execution efficiency and memory usage. Also, we may need to implement a mechanism to dynamically change the number of workers depending on the load. It would be ideal if we could try and evaluate both approaches. But my current impression is that using c-ares is the quickest and best compromise. ## Contribution wanted I have made it up to the PoC, but don't have much time to complete this. @naruse suggested me to create a ticket asking for contributions. Is anyone interested in this? * This patch changes `rsock_getaddrinfo` to accept a timeout argument. There are several places where Qnil is passed as a timeout (where I add `// TODO` in the PoC). We need to consider what timeout we should pass. * This cares only `getaddrinfo`, but we also need to care `getnameinfo` (and something else if any). There may be some issues I'm not aware of. * I have not yet tested this PoC seriously. It would be great if we could evaluate it with some real apps. Also, it would be great to hear from someone who knows more about c-ares. -- https://bugs.ruby-lang.org/

6 months, 3 weeks

6
15
0 0

[ruby-core:111526] [Ruby master Bug#19288] Ractor JSON parsing significantly slower than linear parsing

by maciej.mensfeld (Maciej Mensfeld)

Issue #19288 has been reported by maciej.mensfeld (Maciej Mensfeld). ---------------------------------------- Bug #19288: Ractor JSON parsing significantly slower than linear parsing https://bugs.ruby-lang.org/issues/19288 * Author: maciej.mensfeld (Maciej Mensfeld) * Status: Open * Priority: Normal * ruby -v: ruby 3.2.0 (2022-12-25 revision a528908271) [x86_64-linux] * Backport: 2.7: UNKNOWN, 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN ---------------------------------------- a simple benchmark: ```ruby require 'json' require 'benchmark' CONCURRENT = 5 RACTORS = true ELEMENTS = 100_000 data = CONCURRENT.times.map do ELEMENTS.times.map do { rand => rand, rand => rand, rand => rand, rand => rand }.to_json end end ractors = CONCURRENT.times.map do Ractor.new do Ractor.receive.each { JSON.parse(_1) } end end result = Benchmark.measure do if RACTORS CONCURRENT.times do |i| ractors[i].send(data[i], move: false) end ractors.each(&:take) else # Linear without any threads data.each do |piece| piece.each { JSON.parse(_1) } end end end puts result ``` Gives following results on my 8 core machine: ```shell # without ractors: 2.731748 0.003993 2.735741 ( 2.736349) # with ractors 12.580452 5.089802 17.670254 ( 5.209755) ``` I would expect Ractors not to be two times slower on the CPU intense work. -- https://bugs.ruby-lang.org/

7 months, 1 week

6
16
0 0

[ruby-core:114038] [Ruby master Bug#19749] Confirm correct behaviour when attaching private method with `#define_method`

by itarato (Peter Arato)

Issue #19749 has been reported by itarato (Peter Arato). ---------------------------------------- Bug #19749: Confirm correct behaviour when attaching private method with `#define_method` https://bugs.ruby-lang.org/issues/19749 * Author: itarato (Peter Arato) * Status: Open * Priority: Normal * ruby -v: 3.3.0 * Backport: 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN ---------------------------------------- This issue is a special case of https://bugs.ruby-lang.org/issues/19745: Should dynamically added private methods via `.singleton_class.send(:define_method,...` be accessible publicly? See the following example? ```ruby private def bar; end foo = Object.new foo.singleton_class.send(:define_method, :bar, method(:bar)) foo.bar # No error. ``` The script above runs fine on latest Ruby 3.3. Is this correct to ignore the fact that the added method (method(:bar)) is private? This came up during a TruffleRuby investigation (https://github.com/oracle/truffleruby/issues/3134) where the result for the same script is: `private method 'bar' called for #<Object:0xc8> (NoMethodError)` -- https://bugs.ruby-lang.org/

7 months, 2 weeks

5
13
0 0

[ruby-core:111450] [Ruby master Bug#19268] Mingw64 Build Failure

by cfis (Charlie Savage)

Issue #19268 has been reported by cfis (Charlie Savage). ---------------------------------------- Bug #19268: Mingw64 Build Failure https://bugs.ruby-lang.org/issues/19268 * Author: cfis (Charlie Savage) * Status: Open * Priority: Normal * ruby -v: ruby 3.1.3p185 (2022-11-24 revision 1a6b16756e) [x64-mingw-ucrt] * Backport: 2.7: UNKNOWN, 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN ---------------------------------------- For both Ruby 3.1.3 and Ruby 3.2.0, building on msys2/ucrt64 fails: ``` c linking miniruby.exe /usr/bin/sh: -c: line 1: syntax error near unexpected token `(' /usr/bin/sh: -c: line 1: `/usr/local/ruby/bin/ruby --disable=gems -n -e BEGIN{version=ARGV.shift;mis=ARGV.dup} -e END{abort "UNICODE version mismatch: #{mis}" unless mis.empty?} -e (mis.delete(ARGF.path); ARGF.close) if /ONIG_UNICODE_VERSION_STRING +"#{Regexp.quote(version)}"/o 15.0.0 ./enc/unicode/15.0.0/casefold.h ./enc/unicode/15.0.0/name2ctype.h' make: *** [uncommon.mk:878: .rbconfig.time] Error 2 ``` The fix that works for me is changing: ``` c -e '(mis.delete(ARGF.path); ARGF.close) if /ONIG_UNICODE_VERSION_STRING +"#{Regexp.quote(version)}"/o' \ ``` To ``` c -e "(mis.delete(ARGF.path); ARGF.close) if /ONIG_UNICODE_VERSION_STRING +\"#{Regexp.quote(version)}\"/o" \ ``` ``` -- https://bugs.ruby-lang.org/

7 months, 2 weeks

1
1
0 0

[ruby-core:113381] [Ruby master Bug#19624] Backticks - IO object leakage

by pineman

Issue #19624 has been reported by pineman (João Pinheiro). ---------------------------------------- Bug #19624: Backticks - IO object leakage https://bugs.ruby-lang.org/issues/19624 * Author: pineman (João Pinheiro) * Status: Open * Priority: Normal * ruby -v: ruby 3.2.2 (2023-03-30 revision e51014f9c0) [arm64-darwin22] * Backport: 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN ---------------------------------------- Hi, This code works on ruby 3.0.6: ```ruby `echo` ObjectSpace.each_object(IO) do |io| if ![STDIN, STDOUT, STDERR].include?(io) io.close end end ``` but raises `IOError` on 3.2.2: ``` minimal-repro-case.rb:8:in `close': uninitialized stream (IOError) ``` I found it started failing on ruby 3.1.0 and after, on macOS and Linux. This code is useful for closing unneeded IO objects in forked processes. It looks like backticks is 'leaking' IO objects, waiting for GC, and it didn't used to in 3.1.0. In ruby 3.1.0, inside `rb_f_backquote`, `rb_gc_force_recycle` was removed in favor of `RB_GC_GUARD`. I wonder if this has something to do with the problem. Is this code incorrect since ruby 3.1.0 or is it a bug in ruby? Thanks. ---Files-------------------------------- minimal-repro-case.rb (109 Bytes) -- https://bugs.ruby-lang.org/

7 months, 3 weeks

4
9
0 0

[ruby-core:113926] [Ruby master Bug#19735] Add support for UUID version 7

by nevans (Nicholas Evans)

Issue #19735 has been reported by nevans (Nicholas Evans). ---------------------------------------- Bug #19735: Add support for UUID version 7 https://bugs.ruby-lang.org/issues/19735 * Author: nevans (Nicholas Evans) * Status: Open * Priority: Normal * Backport: 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN ---------------------------------------- Although the specification for UUIDv7 is still in draft, the UUIDv7 algorithm has been stable as the RFC progresses to completion. Version 7 UUIDs can be very useful, because they are lexographically sortable, which can improve e.g: database index locality. See section 6.10 of the draft specification for further explanation: https://www.ietf.org/archive/id/draft-ietf-uuidrev-rfc4122bis-06.html ```ruby require 'random/formatter' Random.uuid_v7 # => "0188ca50-fcc0-7881-b5c5-6d55cd8fc373" Random.uuid_v7 # => "0188ca51-0069-7304-be2e-0c3cd908789b" Random.uuid_v7 # => "0188ca51-04aa-7b57-a6ec-c49573412a9d" Random.uuid_v7 # => "0188ca51-0853-7979-ae37-485460e9f4f1" # or prng = Random.new prng.uuid_v7 # => "0188ca51-5e72-7950-a11d-def7ff977c98" ``` PR here: https://github.com/ruby/ruby/pull/7953 -- https://bugs.ruby-lang.org/

7 months, 3 weeks

3
4
0 0

[ruby-core:113819] [Ruby master Feature#19720] Warning for non-linear Regexps

by Eregon (Benoit Daloze)

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/

7 months, 3 weeks

5
10
0 0

2024

2023

2022

ruby-core June 2023