Fix media attachments enumeration (#14254)

* Fix media attachment enumeration

* Switch media_attachments id to snowflake ids

Co-authored-by: Thibaut Girka <thib@sitedethib.com>
This commit is contained in:
Eugen Rochko 2020-07-07 15:26:51 +02:00 committed by GitHub
parent 81a3db1564
commit 6e25574ce5
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 77 additions and 16 deletions

View File

@ -2,6 +2,7 @@
class MediaProxyController < ApplicationController class MediaProxyController < ApplicationController
include RoutingHelper include RoutingHelper
include Authorization
skip_before_action :store_current_location skip_before_action :store_current_location
skip_before_action :require_functional! skip_before_action :require_functional!
@ -10,12 +11,14 @@ class MediaProxyController < ApplicationController
rescue_from ActiveRecord::RecordInvalid, with: :not_found rescue_from ActiveRecord::RecordInvalid, with: :not_found
rescue_from Mastodon::UnexpectedResponseError, with: :not_found rescue_from Mastodon::UnexpectedResponseError, with: :not_found
rescue_from Mastodon::NotPermittedError, with: :not_found
rescue_from HTTP::TimeoutError, HTTP::ConnectionError, OpenSSL::SSL::SSLError, with: :internal_server_error rescue_from HTTP::TimeoutError, HTTP::ConnectionError, OpenSSL::SSL::SSLError, with: :internal_server_error
def show def show
RedisLock.acquire(lock_options) do |lock| RedisLock.acquire(lock_options) do |lock|
if lock.acquired? if lock.acquired?
@media_attachment = MediaAttachment.remote.find(params[:id]) @media_attachment = MediaAttachment.remote.attached.find(params[:id])
authorize @media_attachment.status, :show?
redownload! if @media_attachment.needs_redownload? && !reject_media? redownload! if @media_attachment.needs_redownload? && !reject_media?
else else
raise Mastodon::RaceConditionError raise Mastodon::RaceConditionError

View File

@ -0,0 +1,17 @@
class MediaAttachmentIdsToTimestampIds < ActiveRecord::Migration[5.1]
def up
# Set up the media_attachments.id column to use our timestamp-based IDs.
safety_assured do
execute("ALTER TABLE media_attachments ALTER COLUMN id SET DEFAULT timestamp_id('media_attachments')")
end
# Make sure we have a sequence to use.
Mastodon::Snowflake.ensure_id_sequences_exist
end
def down
execute("LOCK media_attachments")
execute("SELECT setval('media_attachments_id_seq', (SELECT MAX(id) FROM media_attachments))")
execute("ALTER TABLE media_attachments ALTER COLUMN id SET DEFAULT nextval('media_attachments_id_seq')")
end
end

View File

@ -77,6 +77,16 @@ ActiveRecord::Schema.define(version: 2020_06_28_133322) do
t.index ["target_account_id"], name: "index_account_moderation_notes_on_target_account_id" t.index ["target_account_id"], name: "index_account_moderation_notes_on_target_account_id"
end end
create_table "account_notes", force: :cascade do |t|
t.bigint "account_id"
t.bigint "target_account_id"
t.text "comment", null: false
t.datetime "created_at", null: false
t.datetime "updated_at", null: false
t.index ["account_id", "target_account_id"], name: "index_account_notes_on_account_id_and_target_account_id", unique: true
t.index ["target_account_id"], name: "index_account_notes_on_target_account_id"
end
create_table "account_pins", force: :cascade do |t| create_table "account_pins", force: :cascade do |t|
t.bigint "account_id" t.bigint "account_id"
t.bigint "target_account_id" t.bigint "target_account_id"
@ -471,7 +481,7 @@ ActiveRecord::Schema.define(version: 2020_06_28_133322) do
t.index ["user_id", "timeline"], name: "index_markers_on_user_id_and_timeline", unique: true t.index ["user_id", "timeline"], name: "index_markers_on_user_id_and_timeline", unique: true
end end
create_table "media_attachments", force: :cascade do |t| create_table "media_attachments", id: :bigint, default: -> { "timestamp_id('media_attachments'::text)" }, force: :cascade do |t|
t.bigint "status_id" t.bigint "status_id"
t.string "file_file_name" t.string "file_file_name"
t.string "file_content_type" t.string "file_content_type"
@ -833,16 +843,6 @@ ActiveRecord::Schema.define(version: 2020_06_28_133322) do
t.index ["user_id"], name: "index_user_invite_requests_on_user_id" t.index ["user_id"], name: "index_user_invite_requests_on_user_id"
end end
create_table "account_notes", force: :cascade do |t|
t.bigint "account_id"
t.bigint "target_account_id"
t.text "comment", null: false
t.datetime "created_at", null: false
t.datetime "updated_at", null: false
t.index ["account_id", "target_account_id"], name: "index_account_notes_on_account_id_and_target_account_id", unique: true
t.index ["target_account_id"], name: "index_account_notes_on_target_account_id"
end
create_table "users", force: :cascade do |t| create_table "users", force: :cascade do |t|
t.string "email", default: "", null: false t.string "email", default: "", null: false
t.datetime "created_at", null: false t.datetime "created_at", null: false
@ -918,6 +918,8 @@ ActiveRecord::Schema.define(version: 2020_06_28_133322) do
add_foreign_key "account_migrations", "accounts", on_delete: :cascade add_foreign_key "account_migrations", "accounts", on_delete: :cascade
add_foreign_key "account_moderation_notes", "accounts" add_foreign_key "account_moderation_notes", "accounts"
add_foreign_key "account_moderation_notes", "accounts", column: "target_account_id" add_foreign_key "account_moderation_notes", "accounts", column: "target_account_id"
add_foreign_key "account_notes", "accounts", column: "target_account_id", on_delete: :cascade
add_foreign_key "account_notes", "accounts", on_delete: :cascade
add_foreign_key "account_pins", "accounts", column: "target_account_id", on_delete: :cascade add_foreign_key "account_pins", "accounts", column: "target_account_id", on_delete: :cascade
add_foreign_key "account_pins", "accounts", on_delete: :cascade add_foreign_key "account_pins", "accounts", on_delete: :cascade
add_foreign_key "account_stats", "accounts", on_delete: :cascade add_foreign_key "account_stats", "accounts", on_delete: :cascade
@ -999,8 +1001,6 @@ ActiveRecord::Schema.define(version: 2020_06_28_133322) do
add_foreign_key "statuses_tags", "tags", name: "fk_3081861e21", on_delete: :cascade add_foreign_key "statuses_tags", "tags", name: "fk_3081861e21", on_delete: :cascade
add_foreign_key "tombstones", "accounts", on_delete: :cascade add_foreign_key "tombstones", "accounts", on_delete: :cascade
add_foreign_key "user_invite_requests", "users", on_delete: :cascade add_foreign_key "user_invite_requests", "users", on_delete: :cascade
add_foreign_key "account_notes", "accounts", column: "target_account_id", on_delete: :cascade
add_foreign_key "account_notes", "accounts", on_delete: :cascade
add_foreign_key "users", "accounts", name: "fk_50500f500d", on_delete: :cascade add_foreign_key "users", "accounts", name: "fk_50500f500d", on_delete: :cascade
add_foreign_key "users", "invites", on_delete: :nullify add_foreign_key "users", "invites", on_delete: :nullify
add_foreign_key "users", "oauth_applications", column: "created_by_application_id", on_delete: :nullify add_foreign_key "users", "oauth_applications", column: "created_by_application_id", on_delete: :nullify

View File

@ -28,9 +28,8 @@ describe MediaController do
end end
it 'raises when not permitted to view' do it 'raises when not permitted to view' do
status = Fabricate(:status) status = Fabricate(:status, visibility: :direct)
media_attachment = Fabricate(:media_attachment, status: status) media_attachment = Fabricate(:media_attachment, status: status)
allow_any_instance_of(MediaController).to receive(:authorize).and_raise(ActiveRecord::RecordNotFound)
get :show, params: { id: media_attachment.to_param } get :show, params: { id: media_attachment.to_param }
expect(response).to have_http_status(404) expect(response).to have_http_status(404)

View File

@ -0,0 +1,42 @@
# frozen_string_literal: true
require 'rails_helper'
describe MediaProxyController do
render_views
before do
stub_request(:get, 'http://example.com/attachment.png').to_return(request_fixture('avatar.txt'))
end
describe '#show' do
it 'redirects when attached to a status' do
status = Fabricate(:status)
media_attachment = Fabricate(:media_attachment, status: status, remote_url: 'http://example.com/attachment.png')
get :show, params: { id: media_attachment.id }
expect(response).to have_http_status(302)
end
it 'responds with missing when there is not an attached status' do
media_attachment = Fabricate(:media_attachment, status: nil, remote_url: 'http://example.com/attachment.png')
get :show, params: { id: media_attachment.id }
expect(response).to have_http_status(404)
end
it 'raises when id cant be found' do
get :show, params: { id: 'missing' }
expect(response).to have_http_status(404)
end
it 'raises when not permitted to view' do
status = Fabricate(:status, visibility: :direct)
media_attachment = Fabricate(:media_attachment, status: status, remote_url: 'http://example.com/attachment.png')
get :show, params: { id: media_attachment.id }
expect(response).to have_http_status(404)
end
end
end