From dfd256f29ee817b5ffc563bb554a02d26ae44502 Mon Sep 17 00:00:00 2001
From: Stan Hu <stanhu@gmail.com>
Date: Fri, 20 Mar 2015 05:11:12 -0700
Subject: [PATCH] Support configurable attachment size via Application Settings
Fix bug where error messages from Dropzone would not be displayed on the issues page
Closes #1258
---
CHANGELOG | 2 +
.../javascripts/dropzone_input.js.coffee | 15 ++++---
.../admin/application_settings_controller.rb | 1 +
app/controllers/application_controller.rb | 1 +
app/models/application_setting.rb | 4 +-
app/models/note.rb | 10 ++++-
app/services/projects/upload_service.rb | 8 +++-
.../application_settings/_form.html.haml | 5 +++
app/views/projects/notes/_form.html.haml | 2 +-
config/initializers/1_settings.rb | 1 +
...attachment_size_to_application_settings.rb | 5 +++
db/schema.rb | 3 +-
features/project/issues/issues.feature | 1 +
features/steps/project/issues/issues.rb | 6 +++
lib/file_size_validator.rb | 12 +++++-
lib/gitlab/current_settings.rb | 3 +-
spec/lib/file_size_validator_spec.rb | 43 +++++++++++++++++++
spec/services/projects/upload_service_spec.rb | 10 +++++
18 files changed, 117 insertions(+), 15 deletions(-)
create mode 100644 db/migrate/20150328132231_add_max_attachment_size_to_application_settings.rb
create mode 100644 spec/lib/file_size_validator_spec.rb
diff --git a/CHANGELOG b/CHANGELOG
index 7cca0835afa..57d0bba1cfe 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,11 +1,13 @@
Please view this file on the master branch, on stable branches it's out of date.
v 7.10.0 (unreleased)
+ - Fix bug where error messages from Dropzone would not be displayed on the issues page (Stan Hu)
- Fix broken side-by-side diff view on merge request page (Stan Hu)
- Set Application controller default URL options to ensure all url_for calls are consistent (Stan Hu)
- Allow HTML tags in Markdown input
- Fix code unfold not working on Compare commits page (Stan Hu)
- Fix dots in Wiki slugs causing errors (Stan Hu)
+ - Make maximum attachment size configurable via Application Settings (Stan Hu)
- Update poltergeist to version 1.6.0 to support PhantomJS 2.0 (Zeger-Jan van de Weg)
- Fix cross references when usernames, milestones, or project names contain underscores (Stan Hu)
- Disable reference creation for comments surrounded by code/preformatted blocks (Stan Hu)
diff --git a/app/assets/javascripts/dropzone_input.js.coffee b/app/assets/javascripts/dropzone_input.js.coffee
index 06e9f0001ae..fca2a290e2d 100644
--- a/app/assets/javascripts/dropzone_input.js.coffee
+++ b/app/assets/javascripts/dropzone_input.js.coffee
@@ -10,6 +10,7 @@ class @DropzoneInput
iconSpinner = "<i class=\"fa fa-spinner fa-spin div-dropzone-icon\"></i>"
btnAlert = "<button type=\"button\"" + alertAttr + ">×</button>"
project_uploads_path = window.project_uploads_path or null
+ max_file_size = gon.max_file_size or 10
form_textarea = $(form).find("textarea.markdown-area")
form_textarea.wrap "<div class=\"div-dropzone\"></div>"
@@ -76,7 +77,7 @@ class @DropzoneInput
dictDefaultMessage: ""
clickable: true
paramName: "file"
- maxFilesize: 10
+ maxFilesize: max_file_size
uploadMultiple: false
headers:
"X-CSRF-Token": $("meta[name=\"csrf-token\"]").attr("content")
@@ -108,9 +109,10 @@ class @DropzoneInput
return
error: (temp, errorMessage) ->
- checkIfMsgExists = $(".error-alert").children().length
+ errorAlert = $(form).find('.error-alert')
+ checkIfMsgExists = errorAlert.children().length
if checkIfMsgExists is 0
- $(".error-alert").append divAlert
+ errorAlert.append divAlert
$(".div-dropzone-alert").append btnAlert + errorMessage
return
@@ -221,9 +223,10 @@ class @DropzoneInput
"display": "none"
showError = (message) ->
- checkIfMsgExists = $(".error-alert").children().length
+ errorAlert = $(form).find('.error-alert')
+ checkIfMsgExists = errorAlert.children().length
if checkIfMsgExists is 0
- $(".error-alert").append divAlert
+ errorAlert.append divAlert
$(".div-dropzone-alert").append btnAlert + message
closeAlertMessage = ->
@@ -237,4 +240,4 @@ class @DropzoneInput
formatLink: (link) ->
text = "[#{link.alt}](#{link.url})"
text = "!#{text}" if link.is_image
- text
\ No newline at end of file
+ text
diff --git a/app/controllers/admin/application_settings_controller.rb b/app/controllers/admin/application_settings_controller.rb
index 9a5685877f8..b5fda196bf0 100644
--- a/app/controllers/admin/application_settings_controller.rb
+++ b/app/controllers/admin/application_settings_controller.rb
@@ -38,6 +38,7 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController
:twitter_sharing_enabled,
:sign_in_text,
:home_page_url,
+ :max_attachment_size,
restricted_visibility_levels: []
)
end
diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 2809f90c0d5..80e983b5314 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -203,6 +203,7 @@ class ApplicationController < ActionController::Base
gon.api_version = API::API.version
gon.relative_url_root = Gitlab.config.gitlab.relative_url_root
gon.default_avatar_url = URI::join(Gitlab.config.gitlab.url, ActionController::Base.helpers.image_path('no_avatar.png')).to_s
+ gon.max_file_size = current_application_settings.max_attachment_size;
if current_user
gon.current_user_id = current_user.id
diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb
index 1c87db613ae..6e98c4c2f02 100644
--- a/app/models/application_setting.rb
+++ b/app/models/application_setting.rb
@@ -16,6 +16,7 @@
# default_branch_protection :integer default(2)
# twitter_sharing_enabled :boolean default(TRUE)
# restricted_visibility_levels :text
+# max_attachment_size :integer default(10)
#
class ApplicationSetting < ActiveRecord::Base
@@ -49,7 +50,8 @@ class ApplicationSetting < ActiveRecord::Base
twitter_sharing_enabled: Settings.gitlab['twitter_sharing_enabled'],
gravatar_enabled: Settings.gravatar['enabled'],
sign_in_text: Settings.extra['sign_in_text'],
- restricted_visibility_levels: Settings.gitlab['restricted_visibility_levels']
+ restricted_visibility_levels: Settings.gitlab['restricted_visibility_levels'],
+ max_attachment_size: Settings.gitlab['max_attachment_size']
)
end
diff --git a/app/models/note.rb b/app/models/note.rb
index e86160e7cd9..fdab4517df3 100644
--- a/app/models/note.rb
+++ b/app/models/note.rb
@@ -22,6 +22,7 @@ require 'file_size_validator'
class Note < ActiveRecord::Base
include Mentionable
+ include Gitlab::CurrentSettings
default_value_for :system, false
@@ -36,7 +37,8 @@ class Note < ActiveRecord::Base
validates :note, :project, presence: true
validates :line_code, format: { with: /\A[a-z0-9]+_\d+_\d+\Z/ }, allow_blank: true
- validates :attachment, file_size: { maximum: 10.megabytes.to_i }
+ # Attachments are deprecated and are handled by Markdown uploader
+ validates :attachment, file_size: { maximum: :max_attachment_size }
validates :noteable_id, presence: true, if: ->(n) { n.noteable_type.present? && n.noteable_type != 'Commit' }
validates :commit_id, presence: true, if: ->(n) { n.noteable_type == 'Commit' }
@@ -321,6 +323,10 @@ class Note < ActiveRecord::Base
end
end
+ def max_attachment_size
+ current_application_settings.max_attachment_size.megabytes.to_i
+ end
+
def commit_author
@commit_author ||=
project.team.users.find_by(email: noteable.author_email) ||
@@ -451,7 +457,7 @@ class Note < ActiveRecord::Base
prev_match_line = line
else
prev_lines << line
-
+
break if generate_line_code(line) == self.line_code
prev_lines.shift if prev_lines.length >= max_number_of_lines
diff --git a/app/services/projects/upload_service.rb b/app/services/projects/upload_service.rb
index a186c97628f..992a7a7a1dc 100644
--- a/app/services/projects/upload_service.rb
+++ b/app/services/projects/upload_service.rb
@@ -5,7 +5,7 @@ module Projects
end
def execute
- return nil unless @file
+ return nil unless @file and @file.size <= max_attachment_size
uploader = FileUploader.new(@project)
uploader.store!(@file)
@@ -18,5 +18,11 @@ module Projects
'is_image' => uploader.image?
}
end
+
+ private
+
+ def max_attachment_size
+ current_application_settings.max_attachment_size.megabytes.to_i
+ end
end
end
diff --git a/app/views/admin/application_settings/_form.html.haml b/app/views/admin/application_settings/_form.html.haml
index edfcccfcf4c..4f3565c67eb 100644
--- a/app/views/admin/application_settings/_form.html.haml
+++ b/app/views/admin/application_settings/_form.html.haml
@@ -60,5 +60,10 @@
.col-sm-10
= f.text_area :sign_in_text, class: 'form-control', rows: 4
.help-block Markdown enabled
+ .form-group
+ = f.label :max_attachment_size, 'Maximum attachment size (MB)', class: 'control-label col-sm-2'
+ .col-sm-10
+ = f.number_field :max_attachment_size, class: 'form-control'
+
.form-actions
= f.submit 'Save', class: 'btn btn-primary'
diff --git a/app/views/projects/notes/_form.html.haml b/app/views/projects/notes/_form.html.haml
index be96c302143..2ada6cb6700 100644
--- a/app/views/projects/notes/_form.html.haml
+++ b/app/views/projects/notes/_form.html.haml
@@ -12,7 +12,7 @@
.comment-hints.clearfix
.pull-left Comments are parsed with #{link_to "GitLab Flavored Markdown", help_page_path("markdown", "markdown"),{ target: '_blank', tabindex: -1 }}
.pull-right Attach files by dragging & dropping or #{link_to "selecting them", '#', class: 'markdown-selector', tabindex: -1 }.
-
+ .error-alert
.note-form-actions
.buttons
diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb
index 15c1ae9466f..4c37fbf7cab 100644
--- a/config/initializers/1_settings.rb
+++ b/config/initializers/1_settings.rb
@@ -119,6 +119,7 @@ Settings.gitlab['username_changing_enabled'] = true if Settings.gitlab['username
Settings.gitlab['issue_closing_pattern'] = '((?:[Cc]los(?:e[sd]?|ing)|[Ff]ix(?:e[sd]|ing)?|[Rr]esolv(?:e[sd]?|ing)) +(?:(?:issues? +)?#\d+(?:(?:, *| +and +)?))+)' if Settings.gitlab['issue_closing_pattern'].nil?
Settings.gitlab['default_projects_features'] ||= {}
Settings.gitlab['webhook_timeout'] ||= 10
+Settings.gitlab['max_attachment_size'] ||= 10
Settings.gitlab.default_projects_features['issues'] = true if Settings.gitlab.default_projects_features['issues'].nil?
Settings.gitlab.default_projects_features['merge_requests'] = true if Settings.gitlab.default_projects_features['merge_requests'].nil?
Settings.gitlab.default_projects_features['wiki'] = true if Settings.gitlab.default_projects_features['wiki'].nil?
diff --git a/db/migrate/20150328132231_add_max_attachment_size_to_application_settings.rb b/db/migrate/20150328132231_add_max_attachment_size_to_application_settings.rb
new file mode 100644
index 00000000000..1d161674a9a
--- /dev/null
+++ b/db/migrate/20150328132231_add_max_attachment_size_to_application_settings.rb
@@ -0,0 +1,5 @@
+class AddMaxAttachmentSizeToApplicationSettings < ActiveRecord::Migration
+ def change
+ add_column :application_settings, :max_attachment_size, :integer, default: 10, null: false
+ end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 4a445ae5832..14e32a7946e 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -11,7 +11,7 @@
#
# It's strongly recommended that you check this file into your version control system.
-ActiveRecord::Schema.define(version: 20150324155957) do
+ActiveRecord::Schema.define(version: 20150328132231) do
# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"
@@ -28,6 +28,7 @@ ActiveRecord::Schema.define(version: 20150324155957) do
t.integer "default_branch_protection", default: 2
t.boolean "twitter_sharing_enabled", default: true
t.text "restricted_visibility_levels"
+ t.integer "max_attachment_size", default: 10, null: false
end
create_table "broadcast_messages", force: true do |t|
diff --git a/features/project/issues/issues.feature b/features/project/issues/issues.feature
index b9031f6f32b..af01c7058ea 100644
--- a/features/project/issues/issues.feature
+++ b/features/project/issues/issues.feature
@@ -42,6 +42,7 @@ Feature: Project Issues
Given I visit issue page "Release 0.4"
And I leave a comment like "XML attached"
Then I should see comment "XML attached"
+ And I should see an error alert section within the comment form
@javascript
Scenario: I search issue
diff --git a/features/steps/project/issues/issues.rb b/features/steps/project/issues/issues.rb
index e8ca3f7c176..e2834d51a27 100644
--- a/features/steps/project/issues/issues.rb
+++ b/features/steps/project/issues/issues.rb
@@ -204,6 +204,12 @@ class Spinach::Features::ProjectIssues < Spinach::FeatureSteps
end
end
+ step 'I should see an error alert section within the comment form' do
+ within(".js-main-target-form") do
+ find(".error-alert")
+ end
+ end
+
step 'The code block should be unchanged' do
page.should have_content("```\nCommand [1]: /usr/local/bin/git , see [text](doc/text)\n```")
end
diff --git a/lib/file_size_validator.rb b/lib/file_size_validator.rb
index 42970c1be59..2eae55e534b 100644
--- a/lib/file_size_validator.rb
+++ b/lib/file_size_validator.rb
@@ -25,8 +25,8 @@ class FileSizeValidator < ActiveModel::EachValidator
keys.each do |key|
value = options[key]
- unless value.is_a?(Integer) && value >= 0
- raise ArgumentError, ":#{key} must be a nonnegative Integer"
+ unless (value.is_a?(Integer) && value >= 0) || value.is_a?(Symbol)
+ raise ArgumentError, ":#{key} must be a nonnegative Integer or symbol"
end
end
end
@@ -39,6 +39,14 @@ class FileSizeValidator < ActiveModel::EachValidator
CHECKS.each do |key, validity_check|
next unless check_value = options[key]
+ check_value =
+ case check_value
+ when Integer
+ check_value
+ when Symbol
+ record.send(check_value)
+ end
+
value ||= [] if key == :maximum
value_size = value.size
diff --git a/lib/gitlab/current_settings.rb b/lib/gitlab/current_settings.rb
index 0ebebfa09c4..d8f696d247b 100644
--- a/lib/gitlab/current_settings.rb
+++ b/lib/gitlab/current_settings.rb
@@ -20,7 +20,8 @@ module Gitlab
signin_enabled: Settings.gitlab['signin_enabled'],
gravatar_enabled: Settings.gravatar['enabled'],
sign_in_text: Settings.extra['sign_in_text'],
- restricted_visibility_levels: Settings.gitlab['restricted_visibility_levels']
+ restricted_visibility_levels: Settings.gitlab['restricted_visibility_levels'],
+ max_attachment_size: Settings.gitlab['max_attachment_size']
)
end
end
diff --git a/spec/lib/file_size_validator_spec.rb b/spec/lib/file_size_validator_spec.rb
new file mode 100644
index 00000000000..5c89c854714
--- /dev/null
+++ b/spec/lib/file_size_validator_spec.rb
@@ -0,0 +1,43 @@
+require 'spec_helper'
+
+describe 'Gitlab::FileSizeValidatorSpec' do
+ let(:validator) { FileSizeValidator.new(options) }
+ let(:attachment) { AttachmentUploader.new }
+ let(:note) { create(:note) }
+
+ describe 'options uses an integer' do
+ let(:options) { { maximum: 10, attributes: { attachment: attachment } } }
+
+ it 'attachment exceeds maximum limit' do
+ allow(attachment).to receive(:size) { 100 }
+ validator.validate_each(note, :attachment, attachment)
+ expect(note.errors).to have_key(:attachment)
+ end
+
+ it 'attachment under maximum limit' do
+ allow(attachment).to receive(:size) { 1 }
+ validator.validate_each(note, :attachment, attachment)
+ expect(note.errors).not_to have_key(:attachment)
+ end
+ end
+
+ describe 'options uses a symbol' do
+ let(:options) { { maximum: :test,
+ attributes: { attachment: attachment } } }
+ before do
+ allow(note).to receive(:test) { 10 }
+ end
+
+ it 'attachment exceeds maximum limit' do
+ allow(attachment).to receive(:size) { 100 }
+ validator.validate_each(note, :attachment, attachment)
+ expect(note.errors).to have_key(:attachment)
+ end
+
+ it 'attachment under maximum limit' do
+ allow(attachment).to receive(:size) { 1 }
+ validator.validate_each(note, :attachment, attachment)
+ expect(note.errors).not_to have_key(:attachment)
+ end
+ end
+end
diff --git a/spec/services/projects/upload_service_spec.rb b/spec/services/projects/upload_service_spec.rb
index fc34b456482..e5c47015a03 100644
--- a/spec/services/projects/upload_service_spec.rb
+++ b/spec/services/projects/upload_service_spec.rb
@@ -67,6 +67,16 @@ describe Projects::UploadService do
it { expect(@link_to_file['url']).to match("/#{@project.path_with_namespace}") }
it { expect(@link_to_file['url']).to match('doc_sample.txt') }
end
+
+ context 'for too large a file' do
+ before do
+ txt = fixture_file_upload(Rails.root + 'spec/fixtures/doc_sample.txt', 'text/plain')
+ allow(txt).to receive(:size) { 1000.megabytes.to_i }
+ @link_to_file = upload_file(@project.repository, txt)
+ end
+
+ it { expect(@link_to_file).to eq(nil) }
+ end
end
def upload_file(repository, file)
--
GitLab