IntenseWebs/discourse
3
0
Fork 0
mirror of https://github.com/discourse/discourse.git synced 2024-11-23 09:26:54 -06:00
Code Issues Packages Projects Releases Wiki Activity

FIX: Never mark uploads based on regular emoji secure (#8973)

Browse Source 
Sometimes PullHotlinkedImages pulls down a site emoji and creates a new upload record for it. In the cases where these happen the upload is not created via the normal path that custom emoji follows, so we need to check in UploadSecurity whether the origin of the upload is based on a regular site emoji. If it is we never want to mark it as secure (we don't want emoji not accessible from other posts because of secure media).

This only became apparent because the uploads:ensure_correct_acl rake task uses UploadSecurity to check whether an upload should be secure, which would have marked a whole bunch of regular-old-emojis as secure.
This commit is contained in:
Martin Brennan 2020-02-17 12:30:47 +10:00 committed by GitHub
parent 3b062f79fc
commit dac923379a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 16 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
lib/upload_security.rb
View File

@ -28,7 +28,7 @@ class UploadSecurity
private
def uploading_in_public_context?
@upload.for_theme || @upload.for_site_setting || @upload.for_gravatar || public_type? || used_for_custom_emoji?
@upload.for_theme || @upload.for_site_setting || @upload.for_gravatar || public_type? || used_for_custom_emoji? || based_on_regular_emoji?
end
def supported_media?
@ -72,7 +72,12 @@ class UploadSecurity
end
def used_for_custom_emoji?
return false if @upload.id.blank?
CustomEmoji.exists?(upload_id: @upload.id)
@upload.id.present? && CustomEmoji.exists?(upload_id: @upload.id)
end
def based_on_regular_emoji?
return false if @upload.origin.blank?
uri = URI.parse(@upload.origin)
Emoji.all.map(&:url).include?("#{uri.path}?#{uri.query}")
end
end

8
spec/models/upload_spec.rb
View File

@ -431,6 +431,14 @@ describe Upload do
expect(upload.reload.secure).to eq(false)
end
it 'does not mark an upload whose origin matches a regular emoji as secure (sometimes emojis are downloaded in pull_hotlinked_images)' do
SiteSetting.login_required = true
grinning = Emoji.all.last
upload.update!(secure: false, origin: "http://localhost:3000#{grinning.url}")
expect { upload.update_secure_status }
.not_to change { upload.secure }
end
end
end