Patch #40802: Support builder 3.3.0 - Redmine

Actions

Copy link

Patch #40802

closed

Support builder 3.3.0

Added by Pavel Rosický 7 months ago. Updated 6 months ago.

Status:

Closed

Priority:

Normal

Assignee:

Go MAEDA

Category:

Gems support

Target version:

5.0.9

Start date:

Due date:

% Done:

Estimated time:

Description

== Version 3.3.0

Use BasicObject instead of BlankSlate for better compatibility with modern rubies. https://github.com/rails/builder/pull/15

unfortunately, this change breaks Redmine's API. LoadError: cannot load such file -- blankslate

the following patch removes that dependency in favor of BasicObject

diff --git a/lib/redmine/views/builders/json.rb b/lib/redmine/views/builders/json.rb
index 47ce0cc9b..011c16e58 100644
--- a/lib/redmine/views/builders/json.rb
+++ b/lib/redmine/views/builders/json.rb
@@ -28,7 +28,7 @@ module Redmine
         def initialize(request, response)
           super
           callback = request.params[:callback] || request.params[:jsonp]
-          if callback && Setting.jsonp_enabled?
+          if callback && ::Setting.jsonp_enabled?
             self.jsonp = callback.to_s.gsub(/[^a-zA-Z0-9_.]/, '')
           end
         end
diff --git a/lib/redmine/views/builders/structure.rb b/lib/redmine/views/builders/structure.rb
index f4524143f..ab1e3e411 100644
--- a/lib/redmine/views/builders/structure.rb
+++ b/lib/redmine/views/builders/structure.rb
@@ -17,12 +17,10 @@
 # along with this program; if not, write to the Free Software
 # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.

-require 'blankslate'
-
 module Redmine
   module Views
     module Builders
-      class Structure < BlankSlate
+      class Structure < BasicObject
         def initialize(request, response)
           @struct = [{}]
           @request = request
@@ -38,7 +36,7 @@ module Redmine
         end

         def encode_value(value)
-          if value.is_a?(Time)
+          if value.is_a?(::Time)
             # Rails uses a global setting to format JSON times
             # Don't rely on it for the API as it could have been changed
             value.xmlschema(0)
@@ -49,15 +47,15 @@ module Redmine

         def method_missing(sym, *args, &block)
           if args.count > 0
-            if args.first.is_a?(Hash)
-              if @struct.last.is_a?(Array)
+            if args.first.is_a?(::Hash)
+              if @struct.last.is_a?(::Array)
                 @struct.last << args.first unless block
               else
                 @struct.last[sym] = args.first
               end
             else
               value = encode_value(args.first)
-              if @struct.last.is_a?(Array)
+              if @struct.last.is_a?(::Array)
                 if args.size == 1 && !block
                   @struct.last << value
                 else
@@ -69,13 +67,13 @@ module Redmine
             end
           end
           if block
-            @struct << (args.first.is_a?(Hash) ? args.first : {})
+            @struct << (args.first.is_a?(::Hash) ? args.first : {})
             yield(self)
             ret = @struct.pop
-            if @struct.last.is_a?(Array)
+            if @struct.last.is_a?(::Array)
               @struct.last << ret
             else
-              if @struct.last.has_key?(sym) && @struct.last[sym].is_a?(Hash)
+              if @struct.last.has_key?(sym) && @struct.last[sym].is_a?(::Hash)
                 @struct.last[sym].merge! ret
               else
                 @struct.last[sym] = ret

Related issues

Actions

Copy link

Updated by Go MAEDA 7 months ago

Target version set to 5.0.9

Setting the target version to 5.0.9.

Actions

Copy link

Updated by salman mp 7 months ago

Not only redmine APIs; It also breaks Redmine web app (redmine v4.2.11).
Thanks for given patch.

Actions

Copy link

Updated by Go MAEDA 7 months ago

Subject changed from support builder 3.3.0 to Support builder 3.3.0
Status changed from New to Resolved
Assignee set to Go MAEDA

Committed the patch in r22863. Thank you.

Actions

Copy link

Updated by Brice Beaumesnil 7 months ago

Hello,

This problem is on 5.1-stable branche too

Thanks.

Actions

Copy link

Updated by Go MAEDA 7 months ago

In the 5.0-stable branch, the test hangs if the patch is applied.

It can be reproduced by running ruby test/integration/api_test/memberships_test.rb. In the test/log, SystemStackError was recorded.

SystemStackError (stack level too deep):

lib/redmine/views/builders/structure.rb:69:in `method_missing'
lib/redmine/views/builders/structure.rb:69:in `method_missing'
lib/redmine/views/builders/structure.rb:69:in `method_missing'
lib/redmine/views/builders/structure.rb:69:in `method_missing'
lib/redmine/views/builders/structure.rb:69:in `method_missing'
.
.
.
lib/redmine/views/builders/structure.rb:69:in `method_missing'
lib/redmine/views/builders/structure.rb:69:in `method_missing'
lib/redmine/views/builders/structure.rb:69:in `method_missing'
lib/redmine/views/builders/structure.rb:69:in `method_missing'
lib/redmine/views/builders/structure.rb:69:in `method_missing'
app/views/members/index.api.rsb:3
app/views/members/index.api.rsb:2:in `each'
app/views/members/index.api.rsb:2
lib/redmine/views/builders/structure.rb:32:in `array'
app/views/members/index.api.rsb:1
lib/redmine/views/builders.rb:38:in `for'
app/views/members/index.api.rsb:1
lib/redmine/sudo_mode.rb:61:in `sudo_mode'
test/integration/api_test/memberships_test.rb:37:in `block in <class:MembershipsTest>'

For 5.1-stable and 5.0-stable, is it better to pin the version of builder to 3.2 than merge the patch?

Actions

Copy link

Updated by Brice Beaumesnil 7 months ago

Hello

In this patch the line "require 'blankslate'" can be deleted

Thanks.

Actions

Copy link

Updated by Brice Beaumesnil 7 months ago

Go MAEDA wrote in #note-5:

In the 5.0-stable branch, the test hangs if the patch is applied.

It can be reproduced by running ruby test/integration/api_test/memberships_test.rb. In the test/log, SystemStackError was recorded.

[...]

For 5.1-stable and 5.0-stable, is it better to pin the version of builder to 3.2 than merge the patch?

I try this test with success on 5.1-stable branch

Finished in 2.357875s, 5.5134 runs/s, 19.9332 assertions/s.
13 runs, 47 assertions, 0 failures, 0 errors, 0 skips

Actions

Copy link

Updated by Pavel Rosický 7 months ago

@go_maeda the BasicObject doesn't know the method block_given? see https://ruby-doc.org/core-2.7.1/BasicObject.html so it falls under method_missing, causing an infinite loop...

this can be fixed by replacing it with "::Kernel.block_given?" or a simple "block"

Actions

Copy link

Updated by Go MAEDA 7 months ago

Status changed from Resolved to Closed

Pavel Rosický wrote in #note-8:

@go_maeda the BasicObject doesn't know the method block_given? see https://ruby-doc.org/core-2.7.1/BasicObject.html so it falls under method_missing, causing an infinite loop...

this can be fixed by replacing it with "::Kernel.block_given?" or a simple "block"

Thank you for debugging the issue, the block_given? was replaced with block in r22027 but the 5.0-stable branch still uses it. This is the reason why the infinite loop happens in the 5.0-stable.

I have merged the fix to the stable branches.

Actions

Copy link

#10

Updated by Vincent Robert 6 months ago

I don't know how, but Redmine 4.2 seems impacted by this change. All our automated tests under Redmine 4 are now failing with this error message:

Run bundle exec rake db:create db:migrate
rake aborted!
LoadError: cannot load such file -- blankslate
/redmine/lib/redmine/views/builders/structure.rb:20:in `<top (required)>'
/redmine/lib/redmine/views/builders/json.rb:20:in `<top (required)>'
/redmine/lib/redmine/views/builders.rb:20:in `<top (required)>'
/redmine/lib/redmine.rb:64:in `<top (required)>'
/redmine/config/initializers/30-redmine.rb:7:in `<top (required)>'