diff options
27 files changed, 270 insertions, 281 deletions
diff --git a/CHANGELOG b/CHANGELOG index a9ee816b37d..e3caebcf0a2 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -4,6 +4,8 @@ v 7.9.0 (unreleased) v 7.8.0 (unreleased) - Fix access control and protection against XSS for note attachments and other uploads. + - Fix broken access control for note attachments (Hannes Rosenögger) + - Generalize image upload in drag and drop in markdown to all files (Hannes Rosenögger) - Replace highlight.js with rouge-fork rugments (Stefan Tatschner) - Make project search case insensitive (Hannes Rosenögger) - Include issue/mr participants in list of recipients for reassign/close/reopen emails @@ -205,6 +205,7 @@ group :development do gem "letter_opener" gem 'quiet_assets', '~> 1.0.1' gem 'rack-mini-profiler', require: false + gem "byebug" # Better errors handler gem 'better_errors' diff --git a/Gemfile.lock b/Gemfile.lock index a9784f36ac9..b8cd998e6a9 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -65,6 +65,9 @@ GEM sass (>= 3.2.19) browser (0.7.2) builder (3.2.2) + byebug (3.2.0) + columnize (~> 0.8) + debugger-linecache (~> 1.2) cal-heatmap-rails (0.0.1) capybara (2.2.1) mime-types (>= 1.16) @@ -92,6 +95,7 @@ GEM coffee-script-source (1.6.3) colored (1.2) colorize (0.5.8) + columnize (0.9.0) connection_pool (2.1.0) coveralls (0.7.0) multi_json (~> 1.3) @@ -107,6 +111,7 @@ GEM daemons (1.1.9) database_cleaner (1.3.0) debug_inspector (0.0.2) + debugger-linecache (1.2.0) default_value_for (3.0.0) activerecord (>= 3.2.0, < 5.0) descendants_tracker (0.0.3) @@ -650,6 +655,7 @@ DEPENDENCIES binding_of_caller bootstrap-sass (~> 3.0) browser + byebug cal-heatmap-rails (~> 0.0.1) capybara (~> 2.2.1) carrierwave diff --git a/app/assets/javascripts/dropzone_input.js.coffee b/app/assets/javascripts/dropzone_input.js.coffee index d98d5482937..06e9f0001ae 100644 --- a/app/assets/javascripts/dropzone_input.js.coffee +++ b/app/assets/javascripts/dropzone_input.js.coffee @@ -6,10 +6,10 @@ class @DropzoneInput divHover = "<div class=\"div-dropzone-hover\"></div>" divSpinner = "<div class=\"div-dropzone-spinner\"></div>" divAlert = "<div class=\"" + alertClass + "\"></div>" - iconPicture = "<i class=\"fa fa-picture-o div-dropzone-icon\"></i>" + iconPaperclip = "<i class=\"fa fa-paperclip div-dropzone-icon\"></i>" iconSpinner = "<i class=\"fa fa-spinner fa-spin div-dropzone-icon\"></i>" btnAlert = "<button type=\"button\"" + alertAttr + ">×</button>" - project_image_path_upload = window.project_image_path_upload or null + project_uploads_path = window.project_uploads_path or null form_textarea = $(form).find("textarea.markdown-area") form_textarea.wrap "<div class=\"div-dropzone\"></div>" @@ -19,7 +19,7 @@ class @DropzoneInput form_dropzone = $(form).find('.div-dropzone') form_dropzone.parent().addClass "div-dropzone-wrapper" form_dropzone.append divHover - $(".div-dropzone-hover").append iconPicture + $(".div-dropzone-hover").append iconPaperclip form_dropzone.append divSpinner $(".div-dropzone-spinner").append iconSpinner $(".div-dropzone-spinner").css @@ -72,13 +72,12 @@ class @DropzoneInput form.find(".md-preview-holder").hide() dropzone = form_dropzone.dropzone( - url: project_image_path_upload + url: project_uploads_path dictDefaultMessage: "" clickable: true - paramName: "markdown_img" + paramName: "file" maxFilesize: 10 uploadMultiple: false - acceptedFiles: "image/jpg,image/jpeg,image/gif,image/png" headers: "X-CSRF-Token": $("meta[name=\"csrf-token\"]").attr("content") @@ -132,8 +131,10 @@ class @DropzoneInput child = $(dropzone[0]).children("textarea") - formatLink = (str) -> - "![" + str.alt + "](" + str.url + ")" + formatLink = (link) -> + text = "[#{link.alt}](#{link.url})" + text = "!#{text}" if link.is_image + text handlePaste = (event) -> pasteEvent = event.originalEvent @@ -177,9 +178,9 @@ class @DropzoneInput uploadFile = (item, filename) -> formData = new FormData() - formData.append "markdown_img", item, filename + formData.append "file", item, filename $.ajax - url: project_image_path_upload + url: project_uploads_path type: "POST" data: formData dataType: "json" @@ -233,5 +234,7 @@ class @DropzoneInput $(@).closest('.gfm-form').find('.div-dropzone').click() return - formatLink: (str) -> - "![" + str.alt + "](" + str.url + ")" + formatLink: (link) -> + text = "[#{link.alt}](#{link.url})" + text = "!#{text}" if link.is_image + text
\ No newline at end of file diff --git a/app/assets/javascripts/notes.js.coffee b/app/assets/javascripts/notes.js.coffee index 1c090bd06dc..90e6fd6d154 100644 --- a/app/assets/javascripts/notes.js.coffee +++ b/app/assets/javascripts/notes.js.coffee @@ -39,9 +39,6 @@ class @Notes # reset main target form after submit $(document).on "ajax:complete", ".js-main-target-form", @resetMainTargetForm - # attachment button - $(document).on "click", ".js-choose-note-attachment-button", @chooseNoteAttachment - # update the file name when an attachment is selected $(document).on "change", ".js-note-attachment-input", @updateFormAttachment @@ -73,7 +70,6 @@ class @Notes $(document).off "click", ".js-note-delete" $(document).off "click", ".js-note-attachment-delete" $(document).off "ajax:complete", ".js-main-target-form" - $(document).off "click", ".js-choose-note-attachment-button" $(document).off "click", ".js-discussion-reply-button" $(document).off "click", ".js-add-diff-note-button" $(document).off "visibilitychange" @@ -174,15 +170,6 @@ class @Notes form.find(".js-note-text").data("autosave").reset() ### - Called when clicking the "Choose File" button. - - Opens the file selection dialog. - ### - chooseNoteAttachment: -> - form = $(this).closest("form") - form.find(".js-note-attachment-input").click() - - ### Shows the main form and does some setup on it. Sets some hidden fields in the form. diff --git a/app/assets/stylesheets/sections/notes.scss b/app/assets/stylesheets/sections/notes.scss index 5494845eb8c..40adc8b3ba7 100644 --- a/app/assets/stylesheets/sections/notes.scss +++ b/app/assets/stylesheets/sections/notes.scss @@ -66,6 +66,22 @@ ul.notes { overflow: auto; word-wrap: break-word; @include md-typography; + + a[href*="/uploads/"] { + &:before { + margin-right: 4px; + + font: normal normal normal 14px/1 FontAwesome; + font-size: inherit; + text-rendering: auto; + -webkit-font-smoothing: antialiased; + content: "\f0c6"; + } + + &:hover:before { + text-decoration: none; + } + } } } .note-header { diff --git a/app/controllers/files_controller.rb b/app/controllers/files_controller.rb deleted file mode 100644 index 9671245d3f4..00000000000 --- a/app/controllers/files_controller.rb +++ /dev/null @@ -1,17 +0,0 @@ -class FilesController < ApplicationController - def download - note = Note.find(params[:id]) - uploader = note.attachment - - if uploader.file_storage? - if can?(current_user, :read_project, note.project) - disposition = uploader.image? ? 'inline' : 'attachment' - send_file uploader.file.path, disposition: disposition - else - not_found! - end - else - redirect_to uploader.url - end - end -end diff --git a/app/controllers/projects/uploads_controller.rb b/app/controllers/projects/uploads_controller.rb index 2b4da35bc7f..53b92d8643d 100644 --- a/app/controllers/projects/uploads_controller.rb +++ b/app/controllers/projects/uploads_controller.rb @@ -1,19 +1,35 @@ class Projects::UploadsController < Projects::ApplicationController - layout "project" + layout 'project' before_filter :project + def create + link_to_file = ::Projects::UploadService.new(repository, params[:file]). + execute + + respond_to do |format| + if link_to_file + format.json do + render json: { link: link_to_file } + end + else + format.json do + render json: 'Invalid file.', status: :unprocessable_entity + end + end + end + end + def show - path = File.join(project.path_with_namespace, params[:secret]) - uploader = FileUploader.new('uploads', path) + uploader = FileUploader.new(project, params[:secret]) + + return redirect_to uploader.url unless uploader.file_storage? uploader.retrieve_from_store!(params[:filename]) - if uploader.file.exists? - # Right now, these are always images, so we can safely render them inline. - send_file uploader.file.path, disposition: 'inline' - else - not_found! - end + return not_found! unless uploader.file.exists? + + disposition = uploader.image? ? 'inline' : 'attachment' + send_file uploader.file.path, disposition: disposition end end diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index b0fde88babc..74e188e484d 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -137,18 +137,6 @@ class ProjectsController < ApplicationController end end - def upload_image - link_to_image = ::Projects::ImageService.new(repository, params, root_url).execute - - respond_to do |format| - if link_to_image - format.json { render json: { link: link_to_image } } - else - format.json { render json: 'Invalid file.', status: :unprocessable_entity } - end - end - end - def toggle_star current_user.toggle_star(@project) @project.reload @@ -161,15 +149,6 @@ class ProjectsController < ApplicationController private - def upload_path - base_dir = FileUploader.generate_dir - File.join(repository.path_with_namespace, base_dir) - end - - def accepted_images - %w(png jpg jpeg gif) - end - def set_title @title = 'New Project' end diff --git a/app/controllers/uploads_controller.rb b/app/controllers/uploads_controller.rb index d5877977258..508c2a6221a 100644 --- a/app/controllers/uploads_controller.rb +++ b/app/controllers/uploads_controller.rb @@ -3,15 +3,13 @@ class UploadsController < ApplicationController model = params[:model].camelize.constantize.find(params[:id]) uploader = model.send(params[:mounted_as]) - if uploader.file_storage? - if !model.respond_to?(:project) || can?(current_user, :read_project, model.project) - disposition = uploader.image? ? 'inline' : 'attachment' - send_file uploader.file.path, disposition: disposition - else - not_found! - end - else - redirect_to uploader.url - end + return not_found! if model.respond_to?(:project) && !can?(current_user, :read_project, model.project) + + return redirect_to uploader.url unless uploader.file_storage? + + return not_found! unless uploader.file.exists? + + disposition = uploader.image? ? 'inline' : 'attachment' + send_file uploader.file.path, disposition: disposition end end diff --git a/app/models/user.rb b/app/models/user.rb index a723b1289b6..13d4eae0044 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -177,7 +177,7 @@ class User < ActiveRecord::Base end end - mount_uploader :avatar, AvatarUplaoder + mount_uploader :avatar, AvatarUploader # Scopes scope :admins, -> { where(admin: true) } diff --git a/app/services/projects/image_service.rb b/app/services/projects/image_service.rb deleted file mode 100644 index 7ca7e82c4a3..00000000000 --- a/app/services/projects/image_service.rb +++ /dev/null @@ -1,39 +0,0 @@ -module Projects - class ImageService < BaseService - include Rails.application.routes.url_helpers - def initialize(repository, params, root_url) - @repository, @params, @root_url = repository, params.dup, root_url - end - - def execute - uploader = FileUploader.new('uploads', upload_path, accepted_images) - image = @params['markdown_img'] - - if image && correct_mime_type?(image) - alt = image.original_filename - uploader.store!(image) - link = { - 'alt' => File.basename(alt, '.*'), - 'url' => File.join(@root_url, uploader.url) - } - else - link = nil - end - end - - protected - - def upload_path - base_dir = FileUploader.generate_dir - File.join(@repository.path_with_namespace, base_dir) - end - - def accepted_images - %w(png jpg jpeg gif) - end - - def correct_mime_type?(image) - accepted_images.map{ |format| image.content_type.include? format }.any? - end - end -end diff --git a/app/services/projects/upload_service.rb b/app/services/projects/upload_service.rb new file mode 100644 index 00000000000..b2466b52ad9 --- /dev/null +++ b/app/services/projects/upload_service.rb @@ -0,0 +1,23 @@ +module Projects + class UploadService < BaseService + include Rails.application.routes.url_helpers + def initialize(project, file) + @project, @file = project, file + end + + def execute + return nil unless @file + + uploader = FileUploader.new(@project) + uploader.store!(@file) + + filename = uploader.image? ? uploader.file.basename : uploader.file.filename + + { + 'alt' => filename, + 'url' => project_upload_url(@project, secret: uploader.secret, filename: uploader.file.filename), + 'is_image' => uploader.image? + } + end + end +end diff --git a/app/uploaders/file_uploader.rb b/app/uploaders/file_uploader.rb index 0fa987c93f6..36a28f93c49 100644 --- a/app/uploaders/file_uploader.rb +++ b/app/uploaders/file_uploader.rb @@ -2,40 +2,43 @@ class FileUploader < CarrierWave::Uploader::Base storage :file - def initialize(base_dir, path = '', allowed_extensions = nil) - @base_dir = base_dir - @path = path - @allowed_extensions = allowed_extensions + attr_accessor :project, :secret + + def initialize(project, secret = self.class.generate_secret) + @project = project + @secret = secret end def base_dir - @base_dir + "uploads" end def store_dir - File.join(@base_dir, @path) + File.join(base_dir, @project.path_with_namespace, @secret) end def cache_dir - File.join(@base_dir, 'tmp', @path) - end - - def extension_white_list - @allowed_extensions + File.join(base_dir, 'tmp', @project.path_with_namespace, @secret) end - def store!(file) - @filename = self.class.generate_filename(file) - super + def self.generate_secret + SecureRandom.hex end - def self.generate_filename(file) - original_filename = File.basename(file.original_filename, '.*') - extension = File.extname(file.original_filename) - new_filename = Digest::MD5.hexdigest(original_filename) + extension + def file_storage? + self.class.storage == CarrierWave::Storage::File end - def self.generate_dir - SecureRandom.hex(5) + def image? + img_ext = %w(png jpg jpeg gif bmp tiff) + if file.respond_to?(:extension) + img_ext.include?(file.extension.downcase) + else + # Not all CarrierWave storages respond to :extension + ext = file.path.split('.').last.downcase + img_ext.include?(ext) + end + rescue + false end end diff --git a/app/views/projects/_issuable_form.html.haml b/app/views/projects/_issuable_form.html.haml index 5a57673b584..18897b055aa 100644 --- a/app/views/projects/_issuable_form.html.haml +++ b/app/views/projects/_issuable_form.html.haml @@ -23,7 +23,7 @@ Parsed with #{link_to 'GitLab Flavored Markdown', help_page_path('markdown', 'markdown'), target: '_blank'}. .pull-right - Attach images (JPG, PNG, GIF) by dragging & dropping + Attach files by dragging & dropping or #{link_to 'selecting them', '#', class: 'markdown-selector' }. .clearfix diff --git a/app/views/projects/issues/_form.html.haml b/app/views/projects/issues/_form.html.haml index 2a7b44955cd..afeeed6edf4 100644 --- a/app/views/projects/issues/_form.html.haml +++ b/app/views/projects/issues/_form.html.haml @@ -11,4 +11,4 @@ e.preventDefault(); }); - window.project_image_path_upload = "#{upload_image_project_path @project}"; + window.project_uploads_path = "#{project_uploads_path @project}"; diff --git a/app/views/projects/merge_requests/_form.html.haml b/app/views/projects/merge_requests/_form.html.haml index d52e64666a0..c1a05e4586f 100644 --- a/app/views/projects/merge_requests/_form.html.haml +++ b/app/views/projects/merge_requests/_form.html.haml @@ -9,4 +9,4 @@ e.preventDefault(); }); - window.project_image_path_upload = "#{upload_image_project_path @project}"; + window.project_uploads_path = "#{project_uploads_path @project}"; diff --git a/app/views/projects/merge_requests/_new_submit.html.haml b/app/views/projects/merge_requests/_new_submit.html.haml index bca3e45bf19..4cf2a05b1a3 100644 --- a/app/views/projects/merge_requests/_new_submit.html.haml +++ b/app/views/projects/merge_requests/_new_submit.html.haml @@ -27,7 +27,7 @@ Parsed with #{link_to 'Gitlab Flavored Markdown', help_page_path('markdown', 'markdown'), target: '_blank'}. .pull-right - Attach images (JPG, PNG, GIF) by dragging & dropping + Attach files by dragging & dropping or #{link_to 'selecting them', '#', class: 'markdown-selector'}. .clearfix @@ -113,10 +113,11 @@ e.preventDefault(); }); - window.project_image_path_upload = "#{upload_image_project_path @project}"; + window.project_uploads_path = "#{project_uploads_path @project}"; :javascript var merge_request merge_request = new MergeRequest({ action: 'commits' }); + diff --git a/app/views/projects/milestones/_form.html.haml b/app/views/projects/milestones/_form.html.haml index b3b170d7114..dbcd23eee05 100644 --- a/app/views/projects/milestones/_form.html.haml +++ b/app/views/projects/milestones/_form.html.haml @@ -25,7 +25,7 @@ = render 'projects/zen', f: f, attr: :description, classes: 'description form-control' .hint .pull-left Milestones are parsed with #{link_to "GitLab Flavored Markdown", help_page_path("markdown", "markdown"), target: '_blank'}. - .pull-left Attach images (JPG, PNG, GIF) by dragging & dropping or #{link_to "selecting them", '#', class: 'markdown-selector' }. + .pull-left Attach files by dragging & dropping or #{link_to "selecting them", '#', class: 'markdown-selector' }. .clearfix .error-alert .col-md-6 @@ -51,4 +51,4 @@ onSelect: function(dateText, inst) { $("#milestone_due_date").val(dateText) } }).datepicker("setDate", $.datepicker.parseDate('yy-mm-dd', $('#milestone_due_date').val())); - window.project_image_path_upload = "#{upload_image_project_path @project}"; + window.project_uploads_path = "#{project_uploads_path @project}"; diff --git a/app/views/projects/notes/_edit_form.html.haml b/app/views/projects/notes/_edit_form.html.haml index cdc76f5d96f..ca097f3d55a 100644 --- a/app/views/projects/notes/_edit_form.html.haml +++ b/app/views/projects/notes/_edit_form.html.haml @@ -6,17 +6,9 @@ .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 images (JPG, PNG, GIF) by dragging & dropping or #{link_to "selecting them", '#', class: 'markdown-selector', tabindex: -1 }. + .pull-right Attach files by dragging & dropping or #{link_to "selecting them", '#', class: 'markdown-selector', tabindex: -1 }. .note-form-actions .buttons = f.submit 'Save Comment', class: "btn btn-primary btn-save btn-grouped js-comment-button" - = link_to 'Cancel', "#", class: "btn btn-cancel note-edit-cancel" - - .note-form-option.hidden-xs - %a.choose-btn.btn.js-choose-note-attachment-button - %i.fa.fa-paperclip - %span Choose File ... - - %span.file_name.js-attachment-filename - = f.file_field :attachment, class: "js-note-attachment-input hidden" + = link_to 'Cancel', "#", class: "btn btn-cancel note-edit-cancel"
\ No newline at end of file diff --git a/app/views/projects/notes/_form.html.haml b/app/views/projects/notes/_form.html.haml index 1a4e06289f8..8b331ef819f 100644 --- a/app/views/projects/notes/_form.html.haml +++ b/app/views/projects/notes/_form.html.haml @@ -1,4 +1,4 @@ -= form_for [@project, @note], remote: true, html: { :'data-type' => 'json', multipart: true, id: nil, class: "new_note js-new-note-form common-note-form gfm-form" }, authenticity_token: true do |f| += form_for [@project, @note], remote: true, html: { :'data-type' => 'json', multipart: true, id: nil, class: "new_note js-new-note-form common-note-form" }, authenticity_token: true do |f| = note_target_fields = f.hidden_field :commit_id = f.hidden_field :line_code @@ -11,7 +11,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 images (JPG, PNG, GIF) by dragging & dropping or #{link_to "selecting them", '#', class: 'markdown-selector', tabindex: -1 }. + .pull-right Attach files by dragging & dropping or #{link_to "selecting them", '#', class: 'markdown-selector', tabindex: -1 }. .note-form-actions @@ -20,13 +20,5 @@ = yield(:note_actions) %a.btn.grouped.js-close-discussion-note-form Cancel - .note-form-option.hidden-xs - %a.choose-btn.btn.js-choose-note-attachment-button - %i.fa.fa-paperclip - %span Choose File ... - - %span.file_name.js-attachment-filename - = f.file_field :attachment, class: "js-note-attachment-input hidden" - :javascript - window.project_image_path_upload = "#{upload_image_project_path @project}"; + window.project_uploads_path = "#{project_uploads_path @project}"; diff --git a/app/views/projects/wikis/_form.html.haml b/app/views/projects/wikis/_form.html.haml index 84731e43e95..b1579878ed1 100644 --- a/app/views/projects/wikis/_form.html.haml +++ b/app/views/projects/wikis/_form.html.haml @@ -26,7 +26,7 @@ = render 'projects/zen', f: f, attr: :content, classes: 'description form-control' .col-sm-12.hint .pull-left Wiki content is parsed with #{link_to "GitLab Flavored Markdown", help_page_path("markdown", "markdown"), target: '_blank'} - .pull-right Attach images (JPG, PNG, GIF) by dragging & dropping or #{link_to "selecting them", '#', class: 'markdown-selector' }. + .pull-right Attach files by dragging & dropping or #{link_to "selecting them", '#', class: 'markdown-selector' }. .clearfix .error-alert @@ -43,5 +43,6 @@ = link_to "Cancel", project_wiki_path(@project, :home), class: "btn btn-cancel" :javascript - window.project_image_path_upload = "#{upload_image_project_path @project}"; + window.project_uploads_path = "#{project_uploads_path @project}"; + diff --git a/config/routes.rb b/config/routes.rb index ca56c2d268e..498716b12e0 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -103,11 +103,6 @@ Gitlab::Application.routes.draw do get 'public/projects' => 'explore/projects#index' # - # Attachments serving - # - get 'files/:type/:id/:filename' => 'files#download', constraints: { id: /\d+/, type: /[a-z]+/, filename: /.+/ } - - # # Admin Area # namespace :admin do @@ -232,7 +227,6 @@ Gitlab::Application.routes.draw do put :transfer post :archive post :unarchive - post :upload_image post :toggle_star post :markdown_preview get :autocomplete_sources @@ -268,6 +262,12 @@ Gitlab::Application.routes.draw do end end + resources :uploads, only: [:create] do + collection do + get ":secret/:filename", action: :show, constraints: { filename: /.+/ } + end + end + get '/compare/:from...:to' => 'compare#show', :as => 'compare', :constraints => { from: /.+/, to: /.+/ } diff --git a/spec/controllers/projects/uploads_controller_spec.rb b/spec/controllers/projects/uploads_controller_spec.rb new file mode 100644 index 00000000000..8c99b5ca528 --- /dev/null +++ b/spec/controllers/projects/uploads_controller_spec.rb @@ -0,0 +1,49 @@ +require('spec_helper') + +describe Projects::UploadsController do + let(:project) { create(:project) } + let(:user) { create(:user) } + let(:jpg) { fixture_file_upload(Rails.root + 'spec/fixtures/rails_sample.jpg', 'image/jpg') } + let(:txt) { fixture_file_upload(Rails.root + 'spec/fixtures/doc_sample.txt', 'text/plain') } + + describe 'POST #create' do + before do + sign_in(user) + project.team << [user, :developer] + end + + context "without params['file']" do + it 'returns an error' do + post :create, project_id: project.to_param, format: :json + expect(response.status).to eq(422) + end + end + + context 'with valid image' do + before do + post :create, + project_id: project.to_param, + file: jpg, + format: :json + end + + it 'returns a content with original filename, new link, and correct type.' do + expect(response.body).to match '\"alt\":\"rails_sample\"' + expect(response.body).to match "\"url\":\"/#{project.path_with_namespace}/uploads" + expect(response.body).to match '\"is_image\":true' + end + end + + context 'with valid non-image file' do + before do + post :create, project_id: project.to_param, file: txt, format: :json + end + + it 'returns a content with original filename, new link, and correct type.' do + expect(response.body).to match '\"alt\":\"doc_sample.txt\"' + expect(response.body).to match "\"url\":\"/#{project.path_with_namespace}/uploads" + expect(response.body).to match '\"is_image\":false' + end + end + end +end diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb index ef786ccd324..9be4c2e505c 100644 --- a/spec/controllers/projects_controller_spec.rb +++ b/spec/controllers/projects_controller_spec.rb @@ -4,46 +4,9 @@ describe ProjectsController do let(:project) { create(:project) } let(:public_project) { create(:project, :public) } let(:user) { create(:user) } - let(:jpg) { fixture_file_upload(Rails.root + 'spec/fixtures/rails_sample.jpg', 'image/jpg') } - let(:txt) { fixture_file_upload(Rails.root + 'spec/fixtures/doc_sample.txt', 'text/plain') } - - describe "POST #upload_image" do - before do - sign_in(user) - project.team << [user, :developer] - end - - context "without params['markdown_img']" do - it "returns an error" do - post :upload_image, id: project.to_param, format: :json - expect(response.status).to eq(422) - end - end - - context "with invalid file" do - before do - post :upload_image, id: project.to_param, markdown_img: txt, format: :json - end - - it "returns an error" do - expect(response.status).to eq(422) - end - end - - context "with valid file" do - before do - post :upload_image, id: project.to_param, markdown_img: jpg, format: :json - end - - it "returns a content with original filename and new link." do - expect(response.body).to match "\"alt\":\"rails_sample\"" - expect(response.body).to match "\"url\":\"http://test.host/uploads/#{project.path_with_namespace}" - end - end - end - - describe "POST #toggle_star" do - it "toggles star if user is signed in" do + + describe 'POST #toggle_star' do + it 'toggles star if user is signed in' do sign_in(user) expect(user.starred?(public_project)).to be_falsey post :toggle_star, id: public_project.to_param @@ -52,7 +15,7 @@ describe ProjectsController do expect(user.starred?(public_project)).to be_falsey end - it "does nothing if user is not signed in" do + it 'does nothing if user is not signed in' do post :toggle_star, id: public_project.to_param expect(user.starred?(public_project)).to be_falsey post :toggle_star, id: public_project.to_param diff --git a/spec/services/projects/image_service_spec.rb b/spec/services/projects/image_service_spec.rb deleted file mode 100644 index 23c4e227ae3..00000000000 --- a/spec/services/projects/image_service_spec.rb +++ /dev/null @@ -1,62 +0,0 @@ -require 'spec_helper' - -describe Projects::ImageService do - describe 'Image service' do - before do - @user = create :user - @project = create :project, creator_id: @user.id, namespace: @user.namespace - end - - context 'for valid gif file' do - before do - gif = fixture_file_upload(Rails.root + 'spec/fixtures/banana_sample.gif', 'image/gif') - @link_to_image = upload_image(@project.repository, { 'markdown_img' => gif }, "http://test.example/") - end - - it { expect(@link_to_image).to have_key("alt") } - it { expect(@link_to_image).to have_key("url") } - it { expect(@link_to_image).to have_value("banana_sample") } - it { expect(@link_to_image["url"]).to match("http://test.example/uploads/#{@project.path_with_namespace}") } - it { expect(@link_to_image["url"]).to match("banana_sample.gif") } - end - - context 'for valid png file' do - before do - png = fixture_file_upload(Rails.root + 'spec/fixtures/dk.png', 'image/png') - @link_to_image = upload_image(@project.repository, { 'markdown_img' => png }, "http://test.example/") - end - - it { expect(@link_to_image).to have_key("alt") } - it { expect(@link_to_image).to have_key("url") } - it { expect(@link_to_image).to have_value("dk") } - it { expect(@link_to_image["url"]).to match("http://test.example/uploads/#{@project.path_with_namespace}") } - it { expect(@link_to_image["url"]).to match("dk.png") } - end - - context 'for valid jpg file' do - before do - jpg = fixture_file_upload(Rails.root + 'spec/fixtures/rails_sample.jpg', 'image/jpg') - @link_to_image = upload_image(@project.repository, { 'markdown_img' => jpg }, "http://test.example/") - end - - it { expect(@link_to_image).to have_key("alt") } - it { expect(@link_to_image).to have_key("url") } - it { expect(@link_to_image).to have_value("rails_sample") } - it { expect(@link_to_image["url"]).to match("http://test.example/uploads/#{@project.path_with_namespace}") } - it { expect(@link_to_image["url"]).to match("rails_sample.jpg") } - end - - context 'for txt file' do - before do - txt = fixture_file_upload(Rails.root + 'spec/fixtures/doc_sample.txt', 'text/plain') - @link_to_image = upload_image(@project.repository, { 'markdown_img' => txt }, "http://test.example/") - end - - it { expect(@link_to_image).to be_nil } - end - end - - def upload_image(repository, params, root_url) - Projects::ImageService.new(repository, params, root_url).execute - end -end diff --git a/spec/services/projects/upload_service_spec.rb b/spec/services/projects/upload_service_spec.rb new file mode 100644 index 00000000000..fc34b456482 --- /dev/null +++ b/spec/services/projects/upload_service_spec.rb @@ -0,0 +1,75 @@ +require 'spec_helper' + +describe Projects::UploadService do + describe 'File service' do + before do + @user = create :user + @project = create :project, creator_id: @user.id, namespace: @user.namespace + end + + context 'for valid gif file' do + before do + gif = fixture_file_upload(Rails.root + 'spec/fixtures/banana_sample.gif', 'image/gif') + @link_to_file = upload_file(@project.repository, gif) + end + + it { expect(@link_to_file).to have_key('alt') } + it { expect(@link_to_file).to have_key('url') } + it { expect(@link_to_file).to have_key('is_image') } + it { expect(@link_to_file).to have_value('banana_sample') } + it { expect(@link_to_file['is_image']).to equal(true) } + it { expect(@link_to_file['url']).to match("/#{@project.path_with_namespace}") } + it { expect(@link_to_file['url']).to match('banana_sample.gif') } + end + + context 'for valid png file' do + before do + png = fixture_file_upload(Rails.root + 'spec/fixtures/dk.png', + 'image/png') + @link_to_file = upload_file(@project.repository, png) + end + + it { expect(@link_to_file).to have_key('alt') } + it { expect(@link_to_file).to have_key('url') } + it { expect(@link_to_file).to have_value('dk') } + it { expect(@link_to_file).to have_key('is_image') } + it { expect(@link_to_file['is_image']).to equal(true) } + it { expect(@link_to_file['url']).to match("/#{@project.path_with_namespace}") } + it { expect(@link_to_file['url']).to match('dk.png') } + end + + context 'for valid jpg file' do + before do + jpg = fixture_file_upload(Rails.root + 'spec/fixtures/rails_sample.jpg', 'image/jpg') + @link_to_file = upload_file(@project.repository, jpg) + end + + it { expect(@link_to_file).to have_key('alt') } + it { expect(@link_to_file).to have_key('url') } + it { expect(@link_to_file).to have_key('is_image') } + it { expect(@link_to_file).to have_value('rails_sample') } + it { expect(@link_to_file['is_image']).to equal(true) } + it { expect(@link_to_file['url']).to match("/#{@project.path_with_namespace}") } + it { expect(@link_to_file['url']).to match('rails_sample.jpg') } + end + + context 'for txt file' do + before do + txt = fixture_file_upload(Rails.root + 'spec/fixtures/doc_sample.txt', 'text/plain') + @link_to_file = upload_file(@project.repository, txt) + end + + it { expect(@link_to_file).to have_key('alt') } + it { expect(@link_to_file).to have_key('url') } + it { expect(@link_to_file).to have_key('is_image') } + it { expect(@link_to_file).to have_value('doc_sample.txt') } + it { expect(@link_to_file['is_image']).to equal(false) } + it { expect(@link_to_file['url']).to match("/#{@project.path_with_namespace}") } + it { expect(@link_to_file['url']).to match('doc_sample.txt') } + end + end + + def upload_file(repository, file) + Projects::UploadService.new(repository, file).execute + end +end |