Do label validation for issues/merge requests API

3 files changed, 44 insertions, 4 deletions

diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb
index 8189e433789..d36b29a00b1 100644
--- a/lib/api/helpers.rb
+++ b/lib/api/helpers.rb
@@ -112,6 +112,21 @@ module API
       ActionController::Parameters.new(attrs).permit!
     end
 
+    # Helper method for validating all labels against its names
+    def validate_label_params(params)
+      if params[:labels].present?
+        params[:labels].split(',').each do |label_name|
+          label = user_project.labels.create_with(
+            color: Label::DEFAULT_COLOR).find_or_initialize_by(
+              title: label_name.strip)
+          if label.invalid?
+            return true
+          end
+        end
+      end
+      false
+    end
+
     # error helpers
 
     def forbidden!
diff --git a/lib/api/issues.rb b/lib/api/issues.rb
index b29118b2fd8..055529ccbd8 100644
--- a/lib/api/issues.rb
+++ b/lib/api/issues.rb
@@ -51,12 +51,18 @@ module API
         required_attributes! [:title]
         attrs = attributes_for_keys [:title, :description, :assignee_id, :milestone_id]
 
+        # Validate label names in advance
+        if validate_label_params(params)
+          return render_api_error!('Label names invalid', 405)
+        end
+
         issue = ::Issues::CreateService.new(user_project, current_user, attrs).execute
 
         if issue.valid?
-          # Find or create labels and attach to issue
+          # Find or create labels and attach to issue. Labels are valid because
+          # we already checked its name, so there can't be an error here
           if params[:labels].present?
-            issue.add_labels_by_names(params[:labels].split(","))
+            issue.add_labels_by_names(params[:labels].split(','))
           end
 
           present issue, with: Entities::Issue
@@ -83,12 +89,19 @@ module API
         authorize! :modify_issue, issue
         attrs = attributes_for_keys [:title, :description, :assignee_id, :milestone_id, :state_event]
 
+        # Validate label names in advance
+        if validate_label_params(params)
+          return render_api_error!('Label names invalid', 405)
+        end
+
         issue = ::Issues::UpdateService.new(user_project, current_user, attrs).execute(issue)
 
         if issue.valid?
-          # Find or create labels and attach to issue
+          # Find or create labels and attach to issue. Labels are valid because
+          # we already checked its name, so there can't be an error here
           if params[:labels].present?
-            issue.add_labels_by_names(params[:labels].split(","))
+            # Create and add labels to the new created issue
+            issue.add_labels_by_names(params[:labels].split(','))
           end
 
           present issue, with: Entities::Issue
diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb
index acca7cb6bad..0d765f9280e 100644
--- a/lib/api/merge_requests.rb
+++ b/lib/api/merge_requests.rb
@@ -76,6 +76,12 @@ module API
         authorize! :write_merge_request, user_project
         required_attributes! [:source_branch, :target_branch, :title]
         attrs = attributes_for_keys [:source_branch, :target_branch, :assignee_id, :title, :target_project_id, :description]
+
+        # Validate label names in advance
+        if validate_label_params(params)
+          return render_api_error!('Label names invalid', 405)
+        end
+
         merge_request = ::MergeRequests::CreateService.new(user_project, current_user, attrs).execute
 
         if merge_request.valid?
@@ -109,6 +115,12 @@ module API
         attrs = attributes_for_keys [:source_branch, :target_branch, :assignee_id, :title, :state_event, :description]
         merge_request = user_project.merge_requests.find(params[:merge_request_id])
         authorize! :modify_merge_request, merge_request
+
+        # Validate label names in advance
+        if validate_label_params(params)
+          return render_api_error!('Label names invalid', 405)
+        end
+
         merge_request = ::MergeRequests::UpdateService.new(user_project, current_user, attrs).execute(merge_request)
 
         if merge_request.valid?