From 7a5c4cd0ca692e2fac0d648726b71dcd304602ec Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Wed, 31 Jul 2019 15:38:02 -0700 Subject: [PATCH] Fix SystemStackError when Peek bar is active with Rugged calls Peek attempts to serialize results with `to_json`, which calls `ActiveSupport::JSON`. If an object is passed to `to_json` that contains instance variables, `ActiveSupport` will attempt to recursively traverse all variables. The problem is that we can get into an infinite loop if the instance references to an instance that references to something else that points back to the same instance. To avoid this mess, we just call `to_s` on the object. It appears only `Gitlab::Git::Repository` and `::Repository` are the culprits here. Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/65404 --- lib/peek/views/rugged.rb | 8 ++++++-- spec/lib/peek/views/rugged_spec.rb | 10 +++++++--- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/lib/peek/views/rugged.rb b/lib/peek/views/rugged.rb index 6b9d3e7b1a3..18b3f422852 100644 --- a/lib/peek/views/rugged.rb +++ b/lib/peek/views/rugged.rb @@ -29,8 +29,12 @@ module Peek def format_args(args) args.map do |arg| - # Needed to avoid infinite as_json calls - if arg.is_a?(Gitlab::Git::Repository) + # ActiveSupport::JSON recursively calls as_json on all + # instance variables, and if that instance variable points to + # something that refers back to the same instance, we can wind + # up in an infinite loop. Currently this only seems to happen with + # Gitlab::Git::Repository and ::Repository. + if arg.instance_variables.present? arg.to_s else arg diff --git a/spec/lib/peek/views/rugged_spec.rb b/spec/lib/peek/views/rugged_spec.rb index 8bf996fc6bc..d07d6b51a1f 100644 --- a/spec/lib/peek/views/rugged_spec.rb +++ b/spec/lib/peek/views/rugged_spec.rb @@ -24,7 +24,7 @@ describe Peek::Views::Rugged, :request_store do args: [project.repository.raw, 'HEAD'], duration: 0.123) ::Gitlab::RuggedInstrumentation.add_call_details(feature: :rugged_test2, - args: [project.repository.raw, 'refs/heads/master'], + args: [project.repository, 'refs/heads/master'], duration: 0.456) results = subject.results @@ -32,7 +32,11 @@ describe Peek::Views::Rugged, :request_store do expect(results[:duration]).to eq('1234.00ms') expect(results[:details].count).to eq(2) - expect(results[:details][0][:args]).to eq([project.repository.raw.to_s, "refs/heads/master"]) - expect(results[:details][1][:args]).to eq([project.repository.raw.to_s, "HEAD"]) + expected = [ + [project.repository.raw.to_s, "HEAD"], + [project.repository.to_s, "refs/heads/master"] + ] + + expect(results[:details].map { |data| data[:args] }).to match_array(expected) end end