Direkter S3-Upload mit lokaler Speicherung löst NoMethodError aus statt abgelehnt zu werden

Beitragen Fehler
1

Ich habe dieses Problem nach einer Meldung eines Clients selbst gefunden und behoben. Ich habe KI verwendet, um den Bug-Report zu generieren, weil ich faul war und sie einen besseren Job macht als ich – und dabei einen Kernfehler entdeckt, den ich manuell verifiziert habe.

Nichts Großes, aber es könnte sinnvoll sein, zu prüfen, ob dieses Muster auch an anderen Stellen zu Problemen führt.

Alles begann damit, dass jemand enable_direct_s3_uploads aktiviert hatte, ohne S3 tatsächlich zu konfigurieren und zu aktivieren, was alle Uploads zum Erliegen brachte und im Hintergrund 500-Fehler zurückgab. Ich dachte, der 500-Fehler sei es wert, untersucht zu werden.

Zusammenfassung

Wenn enable_direct_s3_uploads aktiviert ist, während der aktive Upload-Store lokal ist, kann ein Upload-Versuch folgende Ausnahme auslösen:

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'

Dies scheint durch eine Kombination aus einem ungültigen Einstellungszustand und einem überschriebenen before_action-Callback verursacht zu werden.

Konfiguration

Der problematische Zustand ist im Wesentlichen:

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

Für lokale Speicherung implementiert Discourse.store nicht:

signed_request_for_temporary_upload

was erwartbar ist, da diese Operation nur für einen externen/S3-Store Sinn ergibt.

Erwartetes Verhalten

Wenn direkte S3-Uploads aktiviert sind, während der aktive Store lokal ist, sollte die Anfrage sauber durch external_store_check abgelehnt werden.

Idealerweise sollte auch die ungültige Kombination von Site-Einstellungen verhindert oder validiert werden.

Tatsächliches Verhalten

Die Anfrage erreicht ExternalUploadManager.create_direct_upload, das aufruft:

store.signed_request_for_temporary_upload(...)

auf einem FileStore::LocalStore, was zu einem NoMethodError führt.

Mögliche Ursache

ExternalUploadHelpers registriert den folgenden Callback:

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

Dies sollte verhindern, dass die Anfrage den externen Upload-Code erreicht, wenn der Store lokal ist.

Allerdings registriert UploadsController später einen weiteren Callback mit derselben Filtermethode:

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

Rails behandelt die wiederholte Registrierung desselben Callbacks als Neudefinition, daher scheint der spätere Callback die früheren only:-Bedingungen zu ersetzen.

image

Infolgedessen wird external_store_check nicht mehr für generate_presigned_put ausgeführt, wodurch ein lokaler Store den Code-Pfad für direkte Uploads erreichen kann.

Reproduktion

  1. Konfiguriere eine Discourse-Instanz mit lokalem Upload-Speicher.
  2. Stelle sicher, dass Folgendes gilt:
SiteSetting.enable_s3_uploads = false
SiteSetting.enable_direct_s3_uploads = true
  1. Versuche, eine Datei hochzuladen.
  2. Beobachte den NoMethodError von FileStore::LocalStore.

Eine Konsolenprüfung des Zustands kann mit Folgendem durchgeführt werden:

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

Vorschlag zur Behebung

Die Callback-Registrierungen sollten einander nicht überschreiben.

Zum Beispiel sollten die sicheren Upload-Aktionen in den bestehenden external_store_check-Callback aufgenommen werden:

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
  ]

Alternativ kann eine separate Callback-Methode für die sicheren Upload-Aktionen verwendet werden.

Es könnte auch sinnvoll sein, eine Validierung hinzuzufügen, sodass enable_direct_s3_uploads nicht aktiviert werden kann, während S3-/externe Uploads deaktiviert sind.

Workaround

Für Sites, die lokalen Upload-Speicher verwenden:

SiteSetting.enable_direct_s3_uploads = false

verhindert den Fehler.