Merge remote-tracking branch 'upstream/master' into ce-to-ee-2018-09-18

# Conflicts:
#	app/views/profiles/keys/_key_details.html.haml

[ci skip]

Gemfile.rails5.lock

@@ -136,7 +136,7 @@ GEM
     coderay (1.1.2)
     coercible (1.0.0)
       descendants_tracker (~> 0.0.1)
-    commonmarker (0.17.8)
+    commonmarker (0.17.13)
       ruby-enum (~> 0.5)
     concord (0.1.5)
       adamantium (~> 0.2.0)

app/models/user.rb

@@ -651,7 +651,7 @@ class User < ActiveRecord::Base
   # possibility of the commit_email column not existing.
 
   def commit_email
-    return unless has_attribute?(:commit_email)
+    return self.email unless has_attribute?(:commit_email)
 
     # The commit email is the same as the primary email if undefined
     super.presence || self.email

app/validators/url_validator.rb

@@ -41,12 +41,13 @@ class UrlValidator < ActiveModel::EachValidator
   def validate_each(record, attribute, value)
     @record = record
 
-    if value.present?
-      value.strip!
-    else
+    unless value.present?
       record.errors.add(attribute, 'must be a valid URL')
+      return
     end
 
+    value = strip_value!(record, attribute, value)
+
     Gitlab::UrlBlocker.validate!(value, blocker_args)
   rescue Gitlab::UrlBlocker::BlockedUrlError => e
     record.errors.add(attribute, "is blocked: #{e.message}")
@@ -54,6 +55,13 @@ class UrlValidator < ActiveModel::EachValidator
 
   private
 
+  def strip_value!(record, attribute, value)
+    new_value = value.strip
+    return value if new_value == value
+
+    record.public_send("#{attribute}=", new_value) # rubocop:disable GitlabSecurity/PublicSend
+  end
+
   def default_options
     # By default the validator doesn't block any url based on the ip address
     {

app/views/profiles/keys/_key_details.html.haml

@@ -24,5 +24,9 @@
       = @key.key
   .col-md-12
     .float-right
+<<<<<<< HEAD
       - unless @key.is_a? LDAPKey
         = link_to 'Remove', path_to_key(@key, is_admin), data: {confirm: 'Are you sure?'}, method: :delete, class: "btn btn-remove delete-key qa-delete-key-button"
+=======
+      = link_to 'Remove', path_to_key(@key, is_admin), data: {confirm: 'Are you sure?'}, method: :delete, class: "btn btn-remove delete-key qa-delete-key-button"
+>>>>>>> upstream/master

qa/qa/page/main/login.rb

@@ -66,6 +66,8 @@ module QA
           end
 
           using_wait_time 0 do
+            set_initial_password_if_present
+
             sign_in_using_gitlab_credentials(admin)
           end
 

spec/validators/url_validator_spec.rb

@@ -24,6 +24,21 @@ describe UrlValidator do
 
       expect(badge.errors.empty?).to be true
     end
+
+    it 'strips urls' do
+      badge.link_url = "\n\r\n\nhttps://127.0.0.1\r\n\r\n\n\n\n"
+
+      # It's unusual for a validator to modify its arguments. Some extensions,
+      # such as attr_encrypted, freeze the string to signal that modifications
+      # will not be persisted, so freeze this string to ensure the scheme is
+      # compatible with them.
+      badge.link_url.freeze
+
+      subject
+
+      expect(badge.errors).to be_empty
+      expect(badge.link_url).to eq('https://127.0.0.1')
+    end
   end
 
   context 'when allow_localhost is set to false' do