v3: Surface helpful error when trying to create security group with

invalid name

For example, if you `POST /v3/security_groups` with `{"name": "🐧"}`,
you used to get a 500. Now, you get a 422 with message "Security group
contains invalid characters".

Note that in the case of emojis, at least, this error will only occur in
MySQL (not Postgres).

[Finishes #172574458, #172574465]

Co-authored-by: Sarah Weinstein <sweinstein@pivotal.io>
Co-authored-by: Reid Mitchell <rmitchell@pivotal.io>
Co-authored-by: Merric de Launey <mdelauney@pivotal.io>

Author: 3 people

app/actions/security_group_create.rb

@@ -1,5 +1,7 @@
 module VCAP::CloudController
   class SecurityGroupCreate
+    MYSQL_INVALID_VALUE_ERROR = 1366
+
     class Error < ::StandardError
     end
 
@@ -24,6 +26,8 @@ def create(message)
         security_group
       rescue Sequel::ValidationFailed => e
         validation_error!(e, message)
+      rescue Sequel::DatabaseError => e
+        invalid_value_error!(e)
       end
 
       private
@@ -36,17 +40,29 @@ def validation_error!(error, message)
         error!(error.message)
       end
 
+      def invalid_value_error!(error)
+        if error.wrapped_exception.is_a?(Mysql2::Error) && error.wrapped_exception.error_number == MYSQL_INVALID_VALUE_ERROR
+          if /column 'name'/ =~ error.message
+            error!('Security group name contains invalid characters.')
+          elsif /column 'rules'/ =~ error.message
+            error!('Security group rules contain invalid characters.')
+          end
+        end
+
+        raise error
+      end
+
+      def error!(message)
+        raise Error.new(message)
+      end
+
       def valid_spaces(space_guids)
         spaces = Space.where(guid: space_guids).all
         return spaces if spaces.length == space_guids.length
 
         invalid_space_guids = space_guids - spaces.map(&:guid)
         error!("Spaces with guids #{invalid_space_guids} do not exist.")
       end
-
-      def error!(message)
-        raise Error.new(message)
-      end
     end
   end
 end

app/actions/security_group_update.rb

@@ -1,28 +1,56 @@
 module VCAP::CloudController
   class SecurityGroupUpdate
+    MYSQL_INVALID_VALUE_ERROR = 1366
+
     class Error < ::StandardError
     end
 
-    def self.update(security_group, message)
-      security_group.db.transaction do
-        security_group.lock!
+    class << self
+      def update(security_group, message)
+        security_group.db.transaction do
+          security_group.lock!
+
+          security_group.name = message.name if message.requested? :name
+          security_group.rules = message.rules if message.requested? :rules
+
+          security_group.staging_default = message.staging if message.requested?(:globally_enabled) && !message.staging.nil?
+          security_group.running_default = message.running if message.requested?(:globally_enabled) && !message.running.nil?
+
+          security_group.save
+        end
 
-        security_group.name = message.name if message.requested? :name
-        security_group.rules = message.rules if message.requested? :rules
+        security_group
+      rescue Sequel::ValidationFailed => e
+        validation_error!(e, message)
+      rescue Sequel::DatabaseError => e
+        invalid_value_error!(e)
+      end
+
+      private
 
-        security_group.staging_default = message.staging if message.requested?(:globally_enabled) && !message.staging.nil?
-        security_group.running_default = message.running if message.requested?(:globally_enabled) && !message.running.nil?
+      def validation_error!(error, message)
+        if error.errors.on(:name)&.include?(:unique)
+          error!("Security group with name '#{message.name}' already exists.")
+        end
 
-        security_group.save
+        error!(error.message)
       end
 
-      security_group
-    rescue Sequel::ValidationFailed => e
-      if e.errors.on(:name)&.include?(:unique)
-        raise Error.new("Security group with name '#{message.name}' already exists.")
+      def invalid_value_error!(error)
+        if error.wrapped_exception.is_a?(Mysql2::Error) && error.wrapped_exception.error_number == MYSQL_INVALID_VALUE_ERROR
+          if /column 'name'/ =~ error.message
+            error!('Security group name contains invalid characters.')
+          elsif /column 'rules'/ =~ error.message
+            error!('Security group rules contain invalid characters.')
+          end
+        end
+
+        raise error
       end
 
-      raise Error.new(e.message)
+      def error!(message)
+        raise Error.new(message)
+      end
     end
   end
 end

spec/request/security_groups_spec.rb

@@ -143,7 +143,46 @@
         it_behaves_like 'permissions for single object endpoint', ALL_PERMISSIONS
       end
 
-      context 'when a security group with that name already exists' do
+      if ENV['DB'] == 'mysql'
+        context 'when the security group name is invalid' do
+          let(:params) do
+            {
+              name: '🐸🐞'
+            }
+          end
+
+          it 'returns a 422 with a helpful message' do
+            post '/v3/security_groups', params.to_json, admin_header
+
+            expect(last_response).to have_status_code(422)
+            expect(last_response).to have_error_message(
+              'Security group name contains invalid characters.'
+            )
+          end
+        end
+
+        context 'when the security group rules are invalid' do
+          let(:params) do
+            {
+              name: 'bad-rules',
+              rules: [
+                { "protocol": 'all', "destination": '0.0.0.0', "description": 'asd🐞f' }
+              ]
+            }
+          end
+
+          it 'returns a 422 with a helpful message' do
+            post '/v3/security_groups', params.to_json, admin_header
+
+            expect(last_response).to have_status_code(422)
+            expect(last_response).to have_error_message(
+              'Security group rules contain invalid characters.'
+            )
+          end
+        end
+      end
+
+      context 'when a security group with name that already exists' do
         before do
           post '/v3/security_groups', params.to_json, admin_header
         end
@@ -1107,6 +1146,44 @@
         expect(last_response).to have_error_message("Security group with name 'already-taken' already exists")
       end
     end
+
+    if ENV['DB'] == 'mysql'
+      context 'when the security group name is invalid' do
+        let(:params) do
+          {
+            name: '🐸🐞'
+          }
+        end
+
+        it 'returns a 422 with a helpful message' do
+          patch "/v3/security_groups/#{security_group.guid}", params.to_json, admin_header
+
+          expect(last_response).to have_status_code(422)
+          expect(last_response).to have_error_message(
+            'Security group name contains invalid characters.'
+          )
+        end
+      end
+
+      context 'when the security group rules are invalid' do
+        let(:params) do
+          {
+            rules: [
+              { "protocol": 'all', "destination": '0.0.0.0', "description": 'asd🐞f' }
+            ]
+          }
+        end
+
+        it 'returns a 422 with a helpful message' do
+          patch "/v3/security_groups/#{security_group.guid}", params.to_json, admin_header
+
+          expect(last_response).to have_status_code(422)
+          expect(last_response).to have_error_message(
+            'Security group rules contain invalid characters.'
+          )
+        end
+      end
+    end
   end
 
   describe 'DELETE /v3/security_groups/:security_group_guid/relationships/running_spaces/:space_guid' do