Upload direto para o S3 com armazenamento local gera NoMethodError em vez de ser rejeitado

Contribuir Bug
1

Encontrei e resolvi este problema eu mesmo após um cliente relatá-lo. Usei IA para gerar o relatório do bug porque estava preguiçoso e ela faz um trabalho melhor do que eu — e, ao fazê-lo, encontrou um bug no núcleo, que verifiquei manualmente.

Nada grave, mas pode fazer sentido verificar se esse padrão está causando problemas em outros lugares também.

Tudo começou quando alguém marcou enable_direct_s3_uploads sem realmente configurar e habilitar o S3, o que quebrou todos os uploads, retornando erros 500 nos bastidores. Achei que o erro 500 valia a pena investigar.

Resumo

Quando enable_direct_s3_uploads está habilitado, mas o armazenamento de upload ativo é local, a tentativa de upload pode lançar:

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'

Isso parece ser causado por uma combinação de um estado de configuração inválido e um callback before_action sobrescrito.

Configuração

O estado problemático é, na prática:

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

Para armazenamento local, Discourse.store não implementa:

signed_request_for_temporary_upload

o que é esperado, já que essa operação só faz sentido para um armazenamento externo/S3.

Comportamento esperado

Se os uploads diretos via S3 estiverem habilitados enquanto o armazenamento ativo é local, a solicitação deve ser rejeitada limpa pelo external_store_check.

Idealmente, a combinação inválida de configurações do site também deve ser prevenida ou validada.

Comportamento real

A solicitação atinge ExternalUploadManager.create_direct_upload, que chama:

store.signed_request_for_temporary_upload(...)

em um FileStore::LocalStore, resultando em um NoMethodError.

Possível causa

ExternalUploadHelpers registra o seguinte callback:

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

Isso deveria impedir que a solicitação alcance o código de upload externo quando o armazenamento for local.

No entanto, UploadsController registra posteriormente outro callback usando o mesmo método de filtro:

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

O Rails trata o registro repetido do mesmo callback como uma redefinição, então o segundo parece substituir as condições only: anteriores.

image

Como resultado, external_store_check não é mais executado para generate_presigned_put, permitindo que um armazenamento local alcance o caminho de código de upload direto.

Reprodução

  1. Configure uma instância do Discourse com armazenamento de upload local.
  2. Certifique-se de que:
SiteSetting.enable_s3_uploads = false
SiteSetting.enable_direct_s3_uploads = true
  1. Tente fazer upload de um arquivo.
  2. Observe o NoMethodError do FileStore::LocalStore.

Uma verificação do estado no console pode ser feita com:

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

Correção sugerida

Os registros de callbacks não devem sobrescrever uns aos outros.

Por exemplo, inclua as ações de upload seguro no callback existente 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
  ]

Alternativamente, use um método de callback separado para as ações de upload seguro.

Também pode valer a pena adicionar validação para que enable_direct_s3_uploads não possa ser habilitado enquanto os uploads S3/externos estiverem desabilitados.

Solução alternativa

Para sites que usam armazenamento de upload local:

SiteSetting.enable_direct_s3_uploads = false

previne o erro.

2 curtidas
2

Obrigado, deve ter sido corrigido conforme:

1 curtida