Skip to content

Fix error when blob has no auxiliary viewer

Wei-Meng Lee requested to merge weimeng-master-patch-35379 into master

What does this MR do?

Several projects, including some belonging to a large customer (5,000-seat Silver, Zendesk ticket, internal only), have been impacted by a bug:

      "exception.message": "undefined method `valid?' for nil:NilClass",

      (...)

      "exception.backtrace": [
        "app/helpers/blob_helper.rb:362:in `show_suggest_pipeline_creation_celebration?'",
        "app/views/projects/blob/show.html.haml:18",
        "app/controllers/application_controller.rb:134:in `render'",
        "app/controllers/projects/blob_controller.rb:221:in `show_html'",
        "app/controllers/projects/blob_controller.rb:54:in `block (2 levels) in show'",
        "app/controllers/projects/blob_controller.rb:52:in `show'",
        "app/controllers/application_controller.rb:546:in `block in allow_gitaly_ref_name_caching'",
        "lib/gitlab/gitaly_client.rb:325:in `allow_ref_name_caching'",
        "app/controllers/application_controller.rb:545:in `allow_gitaly_ref_name_caching'",
        "ee/lib/gitlab/ip_address_state.rb:10:in `with'",
        "ee/app/controllers/ee/application_controller.rb:44:in `set_current_ip_address'",
        "app/controllers/application_controller.rb:491:in `set_current_admin'",
        "lib/gitlab/session.rb:11:in `with_session'",
        "app/controllers/application_controller.rb:482:in `set_session_storage'",
        "app/controllers/application_controller.rb:476:in `set_locale'",
        "lib/gitlab/error_tracking.rb:50:in `with_context'",
        "app/controllers/application_controller.rb:541:in `sentry_context'",
        "app/controllers/application_controller.rb:469:in `block in set_current_context'",
        "lib/gitlab/application_context.rb:52:in `block in use'",
        "lib/gitlab/application_context.rb:52:in `use'",
        "lib/gitlab/application_context.rb:20:in `with_context'",
        "app/controllers/application_controller.rb:462:in `set_current_context'",
        "ee/lib/omni_auth/strategies/group_saml.rb:41:in `other_phase'",
        "ee/lib/gitlab/jira/middleware.rb:19:in `call'"
      ],

Kibana search: https://log.gprd.gitlab.net/goto/7fe53673055e7b976d56ddb09b6226ef

This MR resolves the bug by adding a safe navigation operator so @blob.auxiliary_viewer.valid?(project: @project, sha: @commit.sha, user: current_user) will not cause an exception when @blob.auxiliary_viewer is nil.

But is @blob.auxiliary_viewer supposed to be nil ever?

Screenshots

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • Label as security and @ mention @gitlab-com/gl-security/appsec
  • The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • Security reports checked/validated by a reviewer from the AppSec team
Edited by Wei-Meng Lee

Merge request reports