Skip to content
Snippets Groups Projects

FindCommits: avoid looping explosions related to `follow`

Merged Georges Racinet requested to merge topic/stable/find-commits-explosion into branch/stable

Correctly treats the case of ancestors of a single rev, ignores follow in range notation cases

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • A quick search in the Rails app code base reveals that, as of upstream v15.2.4, follow is automatically set for paths requests with exactly one path (app/models/repository.rb):

      def commits(ref = nil, opts = {})
        options = {
          repo: raw_repository,
          ref: ref,
          path: opts[:path],
          author: opts[:author],
          follow: Array(opts[:path]).length == 1,
          limit: opts[:limit],
          offset: opts[:offset],
          skip_merges: !!opts[:skip_merges],
          after: opts[:after],
          before: opts[:before],
          all: !!opts[:all],
          first_parent: !!opts[:first_parent],
          order: opts[:order],
          literal_pathspec: opts.fetch(:literal_pathspec, true),
          trailers: opts[:trailers]
        }
    
        commits = Gitlab::Git::Commit.where(options)

    This is the only caller from the app layer. The parameter is exposed from the lib layer in Commit.where and Git::Repository#log.

    Unfortunately, this Repository#commits is in turn used quite generically from the uper layers (controllers, services) and it is therefore quite possible to have Range Notation revspecs together with a single path, possibly by accident (meaning that copy tracing would not matter much for the caller).

Please register or sign in to reply
Loading