Patch #32527
closedFix ruby 2.7 warning: given argument is nil; this will raise a TypeError in the next release
0%
Description
the `request.env['HTTP_USER_AGENT']` may `nil`.
e.g. `curl -A '' http://localhost:3000`
Quote from ruby 2.7 preview3 NEWS file:
Regexp#match and Regexp#match? with a nil argument are deprecated and will raise a TypeError in the next release. [Feature \#13083]
https://github.com/ruby/ruby/blob/v2_7_0_preview3/NEWS#core-classes-updates-outstanding-ones-only
Files
Related issues
Updated by Pavel Rosický almost 5 years ago
There're already tons of deprecation warnings on Ruby 2.7. The patch looks good, but you have to use Regex#match? instead of String#match?, because Redmine still supports Ruby 2.3.
Updated by Go MAEDA almost 5 years ago
- Related to Feature #31500: Ruby 2.7 support added
Updated by Go MAEDA almost 5 years ago
Pavel Rosický wrote:
The patch looks good, but you have to use Regex#match? instead of String#match?, because Redmine still supports Ruby 2.3.
$ ruby -v ruby 2.3.8p459 (2018-10-18 revision 65136) [x86_64-darwin18] $ ruby test/functional/attachments_controller_test.rb Run options: --seed 63689 # Running: ..................E Error: AttachmentsControllerTest#test_download_text_file: NoMethodError: undefined method `match?' for "Rails Testing":String Did you mean? match app/controllers/application_controller.rb:647:in `filename_for_content_disposition' app/controllers/attachments_controller.rb:75:in `download' lib/redmine/sudo_mode.rb:64:in `sudo_mode' test/functional/attachments_controller_test.rb:302:in `test_download_text_file' bin/rails test test/functional/attachments_controller_test.rb:301
Updated by Seiei Miyagi almost 5 years ago
I will update the patch. By the way, in ruby 2.3, both “String # match?” and “Regexp # match?” do not exist.
https://docs.ruby-lang.org/en/2.3.0/Regexp.html
root@0554350dda7e:/all-ruby# bin/ruby-2.3.8 -ve '//.match?("")' ruby 2.3.8p459 (2018-10-18 revision 65136) [x86_64-linux] -e:1:in `<main>': undefined method `match?' for //:Regexp (NoMethodError) Did you mean? match
I don't know why the build passed on ruby-2.3
https://www.redmine.org/builds/
Updated by Go MAEDA almost 5 years ago
Seiei Miyagi wrote:
I will update the patch. By the way, in ruby 2.3, both “String # match?” and “Regexp # match?” do not exist.
https://docs.ruby-lang.org/en/2.3.0/Regexp.html[...]
I don't know why the build passed on ruby-2.3
https://www.redmine.org/builds/
Regexp#match?
is defined by ActiveSupport for older versions of Ruby. Please see #28940 for details.
Updated by Seiei Miyagi almost 5 years ago
- File 0001-Fix-ruby-2.7-warning-given-argument-is-nil-this-will.patch 0001-Fix-ruby-2.7-warning-given-argument-is-nil-this-will.patch added
Go MAEDA wrote:
Seiei Miyagi wrote:
I will update the patch. By the way, in ruby 2.3, both “String # match?” and “Regexp # match?” do not exist.
https://docs.ruby-lang.org/en/2.3.0/Regexp.html[...]
I don't know why the build passed on ruby-2.3
https://www.redmine.org/builds/
Regexp#match?
is defined by ActiveSupport for older versions of Ruby. Please see #28940 for details.
I fully understand. thank you!
Updated the patch.
Updated by Go MAEDA almost 5 years ago
Seiei Miyagi wrote:
Updated the patch.
What do you think about the following code? It is simpler and very slightly efficient.
Index: app/controllers/application_controller.rb
===================================================================
--- app/controllers/application_controller.rb (リビジョン 19319)
+++ app/controllers/application_controller.rb (作業コピー)
@@ -644,7 +644,7 @@
# Returns a string that can be used as filename value in Content-Disposition header
def filename_for_content_disposition(name)
- %r{(MSIE|Trident|Edge)}.match?(request.env['HTTP_USER_AGENT']) ? ERB::Util.url_encode(name) : name
+ %r{(MSIE|Trident|Edge)}.match?(request.env['HTTP_USER_AGENT'].to_s) ? ERB::Util.url_encode(name) : name
end
def api_request?
Updated by Seiei Miyagi almost 5 years ago
Go MAEDA wrote:
Seiei Miyagi wrote:
Updated the patch.
What do you think about the following code? It is simpler and very slightly efficient.
[...]
Looks great!
My code is calling request.env[] twice, thus your code is better.
Updated by Go MAEDA almost 5 years ago
- Category set to Ruby support
- Status changed from New to Closed
Committed the patch in #32527#note-7 as a part of #31500.
Thank you for reporting this issue.
Updated by Pavel Rosický almost 5 years ago
fyi, the change will be probably reverted in the final release, see https://bugs.ruby-lang.org/issues/13083