Прямая загрузка в S3 с локальным хранилищем вызывает NoMethodError вместо отклонения

Внести вклад Ошибка
1

Я сам нашёл и решил эту проблему после того, как один из клиентов сообщил о ней. Я использовал ИИ для генерации отчёта об ошибке, потому что был ленив, и он справился с этим лучше, чем я, — и при этом обнаружил фундаментальный баг, который я вручную подтвердил.

Ничего критичного, но, возможно, имеет смысл проверить, не вызывает ли этот паттерн проблемы в других местах.

Всё началось с того, что кто-то включил enable_direct_s3_uploads, не настроив и не активировав S3, из-за чего все загрузки сломались, возвращая ошибки 500 «под капотом». Я подумал, что ошибка 500 заслуживает расследования.

Краткое описание

Когда enable_direct_s3_uploads включён, а активным хранилищем загрузок является локальное, попытка загрузки может вызвать:

NoMethodError (undefined method 'signed_request_for_temporary_upload' for an instance of FileStore::LocalStore)

app/services/external_upload_manager.rb:34:in 'ExternalUploadManager.create_direct_upload'
lib/external_upload_helpers.rb:56:in 'ExternalUploadHelpers#generate_presigned_put'
app/controllers/application_controller.rb:443:in 'block in ApplicationController#with_resolved_locale'
app/controllers/application_controller.rb:443:in 'ApplicationController#with_resolved_locale'

Похоже, это вызвано сочетанием некорректного состояния настроек и переопределённого колбэка before_action.

Конфигурация

Проблемное состояние выглядит примерно так:

SiteSetting.enable_direct_s3_uploads == true
SiteSetting.enable_s3_uploads == false
Discourse.store.is_a?(FileStore::LocalStore) == true

Для локального хранилища Discourse.store не реализует:

signed_request_for_temporary_upload

что ожидаемо, поскольку эта операция имеет смысл только для внешнего/S3-хранилища.

Ожидаемое поведение

Если прямые загрузки в S3 включены, а активное хранилище локальное, запрос должен быть корректно отклонён с помощью external_store_check.

В идеале некорректное сочетание настроек сайта также должно предотвращаться или валидироваться.

Фактическое поведение

Запрос достигает ExternalUploadManager.create_direct_upload, который вызывает:

store.signed_request_for_temporary_upload(...)

для объекта FileStore::LocalStore, что приводит к NoMethodError.

Возможная причина

ExternalUploadHelpers регистрирует следующий колбэк:

before_action :external_store_check,
  only: %i[
    generate_presigned_put
    complete_external_upload
    create_multipart
    batch_presign_multipart_parts
    complete_multipart
    abort_multipart
  ]

Это должно предотвращать достижение кода внешних загрузок, когда хранилище локальное.

Однако UploadsController позже регистрирует другой колбэк, используя тот же метод фильтра:

before_action :external_store_check,
  only: %i[_show_secure_deprecated show_secure]

Rails рассматривает повторную регистрацию одного и того же колбэка как переопределение, поэтому, по всей видимости, последняя запись заменяет более ранние условия only:.

image

В результате external_store_check больше не выполняется для generate_presigned_put, позволяя локальному хранилищу достичь кода прямых загрузок.

Воспроизведение

  1. Настройте экземпляр Discourse с локальным хранилищем загрузок.
  2. Убедитесь, что:
SiteSetting.enable_s3_uploads = false
SiteSetting.enable_direct_s3_uploads = true
  1. Попробуйте загрузить файл.
  2. Наблюдайте NoMethodError от FileStore::LocalStore.

Проверка состояния в консоли может быть выполнена с помощью:

[
  SiteSetting.enable_direct_s3_uploads,
  SiteSetting.enable_s3_uploads,
  Discourse.store.class,
  Discourse.store.external?
]

Предлагаемое исправление

Регистрации колбэков не должны переопределять друг друга.

Например, включите действия безопасных загрузок в существующий колбэк external_store_check:

before_action :external_store_check,
  only: %i[
    generate_presigned_put
    complete_external_upload
    create_multipart
    batch_presign_multipart_parts
    complete_multipart
    abort_multipart
    _show_secure_deprecated
    show_secure
  ]

В качестве альтернативы используйте отдельный метод колбэка для действий безопасных загрузок.

Также может быть целесообразно добавить валидацию, чтобы enable_direct_s3_uploads не могло быть включено, пока S3/внешние загрузки отключены.

Обходное решение

Для сайтов, использующих локальное хранилище загрузок:

SiteSetting.enable_direct_s3_uploads = false

предотвращает ошибку.

2 лайка
2

Спасибо, должно быть исправлено согласно:

1 лайк