Il caricamento diretto su S3 con store locale genera NoMethodError invece di essere rifiutato

Contribuisci Bug
1

Ho trovato e risolto questo problema da solo dopo che un cliente lo aveva segnalato. Ho usato l’IA per generare il report del bug perché ero pigro e lo fa meglio di me - e nel farlo ha scoperto un bug nel codice principale, che ho verificato manualmente.

Non è nulla di grave, ma potrebbe avere senso verificare se questo schema sta causando problemi anche in altri luoghi.

Tutto è iniziato quando qualcuno ha spuntato enable_direct_s3_uploads senza effettivamente configurare e abilitare S3, il che ha rotto tutti gli upload, restituendo errori 500 in background. Ho pensato che l’errore 500 valesse la pena di essere indagato.

Riepilogo

Quando enable_direct_s3_uploads è abilitato mentre l’archivio di upload attivo è locale, il tentativo di caricare un file può generare:

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'

Questo sembra essere causato da una combinazione di uno stato di configurazione non valido e di un callback before_action sovrascritto.

Configurazione

Lo stato problematico è sostanzialmente:

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

Per lo storage locale, Discourse.store non implementa:

signed_request_for_temporary_upload

il che è previsto, poiché quell’operazione ha senso solo per un archivio esterno/S3.

Comportamento atteso

Se gli upload S3 diretti sono abilitati mentre l’archivio attivo è locale, la richiesta dovrebbe essere rifiutata in modo pulito da external_store_check.

Idealmente, la combinazione di impostazioni del sito non valida dovrebbe anche essere prevenuta o validata.

Comportamento attuale

La richiesta raggiunge ExternalUploadManager.create_direct_upload, che chiama:

store.signed_request_for_temporary_upload(...)

su un FileStore::LocalStore, risultando in un NoMethodError.

Possibile causa

ExternalUploadHelpers registra il seguente callback:

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

Questo dovrebbe impedire che la richiesta raggiunga il codice di upload esterno quando l’archivio è locale.

Tuttavia, UploadsController registra successivamente un altro callback usando lo stesso metodo di filtro:

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

Rails tratta la registrazione ripetuta dello stesso callback come una ridefinizione, quindi quest’ultimo sembra sostituire le condizioni only: precedenti.

image

Di conseguenza, external_store_check non viene più eseguito per generate_presigned_put, permettendo a un archivio locale di raggiungere il percorso del codice di upload diretto.

Riproduzione

  1. Configura un’istanza di Discourse con storage di upload locale.
  2. Assicurati che:
SiteSetting.enable_s3_uploads = false
SiteSetting.enable_direct_s3_uploads = true
  1. Cerca di caricare un file.
  2. Osserva il NoMethodError da FileStore::LocalStore.

Un controllo dello stato tramite console può essere fatto con:

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

Correzione suggerita

Le registrazioni dei callback non dovrebbero sovrascriversi a vicenda.

Ad esempio, includi le azioni di upload sicuro nel callback external_store_check esistente:

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
  ]

In alternativa, usa un metodo di callback separato per le azioni di upload sicuro.

Potrebbe anche valere la pena aggiungere una validazione in modo che enable_direct_s3_uploads non possa essere abilitato mentre gli upload S3/esterni sono disabilitati.

Soluzione temporanea

Per i siti che utilizzano lo storage di upload locale:

SiteSetting.enable_direct_s3_uploads = false

previene l’errore.