From a3fe217ac696273e514609180487eb8fb7139e36 Mon Sep 17 00:00:00 2001
From: Mantas Masalskis <mm@zammad.com>
Date: Fri, 30 Apr 2021 13:36:23 +0000
Subject: [PATCH] Fixes #3492 - POST /api/v1/users/avatar with empty input
 causes "undefined method `substr' for nil:NilClass"

---
 app/controllers/users_controller.rb | 15 +++++++++++++--
 lib/static_assets.rb                |  2 +-
 spec/lib/static_assets_spec.rb      | 13 +++++++++++++
 spec/requests/user_spec.rb          | 24 ++++++++++++++++++++++++
 4 files changed, 51 insertions(+), 3 deletions(-)
 create mode 100644 spec/lib/static_assets_spec.rb

diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb
index 71dd0175f..e8c14f140 100644
--- a/app/controllers/users_controller.rb
+++ b/app/controllers/users_controller.rb
@@ -781,8 +781,19 @@ curl http://localhost/api/v1/users/avatar -v -u #{login}:#{password} -H "Content
 
   def avatar_new
     # get & validate image
-    file_full   = StaticAssets.data_url_attributes(params[:avatar_full])
-    file_resize = StaticAssets.data_url_attributes(params[:avatar_resize])
+    begin
+      file_full = StaticAssets.data_url_attributes(params[:avatar_full])
+    rescue
+      render json: { error: 'Full size image is invalid' }, status: :unprocessable_entity
+      return
+    end
+
+    begin
+      file_resize = StaticAssets.data_url_attributes(params[:avatar_resize])
+    rescue
+      render json: { error: 'Resized image is invalid' }, status: :unprocessable_entity
+      return
+    end
 
     avatar = Avatar.add(
       object:    'User',
diff --git a/lib/static_assets.rb b/lib/static_assets.rb
index 1dcd2d42e..1b27f339a 100644
--- a/lib/static_assets.rb
+++ b/lib/static_assets.rb
@@ -24,7 +24,7 @@ returns
       end
       return data
     end
-    raise "Unable to parse data url: #{data_url.substr(0, 100)}"
+    raise "Unable to parse data url: #{data_url&.slice(0, 100)}"
   end
 
 =begin
diff --git a/spec/lib/static_assets_spec.rb b/spec/lib/static_assets_spec.rb
new file mode 100644
index 000000000..ee9c05f29
--- /dev/null
+++ b/spec/lib/static_assets_spec.rb
@@ -0,0 +1,13 @@
+require 'rails_helper'
+
+RSpec.describe StaticAssets do
+  describe '.data_url_attributes' do
+    it 'raises error if empty string given' do
+      expect { described_class.data_url_attributes('') }.to raise_error(/Unable to parse data url/)
+    end
+
+    it 'raises error if nil' do
+      expect { described_class.data_url_attributes(nil) }.to raise_error(/Unable to parse data url/)
+    end
+  end
+end
diff --git a/spec/requests/user_spec.rb b/spec/requests/user_spec.rb
index b86568887..c14975e54 100644
--- a/spec/requests/user_spec.rb
+++ b/spec/requests/user_spec.rb
@@ -1431,4 +1431,28 @@ RSpec.describe 'User', type: :request do
       end
     end
   end
+
+  describe 'POST /api/v1/users/avatar', authenticated_as: :user do
+    let(:user) { create(:user) }
+    let(:base64) { 'data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAUAAAAFCAYAAACNbyblAAAAHElEQVQI12P4//8/w38GIAXDIBKE0DHxgljNBAAO9TXL0Y4OHwAAAABJRU5ErkJggg==' }
+
+    def make_request(params)
+      post '/api/v1/users/avatar', params: params, as: :json
+    end
+
+    it 'returns verbose error when full image is missing' do
+      make_request(avatar_full: '')
+      expect(json_response).to include('error' => match(/Full/).and(match(/is invalid/)))
+    end
+
+    it 'returns verbose error when resized image is missing' do
+      make_request(avatar_full: base64)
+      expect(json_response).to include('error' => match(/Resized/).and(match(/is invalid/)))
+    end
+
+    it 'successfully changes avatar' do
+      expect { make_request(avatar_full: base64, avatar_resize: base64) }
+        .to change { Avatar.list('User', user.id) }
+    end
+  end
 end