kotovalexarian-likes-gitlab/gitlab-org--gitlab-foss
1
0
Fork 0
Code Releases Activity

Add latest changes from gitlab-org/gitlab@master

Browse source
This commit is contained in:
GitLab Bot 2020-08-28 06:10:45 +00:00
parent 6fc9fb92ab
commit 540c69c58c
16 changed files with 658 additions and 409 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

24
app/controllers/concerns/send_file_upload.rb
View file

@ -2,6 +2,8 @@
module SendFileUpload
def send_upload(file_upload, send_params: {}, redirect_params: {}, attachment: nil, proxy: false, disposition: 'attachment')
content_type = content_type_for(attachment)
if attachment
response_disposition = ActionDispatch::Http::ContentDisposition.format(disposition: disposition, filename: attachment)
@ -9,7 +11,7 @@ module SendFileUpload
# Google Cloud Storage, so the metadata needs to be cleared on GCS for
# this to work. However, this override works with AWS.
redirect_params[:query] = { "response-content-disposition" => response_disposition,
"response-content-type" => guess_content_type(attachment) }
"response-content-type" => content_type }
# By default, Rails will send uploads with an extension of .js with a
# content-type of text/javascript, which will trigger Rails'
# cross-origin JavaScript protection.
@ -20,7 +22,7 @@ module SendFileUpload
if image_scaling_request?(file_upload)
location = file_upload.file_storage? ? file_upload.path : file_upload.url
headers.store(*Gitlab::Workhorse.send_scaled_image(location, params[:width].to_i))
headers.store(*Gitlab::Workhorse.send_scaled_image(location, params[:width].to_i, content_type))
head :ok
elsif file_upload.file_storage?
send_file file_upload.path, send_params
@ -32,6 +34,12 @@ module SendFileUpload
end
end
def content_type_for(attachment)
return '' unless attachment
guess_content_type(attachment)
end
def guess_content_type(filename)
types = MIME::Types.type_for(filename)
@ -45,12 +53,16 @@ module SendFileUpload
private
def image_scaling_request?(file_upload)
avatar_image_upload?(file_upload) && valid_image_scaling_width? && current_user &&
Feature.enabled?(:dynamic_image_resizing, current_user)
Feature.enabled?(:dynamic_image_resizing, current_user) &&
avatar_safe_for_scaling?(file_upload) && valid_image_scaling_width? && current_user
end
def avatar_image_upload?(file_upload)
file_upload.try(:image?) && file_upload.try(:mounted_as)&.to_sym == :avatar
def avatar_safe_for_scaling?(file_upload)
file_upload.try(:image_safe_for_scaling?) && mounted_as_avatar?(file_upload)
end
def mounted_as_avatar?(file_upload)
file_upload.try(:mounted_as)&.to_sym == :avatar
end
def valid_image_scaling_width?

11
app/graphql/resolvers/merge_requests_resolver.rb
View file

@ -29,11 +29,14 @@ module Resolvers
as: :label_name,
description: 'Array of label names. All resolved merge requests will have all of these labels.'
argument :merged_after, Types::TimeType,
required: false,
description: 'Merge requests merged after this date'
required: false,
description: 'Merge requests merged after this date'
argument :merged_before, Types::TimeType,
required: false,
description: 'Merge requests merged before this date'
required: false,
description: 'Merge requests merged before this date'
argument :milestone_title, GraphQL::STRING_TYPE,
required: false,
description: 'Title of the milestone'
def self.single
::Resolvers::MergeRequestResolver

12
app/graphql/resolvers/project_merge_requests_resolver.rb Normal file
View file

@ -0,0 +1,12 @@
# frozen_string_literal: true
module Resolvers
class ProjectMergeRequestsResolver < MergeRequestsResolver
argument :assignee_username, GraphQL::STRING_TYPE,
required: false,
description: 'Username of the assignee'
argument :author_username, GraphQL::STRING_TYPE,
required: false,
description: 'Username of the author'
end
end

2
app/graphql/types/project_type.rb
View file

@ -134,7 +134,7 @@ module Types
null: true,
description: 'Merge requests of the project',
extras: [:lookahead],
resolver: Resolvers::MergeRequestsResolver
resolver: Resolvers::ProjectMergeRequestsResolver
field :merge_request,
Types::MergeRequestType,

776
app/workers/all_queues.yml
View file

File diff suppressed because it is too large Load diff

5
changelogs/unreleased/235984-add-filters-to-mrs-graphql-api.yml Normal file
View file

@ -0,0 +1,5 @@
---
title: Filter Merge Requests by author, assignee and milestone in GraphQL
merge_request: 40265
author:
type: added

25
doc/api/graphql/reference/gitlab_schema.graphql
View file

@ -11552,6 +11552,16 @@ type Project {
"""
after: String
"""
Username of the assignee
"""
assigneeUsername: String
"""
Username of the author
"""
authorUsername: String
"""
Returns the elements in the list that come before the specified cursor.
"""
@ -11587,6 +11597,11 @@ type Project {
"""
mergedBefore: Time
"""
Title of the milestone
"""
milestoneTitle: String
"""
Array of source branch names. All resolved merge requests will have one of these branches as their source.
"""
@ -16644,6 +16659,11 @@ type User {
"""
mergedBefore: Time
"""
Title of the milestone
"""
milestoneTitle: String
"""
The global ID of the project the authored merge requests should be in. Incompatible with projectPath.
"""
@ -16714,6 +16734,11 @@ type User {
"""
mergedBefore: Time
"""
Title of the milestone
"""
milestoneTitle: String
"""
The global ID of the project the authored merge requests should be in. Incompatible with projectPath.
"""

50
doc/api/graphql/reference/gitlab_schema.json
View file

@ -34304,6 +34304,36 @@
},
"defaultValue": null
},
{
"name": "milestoneTitle",
"description": "Title of the milestone",
"type": {
"kind": "SCALAR",
"name": "String",
"ofType": null
},
"defaultValue": null
},
{
"name": "assigneeUsername",
"description": "Username of the assignee",
"type": {
"kind": "SCALAR",
"name": "String",
"ofType": null
},
"defaultValue": null
},
{
"name": "authorUsername",
"description": "Username of the author",
"type": {
"kind": "SCALAR",
"name": "String",
"ofType": null
},
"defaultValue": null
},
{
"name": "after",
"description": "Returns the elements in the list that come after the specified cursor.",
@ -48966,6 +48996,16 @@
},
"defaultValue": null
},
{
"name": "milestoneTitle",
"description": "Title of the milestone",
"type": {
"kind": "SCALAR",
"name": "String",
"ofType": null
},
"defaultValue": null
},
{
"name": "projectPath",
"description": "The full-path of the project the authored merge requests should be in. Incompatible with projectId.",
@ -49141,6 +49181,16 @@
},
"defaultValue": null
},
{
"name": "milestoneTitle",
"description": "Title of the milestone",
"type": {
"kind": "SCALAR",
"name": "String",
"ofType": null
},
"defaultValue": null
},
{
"name": "projectPath",
"description": "The full-path of the project the authored merge requests should be in. Incompatible with projectId.",

8
lib/gitlab/file_type_detection.rb
View file

@ -20,6 +20,8 @@
module Gitlab
module FileTypeDetection
SAFE_IMAGE_EXT = %w[png jpg jpeg gif bmp tiff ico].freeze
SAFE_IMAGE_FOR_SCALING_EXT = %w[png jpg jpeg].freeze
PDF_EXT = 'pdf'
# We recommend using the .mp4 format over .mov. Videos in .mov format can
# still be used but you really need to make sure they are served with the
@ -46,6 +48,12 @@ module Gitlab
extension_match?(SAFE_IMAGE_EXT)
end
# For the time being, we restrict image scaling requests to the most popular and safest formats only,
# which are JPGs and PNGs. See https://gitlab.com/gitlab-org/gitlab/-/issues/237848 for more info.
def image_safe_for_scaling?
extension_match?(SAFE_IMAGE_FOR_SCALING_EXT)
end
def video?
extension_match?(SAFE_VIDEO_EXT)
end

5
lib/gitlab/workhorse.rb
View file

@ -156,10 +156,11 @@ module Gitlab
]
end
def send_scaled_image(location, width)
def send_scaled_image(location, width, content_type)
params = {
'Location' => location,
'Width' => width
'Width' => width,
'ContentType' => content_type
}
[

21
spec/controllers/concerns/send_file_upload_spec.rb
View file

@ -49,10 +49,14 @@ RSpec.describe SendFileUpload do
end
shared_examples 'handles image resize requests' do
let(:params) do
{ attachment: 'avatar.png' }
end
let(:headers) { double }
before do
allow(uploader).to receive(:image?).and_return(true)
allow(uploader).to receive(:image_safe_for_scaling?).and_return(true)
allow(uploader).to receive(:mounted_as).and_return(:avatar)
allow(controller).to receive(:headers).and_return(headers)
@ -75,7 +79,10 @@ RSpec.describe SendFileUpload do
expect(controller).not_to receive(:send_file)
expect(controller).to receive(:params).at_least(:once).and_return(width: '64')
expect(controller).to receive(:head).with(:ok)
expect(headers).to receive(:store).with(Gitlab::Workhorse::SEND_DATA_HEADER, /^send-scaled-img:/)
expect(Gitlab::Workhorse).to receive(:send_scaled_image).with(a_string_matching('^(/.+|https://.+)'), 64, 'image/png').and_return([
Gitlab::Workhorse::SEND_DATA_HEADER, "send-scaled-img:faux"
])
expect(headers).to receive(:store).with(Gitlab::Workhorse::SEND_DATA_HEADER, "send-scaled-img:faux")
subject
end
@ -115,6 +122,15 @@ RSpec.describe SendFileUpload do
subject
end
end
context 'when image file type is not considered safe for scaling' do
it 'does not write workhorse command header' do
expect(uploader).to receive(:image_safe_for_scaling?).and_return(false)
expect(headers).not_to receive(:store).with(Gitlab::Workhorse::SEND_DATA_HEADER, /^send-scaled-img:/)
subject
end
end
end
context 'when feature is disabled' do
@ -123,7 +139,6 @@ RSpec.describe SendFileUpload do
end
it 'does not write workhorse command header' do
expect(controller).to receive(:params).at_least(:once).and_return(width: '64')
expect(headers).not_to receive(:store).with(Gitlab::Workhorse::SEND_DATA_HEADER, /^send-scaled-img:/)
subject

20
spec/graphql/resolvers/merge_requests_resolver_spec.rb
View file

@ -6,7 +6,9 @@ RSpec.describe Resolvers::MergeRequestsResolver do
include GraphqlHelpers
let_it_be(:project) { create(:project, :repository) }
let_it_be(:milestone) { create(:milestone, project: project) }
let_it_be(:current_user) { create(:user) }
let_it_be(:other_user) { create(:user) }
let_it_be(:common_attrs) { { author: current_user, source_project: project, target_project: project } }
let_it_be(:merge_request_1) { create(:merge_request, :simple, **common_attrs) }
let_it_be(:merge_request_2) { create(:merge_request, :rebased, **common_attrs) }
@ -14,8 +16,10 @@ RSpec.describe Resolvers::MergeRequestsResolver do
let_it_be(:merge_request_4) { create(:merge_request, :unique_branches, :locked, **common_attrs) }
let_it_be(:merge_request_5) { create(:merge_request, :simple, :locked, **common_attrs) }
let_it_be(:merge_request_6) { create(:labeled_merge_request, :unique_branches, labels: create_list(:label, 2), **common_attrs) }
let_it_be(:merge_request_with_milestone) { create(:merge_request, :unique_branches, **common_attrs, milestone: milestone) }
let_it_be(:other_project) { create(:project, :repository) }
let_it_be(:other_merge_request) { create(:merge_request, source_project: other_project, target_project: other_project) }
let(:iid_1) { merge_request_1.iid }
let(:iid_2) { merge_request_2.iid }
let(:other_iid) { other_merge_request.iid }
@ -32,7 +36,7 @@ RSpec.describe Resolvers::MergeRequestsResolver do
it 'returns all merge requests' do
result = resolve_mr(project, {})
expect(result).to contain_exactly(merge_request_1, merge_request_2, merge_request_3, merge_request_4, merge_request_5, merge_request_6)
expect(result).to contain_exactly(merge_request_1, merge_request_2, merge_request_3, merge_request_4, merge_request_5, merge_request_6, merge_request_with_milestone)
end
it 'returns only merge requests that the current user can see' do
@ -179,6 +183,20 @@ RSpec.describe Resolvers::MergeRequestsResolver do
end
end
context 'by milestone' do
it 'filters merge requests by milestone title' do
result = resolve_mr(project, milestone_title: milestone.title)
expect(result).to eq([merge_request_with_milestone])
end
it 'does not find anything' do
result = resolve_mr(project, milestone_title: 'unknown-milestone')
expect(result).to be_empty
end
end
describe 'combinations' do
it 'requires all filters' do
create(:merge_request, :closed, source_project: project, target_project: project, source_branch: merge_request_4.source_branch)

56
spec/graphql/resolvers/project_merge_requests_resolver_spec.rb Normal file
View file

@ -0,0 +1,56 @@
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Resolvers::ProjectMergeRequestsResolver do
include GraphqlHelpers
let_it_be(:project) { create(:project, :repository) }
let_it_be(:current_user) { create(:user) }
let_it_be(:other_user) { create(:user) }
let_it_be(:merge_request_with_author_and_assignee) do
create(:merge_request,
:unique_branches,
source_project: project,
target_project: project,
author: other_user,
assignee: other_user)
end
before do
project.add_developer(current_user)
end
context 'by assignee' do
it 'filters merge requests by assignee username' do
result = resolve_mr(project, assignee_username: other_user.username)
expect(result).to eq([merge_request_with_author_and_assignee])
end
it 'does not find anything' do
result = resolve_mr(project, assignee_username: 'unknown-user')
expect(result).to be_empty
end
end
context 'by author' do
it 'filters merge requests by author username' do
result = resolve_mr(project, author_username: other_user.username)
expect(result).to eq([merge_request_with_author_and_assignee])
end
it 'does not find anything' do
result = resolve_mr(project, author_username: 'unknown-user')
expect(result).to be_empty
end
end
def resolve_mr(project, args, resolver: described_class, user: current_user)
resolve(resolver, obj: project, args: args, ctx: { current_user: user })
end
end

7
spec/graphql/types/project_type_spec.rb
View file

@ -59,7 +59,7 @@ RSpec.describe GitlabSchema.types['Project'] do
subject { described_class.fields['mergeRequests'] }
it { is_expected.to have_graphql_type(Types::MergeRequestType.connection_type) }
it { is_expected.to have_graphql_resolver(Resolvers::MergeRequestsResolver) }
it { is_expected.to have_graphql_resolver(Resolvers::ProjectMergeRequestsResolver) }
it do
is_expected.to have_graphql_arguments(:iids,
@ -72,7 +72,10 @@ RSpec.describe GitlabSchema.types['Project'] do
:first,
:last,
:merged_after,
:merged_before
:merged_before,
:author_username,
:assignee_username,
:milestone_title
)
end
end

39
spec/lib/gitlab/file_type_detection_spec.rb
View file

@ -192,6 +192,20 @@ RSpec.describe Gitlab::FileTypeDetection do
end
end
describe '#image_safe_for_scaling?' do
it 'returns true for allowed image formats' do
uploader.store!(upload_fixture('rails_sample.jpg'))
expect(uploader).to be_image_safe_for_scaling
end
it 'returns false for other files' do
uploader.store!(upload_fixture('unsanitized.svg'))
expect(uploader).not_to be_image_safe_for_scaling
end
end
describe '#dangerous_image?' do
it 'returns true if filename has a dangerous extension' do
uploader.store!(upload_fixture('unsanitized.svg'))
@ -377,6 +391,31 @@ RSpec.describe Gitlab::FileTypeDetection do
end
end
describe '#image_safe_for_scaling?' do
using RSpec::Parameterized::TableSyntax
where(:filename, :expectation) do
'img.jpg' | true
'img.jpeg' | true
'img.png' | true
'img.svg' | false
end
with_them do
it "returns expected result" do
allow(custom_class).to receive(:filename).and_return(filename)
expect(custom_class.image_safe_for_scaling?).to be(expectation)
end
end
it 'returns false if filename is blank' do
allow(custom_class).to receive(:filename).and_return(nil)
expect(custom_class).not_to be_image_safe_for_scaling
end
end
describe '#video?' do
it 'returns true for a video file' do
allow(custom_class).to receive(:filename).and_return('video_sample.mp4')

6
spec/lib/gitlab/workhorse_spec.rb
View file

@ -424,8 +424,9 @@ RSpec.describe Gitlab::Workhorse do
describe '.send_scaled_image' do
let(:location) { 'http://example.com/avatar.png' }
let(:width) { '150' }
let(:content_type) { 'image/png' }
subject { described_class.send_scaled_image(location, width) }
subject { described_class.send_scaled_image(location, width, content_type) }
it 'sets the header correctly' do
key, command, params = decode_workhorse_header(subject)
@ -434,7 +435,8 @@ RSpec.describe Gitlab::Workhorse do
expect(command).to eq("send-scaled-img")
expect(params).to eq({
'Location' => location,
'Width' => width
'Width' => width,
'ContentType' => content_type
}.deep_stringify_keys)
end
end