Revert "Merge branch 'sh-bump-ruby-version' into 'master'"

This reverts merge request !257

.gitlab-ci.yml

-image: "ruby:2.5"
+image: "ruby:2.3"
 
 before_script:
   - export PATH=~/bin:/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/go/bin
@@ -30,18 +30,18 @@ rubocop:
   except:
     - tags
 
-#ruby 2.4
-rspec:ruby2.4:
-  image: ruby:2.4
+#ruby 2.2
+rspec:ruby2.2:
+  image: ruby:2.2
   <<: *rspec_definition
   tags:
     - ruby
   except:
     - tags
 
-#ruby 2.3
-rspec:ruby2.3:
-  image: ruby:2.3
+#ruby 2.1
+rspec:ruby2.1:
+  image: ruby:2.1
   <<: *rspec_definition
   tags:
     - ruby
@@ -77,7 +77,7 @@ go:1.10:
 
 go:1.11:
   <<: *go_definition
-  image: golang:1.11
+  image: golang:1.10
 
 codequality:
   image: docker:stable

.rubocop.yml

@@ -6,7 +6,6 @@ AllCops:
     - 'tmp/**/*'
     - 'bin/**/*'
     - 'hooks/**/*'
-    - 'support/**/*'
     - 'Guardfile'
 
 Layout/DotPosition:
@@ -36,9 +35,6 @@ Metrics/CyclomaticComplexity:
 Metrics/PerceivedComplexity:
   Enabled: false
 
-Naming/AccessorMethodName:
-  Enabled: false
-
 Style/Documentation:
   Enabled: false
 
@@ -51,6 +47,9 @@ Style/StringLiterals:
 Style/GlobalVars:
   Enabled: false
 
+Style/AccessorMethodName:
+  Enabled: false
+
 Style/GuardClause:
   Enabled: false
 

.ruby-version

-2.5.3
+2.3.7

Gemfile

 source 'https://rubygems.org'
 
-group :development do
-  gem 'pry-byebug', '~> 3.6', require: false
-end
-
 group :development, :test do
   gem 'listen', '~> 0.5.0'
   gem 'rspec', '~> 3.8.0'
   gem 'rspec-parameterized', '~> 0.4.0'
-  gem 'rubocop', '0.61', require: false
-  gem 'simplecov', '~> 0.16.1', require: false
+  gem 'rubocop', '0.49.1', require: false
+  gem 'simplecov', '~> 0.9.0', require: false
   gem 'vcr', '~> 4.0.0'
   gem 'webmock', '~> 3.4.0'
 end

Gemfile.lock

@@ -8,27 +8,26 @@ GEM
     addressable (2.5.2)
       public_suffix (>= 2.0.2, < 4.0)
     ast (2.4.0)
-    binding_ninja (0.2.2)
-    byebug (10.0.2)
+    binding_of_caller (0.8.0)
+      debug_inspector (>= 0.0.1)
     coderay (1.1.2)
     concord (0.1.5)
       adamantium (~> 0.2.0)
       equalizer (~> 0.0.9)
     crack (0.4.3)
       safe_yaml (~> 1.0.0)
+    debug_inspector (0.0.3)
     diff-lcs (1.3)
-    docile (1.3.1)
+    docile (1.1.5)
     equalizer (0.0.11)
     hashdiff (0.3.7)
     ice_nine (0.11.2)
-    jaro_winkler (1.5.1)
-    json (2.1.0)
     listen (0.5.3)
     memoizable (0.4.2)
       thread_safe (~> 0.3, >= 0.3.1)
-    method_source (0.9.2)
+    multi_json (1.13.1)
     parallel (1.12.1)
-    parser (2.5.3.0)
+    parser (2.5.1.2)
       ast (~> 2.4.0)
     powerpack (0.1.2)
     proc_to_ast (0.1.0)
@@ -36,51 +35,46 @@ GEM
       parser
       unparser
     procto (0.0.3)
-    pry (0.12.2)
-      coderay (~> 1.1.0)
-      method_source (~> 0.9.0)
-    pry-byebug (3.6.0)
-      byebug (~> 10.0)
-      pry (~> 0.10)
     public_suffix (3.0.3)
-    rainbow (3.0.0)
+    rainbow (2.2.2)
+      rake
+    rake (12.3.1)
     rspec (3.8.0)
       rspec-core (~> 3.8.0)
       rspec-expectations (~> 3.8.0)
       rspec-mocks (~> 3.8.0)
     rspec-core (3.8.0)
       rspec-support (~> 3.8.0)
-    rspec-expectations (3.8.2)
+    rspec-expectations (3.8.1)
       diff-lcs (>= 1.2.0, < 2.0)
       rspec-support (~> 3.8.0)
     rspec-mocks (3.8.0)
       diff-lcs (>= 1.2.0, < 2.0)
       rspec-support (~> 3.8.0)
-    rspec-parameterized (0.4.1)
-      binding_ninja (>= 0.2.1)
+    rspec-parameterized (0.4.0)
+      binding_of_caller
       parser
       proc_to_ast
       rspec (>= 2.13, < 4)
       unparser
     rspec-support (3.8.0)
-    rubocop (0.61.0)
-      jaro_winkler (~> 1.5.1)
+    rubocop (0.49.1)
       parallel (~> 1.10)
-      parser (>= 2.5, != 2.5.1.1)
+      parser (>= 2.3.3.1, < 3.0)
       powerpack (~> 0.1)
-      rainbow (>= 2.2.2, < 4.0)
+      rainbow (>= 1.99.1, < 3.0)
       ruby-progressbar (~> 1.7)
-      unicode-display_width (~> 1.4.0)
-    ruby-progressbar (1.10.0)
+      unicode-display_width (~> 1.0, >= 1.0.1)
+    ruby-progressbar (1.9.0)
     safe_yaml (1.0.4)
-    simplecov (0.16.1)
-      docile (~> 1.1)
-      json (>= 1.8, < 3)
-      simplecov-html (~> 0.10.0)
-    simplecov-html (0.10.2)
+    simplecov (0.9.2)
+      docile (~> 1.1.0)
+      multi_json (~> 1.0)
+      simplecov-html (~> 0.9.0)
+    simplecov-html (0.9.0)
     thread_safe (0.3.6)
     unicode-display_width (1.4.0)
-    unparser (0.4.2)
+    unparser (0.2.8)
       abstract_type (~> 0.0.7)
       adamantium (~> 0.2.0)
       concord (~> 0.1.5)
@@ -99,13 +93,12 @@ PLATFORMS
 
 DEPENDENCIES
   listen (~> 0.5.0)
-  pry-byebug (~> 3.6)
   rspec (~> 3.8.0)
   rspec-parameterized (~> 0.4.0)
-  rubocop (= 0.61)
-  simplecov (~> 0.16.1)
+  rubocop (= 0.49.1)
+  simplecov (~> 0.9.0)
   vcr (~> 4.0.0)
   webmock (~> 3.4.0)
 
 BUNDLED WITH
-   1.17.1
+   1.16.3

lib/action/custom.rb

@@ -96,13 +96,12 @@ module Action
 
     def print_flush(str)
       return false unless str
-
       $stdout.print(Base64.decode64(str))
       $stdout.flush
     end
 
     def inform_client(str)
-      warn(format_gitlab_output(str))
+      $stderr.puts(format_gitlab_output(str))
     end
 
     def format_gitlab_output(str)

lib/gitlab_access.rb

@@ -31,10 +31,10 @@ class GitlabAccess
 
     true
   rescue GitlabNet::ApiUnreachableError
-    warn "GitLab: Failed to authorize your Git request: internal API unreachable"
+    $stderr.puts "GitLab: Failed to authorize your Git request: internal API unreachable"
     false
   rescue AccessDeniedError => ex
-    warn "GitLab: #{ex.message}"
+    $stderr.puts "GitLab: #{ex.message}"
     false
   end
 

lib/gitlab_keys.rb

 require 'timeout'
-require 'open3'
 
 require_relative 'gitlab_config'
 require_relative 'gitlab_logger'
@@ -15,7 +14,7 @@ class GitlabKeys # rubocop:disable Metrics/ClassLength
   end
 
   def self.command_key(key_id)
-    unless /\A[a-z0-9-]+\z/ =~ key_id # rubocop:disable Performance/RegexpMatch
+    unless /\A[a-z0-9-]+\z/ =~ key_id
       raise KeyError, "Invalid key_id: #{key_id.inspect}"
     end
 
@@ -108,9 +107,7 @@ class GitlabKeys # rubocop:disable Metrics/ClassLength
     open_auth_file('r') do |f|
       f.each_line do |line|
         matchd = line.match(/key-(\d+)/)
-
         next unless matchd
-
         puts matchd[1]
       end
     end
@@ -141,7 +138,6 @@ class GitlabKeys # rubocop:disable Metrics/ClassLength
       open_auth_file('r+') do |f|
         while line = f.gets # rubocop:disable Lint/AssignmentInCondition
           next unless line.start_with?("command=\"#{self.class.command_key(@key_id)}\"")
-
           f.seek(-line.length, IO::SEEK_CUR)
           # Overwrite the line with #'s. Because the 'line' variable contains
           # a terminating '\n', we write line.length - 1 '#' characters.
@@ -159,24 +155,20 @@ class GitlabKeys # rubocop:disable Metrics/ClassLength
 
   def check_permissions
     open_auth_file(File::RDWR | File::CREAT) { true }
-  rescue StandardError => ex
+  rescue => ex
     puts "error: could not open #{auth_file}: #{ex}"
-
-    cmd = if File.exist?(auth_file)
-            %W{ls -l #{auth_file}}
-          else
-            # Maybe the parent directory is not writable?
-            %W{ls -ld #{File.dirname(auth_file)}}
-          end
-
-    output, = Open3.capture2e(cmd.join(' '))
-    puts output
+    if File.exist?(auth_file)
+      system('ls', '-l', auth_file)
+    else
+      # Maybe the parent directory is not writable?
+      system('ls', '-ld', File.dirname(auth_file))
+    end
     false
   end
 
   def lock(timeout = 10)
     File.open(lock_file, "w+") do |f|
-      begin # rubocop:disable Style/RedundantBegin
+      begin
         f.flock File::LOCK_EX
         Timeout.timeout(timeout) { yield }
       ensure
@@ -190,7 +182,7 @@ class GitlabKeys # rubocop:disable Metrics/ClassLength
   end
 
   def open_auth_file(mode)
-    File.open(auth_file, mode, 0o600) do |file|
+    open(auth_file, mode, 0o600) do |file|
       file.chmod(0o600)
       yield file
     end

lib/gitlab_lfs_authentication.rb

@@ -13,7 +13,7 @@ class GitlabLfsAuthentication
   def self.build_from_json(json)
     values = JSON.parse(json)
     new(values['username'], values['lfs_token'], values['repository_http_path'])
-  rescue StandardError
+  rescue
     nil
   end
 

lib/gitlab_logger.rb

@@ -7,14 +7,15 @@ require_relative 'gitlab_config'
 def convert_log_level(log_level)
   Logger.const_get(log_level.upcase)
 rescue NameError
-  warn "WARNING: Unrecognized log level #{log_level.inspect}. Falling back to INFO."
+  $stderr.puts "WARNING: Unrecognized log level #{log_level.inspect}."
+  $stderr.puts "WARNING: Falling back to INFO."
   Logger::INFO
 end
 
 class GitlabLogger
   # Emulate the quoting logic of logrus
   # https://github.com/sirupsen/logrus/blob/v1.0.5/text_formatter.go#L143-L156
-  SHOULD_QUOTE = /[^a-zA-Z0-9\-._\/@^+]/.freeze
+  SHOULD_QUOTE = /[^a-zA-Z0-9\-._\/@^+]/
 
   LEVELS = {
     Logger::INFO => 'info'.freeze,

lib/gitlab_net.rb

-# frozen_string_literal: true
-
 require 'net/http'
 require 'openssl'
 require 'json'
@@ -13,7 +11,7 @@ class GitlabNet # rubocop:disable Metrics/ClassLength
   include HTTPHelper
 
   CHECK_TIMEOUT = 5
-  API_INACCESSIBLE_MESSAGE = 'API is not accessible'
+  API_INACCESSIBLE_MESSAGE = 'API is not accessible'.freeze
 
   def check_access(cmd, gl_repository, repo, who, changes, protocol, env: {})
     changes = changes.join("\n") unless changes.is_a?(String)
@@ -78,8 +76,8 @@ class GitlabNet # rubocop:disable Metrics/ClassLength
   def merge_request_urls(gl_repository, repo_path, changes)
     changes = changes.join("\n") unless changes.is_a?(String)
     changes = changes.encode('UTF-8', 'ASCII', invalid: :replace, replace: '')
-    url = "#{internal_api_endpoint}/merge_request_urls?project=#{uri_escape(repo_path)}&changes=#{uri_escape(changes)}"
-    url += "&gl_repository=#{uri_escape(gl_repository)}" if gl_repository
+    url = "#{internal_api_endpoint}/merge_request_urls?project=#{URI.escape(repo_path)}&changes=#{URI.escape(changes)}"
+    url += "&gl_repository=#{URI.escape(gl_repository)}" if gl_repository
     resp = get(url)
 
     if resp.code == '200'
@@ -87,7 +85,7 @@ class GitlabNet # rubocop:disable Metrics/ClassLength
     else
       []
     end
-  rescue StandardError
+  rescue
     []
   end
 
@@ -96,9 +94,9 @@ class GitlabNet # rubocop:disable Metrics/ClassLength
   end
 
   def authorized_key(key)
-    resp = get("#{internal_api_endpoint}/authorized_keys?key=#{URI.escape(key, '+/=')}") # rubocop:disable Lint/UriEscapeUnescape
+    resp = get("#{internal_api_endpoint}/authorized_keys?key=#{URI.escape(key, '+/=')}")
     JSON.parse(resp.body) if resp.code == "200"
-  rescue StandardError
+  rescue
     nil
   end
 
@@ -108,7 +106,7 @@ class GitlabNet # rubocop:disable Metrics/ClassLength
     resp = post("#{internal_api_endpoint}/two_factor_recovery_codes", id_sym => id)
 
     JSON.parse(resp.body) if resp.code == '200'
-  rescue StandardError
+  rescue
     {}
   end
 
@@ -117,7 +115,7 @@ class GitlabNet # rubocop:disable Metrics/ClassLength
     resp = post("#{internal_api_endpoint}/notify_post_receive", params)
 
     resp.code == '200'
-  rescue StandardError
+  rescue
     false
   end
 
@@ -145,15 +143,11 @@ class GitlabNet # rubocop:disable Metrics/ClassLength
   def self.parse_who(who)
     if who.start_with?("key-")
       value = who.gsub("key-", "")
-
       raise ArgumentError, "who='#{who}' is invalid!" unless value =~ /\A[0-9]+\z/
-
       [:key_id, 'key_id', value]
     elsif who.start_with?("user-")
       value = who.gsub("user-", "")
-
       raise ArgumentError, "who='#{who}' is invalid!" unless value =~ /\A[0-9]+\z/
-
       [:user_id, 'user_id', value]
     elsif who.start_with?("username-")
       [:username, 'username', who.gsub("username-", "")]
@@ -167,8 +161,4 @@ class GitlabNet # rubocop:disable Metrics/ClassLength
   def sanitize_path(repo)
     repo.delete("'")
   end
-
-  def uri_escape(str)
-    URI.escape(str) # rubocop:disable Lint/UriEscapeUnescape
-  end
 end

lib/gitlab_post_receive.rb

@@ -25,7 +25,6 @@ class GitlabPostReceive
     end
 
     return false unless response
-
     print_broadcast_message(response['broadcast_message']) if response['broadcast_message']
     print_merge_request_links(response['merge_request_urls']) if response['merge_request_urls']
     puts response['redirected_message'] if response['redirected_message']
@@ -44,7 +43,6 @@ class GitlabPostReceive
 
   def print_merge_request_links(merge_request_urls)
     return if merge_request_urls.empty?
-
     puts
     merge_request_urls.each { |mr| print_merge_request_link(mr) }
   end

lib/gitlab_shell.rb

@@ -81,23 +81,23 @@ class GitlabShell # rubocop:disable Metrics/ClassLength
 
     true
   rescue GitlabNet::ApiUnreachableError
-    warn "GitLab: Failed to authorize your Git request: internal API unreachable"
+    $stderr.puts "GitLab: Failed to authorize your Git request: internal API unreachable"
     false
   rescue AccessDeniedError => ex
     $logger.warn('Access denied', command: origin_cmd, user: log_username)
-    warn "GitLab: #{ex.message}"
+    $stderr.puts "GitLab: #{ex.message}"
     false
   rescue DisallowedCommandError
     $logger.warn('Denied disallowed command', command: origin_cmd, user: log_username)
-    warn "GitLab: Disallowed command"
+    $stderr.puts "GitLab: Disallowed command"
     false
   rescue InvalidRepositoryPathError
-    warn "GitLab: Invalid repository path"
+    $stderr.puts "GitLab: Invalid repository path"
     false
   rescue Action::Custom::BaseError => ex
     $logger.warn('Custom action error', exception: ex.class, message: ex.message,
                                         command: origin_cmd, user: log_username)
-    warn ex.message
+    $stderr.puts ex.message
     false
   end
 
@@ -121,7 +121,6 @@ class GitlabShell # rubocop:disable Metrics/ClassLength
     case @command
     when GIT_LFS_AUTHENTICATE_COMMAND
       raise DisallowedCommandError unless args.count >= 2
-
       @repo_name = args[1]
       case args[2]
       when 'download'
@@ -133,7 +132,6 @@ class GitlabShell # rubocop:disable Metrics/ClassLength
       end
     else
       raise DisallowedCommandError unless args.count == 2
-
       @repo_name = args.last
     end
 
@@ -203,7 +201,7 @@ class GitlabShell # rubocop:disable Metrics/ClassLength
     begin
       if defined?(@who)
         @user = api.discover(@who)
-        @gl_id = "user-#{@user['id']}" if @user&.key?('id')
+        @gl_id = "user-#{@user['id']}" if @user && @user.key?('id')
       else
         @user = api.discover(@gl_id)
       end

lib/http_helper.rb

@@ -74,7 +74,7 @@ module HTTPHelper
     begin
       start_time = Time.new
       response = http.start { http.request(request) }
-    rescue StandardError => e
+    rescue => e
       $logger.warn('Failed to connect', method: method.to_s.upcase, url: url, error: e)
       raise GitlabNet::ApiUnreachableError
     ensure

lib/httpunix.rb

@@ -12,14 +12,13 @@ module URI
     def hostname
       # decode %XX from path to file
       v = host
-      URI.decode(v) # rubocop:disable Lint/UriEscapeUnescape
+      URI.decode(v)
     end
 
     # port is not allowed in URI
     DEFAULT_PORT = nil
-    def set_port(value)
-      return value unless value
-
+    def set_port(v)
+      return v unless v
       raise InvalidURIError, "http+unix:// cannot contain port"
     end
   end

spec/gitlab_keys_spec.rb

@@ -5,8 +5,6 @@ require 'stringio'
 describe GitlabKeys do
   before do
     $logger = double('logger').as_null_object
-    # The default 'auth_file' value from config.yml.example is '/home/git/.ssh/authorized_keys'
-    allow(GitlabConfig).to receive_message_chain(:new, :auth_file).and_return('/home/git/.ssh/authorized_keys')
   end
 
   describe '.command' do
@@ -150,9 +148,7 @@ describe GitlabKeys do
 
     context "without file writing" do
       before do
-        file = double(:file, puts: nil, chmod: nil, flock: nil)
-        expect(File).to receive(:open).with(tmp_authorized_keys_path + '.lock', 'w+').and_yield(file)
-        expect(File).to receive(:open).with(tmp_authorized_keys_path, "a", 0o600).and_yield(file)
+        expect(gitlab_keys).to receive(:open).and_yield(double(:file, puts: nil, chmod: nil))
       end
 
       it "should log an add-key event" do
@@ -193,8 +189,7 @@ describe GitlabKeys do
 
     context "without file writing" do
       before do
-        allow(File).to receive(:open).with("#{ROOT_PATH}/config.yml", 'r:bom|utf-8').and_call_original
-        allow(File).to receive(:open).with('/home/git/.ssh/authorized_keys', 'r+', 384)
+        allow(gitlab_keys).to receive(:open)
         allow(gitlab_keys).to receive(:lock).and_yield
       end
 
@@ -230,8 +225,7 @@ describe GitlabKeys do
     let(:gitlab_keys) { build_gitlab_keys('clear') }
 
     it "should return true" do
-      allow(File).to receive(:open).with("#{ROOT_PATH}/config.yml", 'r:bom|utf-8').and_call_original
-      allow(File).to receive(:open).with('/home/git/.ssh/authorized_keys', 'w', 384)
+      allow(gitlab_keys).to receive(:open)
       expect(gitlab_keys.send(:clear)).to be_truthy
     end
   end
@@ -246,7 +240,7 @@ describe GitlabKeys do
 
     it 'returns false if opening raises an exception' do
       expect(gitlab_keys).to receive(:open_auth_file).and_raise("imaginary error")
-      expect { expect(gitlab_keys.exec).to eq(false) }.to output(/imaginary error/).to_stdout
+      expect(gitlab_keys.exec).to eq(false)
     end
 
     it 'creates the keys file if it does not exist' do

spec/gitlab_logger_spec.rb

@@ -3,13 +3,11 @@ require_relative '../lib/gitlab_logger'
 require 'securerandom'
 
 describe :convert_log_level do
-  it "returns desired Logger::<type>" do
-    expect(convert_log_level(:debug)).to eq(Logger::DEBUG)
-  end
+  subject { convert_log_level :extreme }
 
   it "converts invalid log level to Logger::INFO" do
-    expect { convert_log_level(:extreme) }.to output(/WARNING: Unrecognized log level :extreme. Falling back to INFO./).to_stderr
-    expect(convert_log_level(:extreme)).to eq(Logger::INFO)
+    expect($stderr).to receive(:puts).at_least(:once)
+    is_expected.to eq(Logger::INFO)
   end
 end
 

spec/gitlab_net_spec.rb

@@ -67,7 +67,7 @@ describe GitlabNet, vcr: true do
 
   describe '#lfs_authenticate' do
     context 'lfs authentication succeeded' do
-      let(:repository_http_path) { URI.join(internal_api_endpoint.sub('/api/v4/internal', ''), project).to_s }
+      let(:repository_http_path) { URI.join(internal_api_endpoint.sub('api/v4', ''), project).to_s }
 
       context 'for download operation' do
         it 'should return the correct data' do

spec/gitlab_shell_gitlab_shell_spec.rb

 require_relative 'spec_helper'
-require 'open3'
 
 describe 'bin/gitlab-shell' do
   def original_root_path
@@ -132,13 +131,12 @@ describe 'bin/gitlab-shell' do
 
   def run!(args)
     cmd = [
-      'SSH_CONNECTION=fake',
       gitlab_shell_path,
       args
-    ].flatten.join(' ')
+    ].flatten.compact
 
-    stdout, _stderr, status = Open3.capture3(cmd)
+    output = IO.popen({'SSH_CONNECTION' => 'fake'}, cmd, &:read)
 
-    [stdout, status]
+    [output, $?]
   end
 end

spec/vcr_cassettes/custom-action-ok-not-json.yml

@@ -46,6 +46,6 @@ http_interactions:
       - '1.436040'
     body:
       encoding: UTF-8
-      string: '{'
+      string: '""'
     http_version:
   recorded_at: Fri, 20 Jul 2018 06:18:58 GMT