Limit Date.parse input length and make interruptible#6951
Merged
headius merged 5 commits intojruby:jruby-9.2.20.0from Nov 30, 2021
Merged
Limit Date.parse input length and make interruptible#6951headius merged 5 commits intojruby:jruby-9.2.20.0from
headius merged 5 commits intojruby:jruby-9.2.20.0from
Conversation
This prevents extremely long input from tying up the system for a long time.
This allows the running matches to be interrupted from Ruby, to avoid them running for an excessive amount of time.
Member
Author
|
The lack of testing on the 9.2 branch is a bit of a problem here. I fixed some failures in Date specs and they are now green.
|
Member
Author
|
@enebo If we intend this to be in 9.2.19.1 we can make a 9.2.19 branch and I will rebase this. Ditto for 9.3.1.1, and I will do a separate PR merging 9.2.19.1 over. |
headius
added a commit
that referenced
this pull request
Dec 1, 2021
commit 89e4ee5 Merge: da0c593 bbaf162 Author: Charles Oliver Nutter <headius@headius.com> Date: Wed Dec 1 10:16:14 2021 -0600 Merge pull request #6953 from headius/update_jnr Update JNR dependencies commit bbaf162 Author: Charles Oliver Nutter <headius@headius.com> Date: Wed Dec 1 10:12:26 2021 -0600 Update JNR dependencies * jffi to 1.3.8 * jnr-ffi to 2.2.10 * jnr-posix to 3.1.13 * jnr-enxio to 0.32.12 * jnr-unixsocket to 0.38.14 commit da0c593 Merge: caa52b8 a9f3b6b Author: Charles Oliver Nutter <headius@headius.com> Date: Wed Dec 1 09:39:08 2021 -0600 Merge branch 'jruby-9.2.20.0' into jruby-9.2 commit a9f3b6b Author: Charles Oliver Nutter <headius@headius.com> Date: Tue Nov 30 10:12:50 2021 -0600 Exclude broken parse test imported from json This test passes to DateTime.parse a keyword argument "create_additions" that is unknown to DateTime, but because CRuby only checks for the "limit" option it expects, this does not cause any error. In JRuby, where we still have this code in Ruby, the unknown keyword is explicitly rejected. We exclude this for now and will fix the bad test in json lib. commit 34a168e Merge: 1a32554 3e1a262 Author: Charles Oliver Nutter <headius@headius.com> Date: Tue Nov 30 08:44:32 2021 -0600 Merge pull request #6951 from headius/date_parse_limit Limit Date.parse input length and make interruptible commit 3e1a262 Author: Charles Oliver Nutter <headius@headius.com> Date: Tue Nov 30 03:12:00 2021 -0600 Match CRuby type conversions here commit aa5e8eb Author: Charles Oliver Nutter <headius@headius.com> Date: Tue Nov 30 02:21:21 2021 -0600 Add CRuby's test for Date parse limiting commit 4a6e676 Author: Charles Oliver Nutter <headius@headius.com> Date: Tue Nov 30 02:17:51 2021 -0600 Make all Date parser regexp calls interruptible This allows the running matches to be interrupted from Ruby, to avoid them running for an excessive amount of time. commit af68c6c Author: Charles Oliver Nutter <headius@headius.com> Date: Tue Nov 30 02:16:58 2021 -0600 Add limit option to Date parse methods This prevents extremely long input from tying up the system for a long time.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Excessively long input to the various Date parsing methods can cause the calling thread to run for a long time, perhaps tying it up indefinitely. This PR incorporates to changes similar to those in ruby/date@3959acc.
limitoption to the affected Date parse methods (see commit above for the list).The interruptibility patch here begs a question: should we just flip the switch and make all regexp matches interruptible all the time? We opted not to do so in jruby/joni#6, but have since reduced the overhead of the interrupt check. It seems like CRuby has made all regexp interruptible all the time, and perhaps we should follow suit. This would eliminate the need for that part of the PR.
A partial workaround for users is to enable interruptible regexp matches with the JRuby property "regexp.interruptible=true" in .jruby_opts, as
-Xregexp.interruptibleon the command line, or by setting the JVM propertyjruby.regexp.interruptible=true, and then wrap the Date parsing methods in Timeout (or perhaps your framework will do this for you).A more complete workaround would be to apply the Ruby changes here to your local JRuby environment. This will not make the matches interruptible, but the default 128 length limit will prevent such large strings from being parsed.
cc @jordansissel, @rsim