heimrichhannot / contao-utils-bundle

This bundle offers various utility functionality for the Contao CMS.
GNU Lesser General Public License v3.0
8 stars 4 forks source link

Filedownload geht nicht richtig #22

Closed Olli closed 3 weeks ago

Olli commented 3 years ago

Context Contao version: 4.9.9 Bundle version: 2.185.0 PHP version: 7.4

Description

Ich habe im Template stehen

      {% for document in raw.documents | deserialize(true) %}
        {{ document | download(true) | raw }}
      {% endfor %}

Das erzeugt folgenden Link

<a href="seite/unterseite.html?file=files/content/Bilder/pdfs/Diagramme/Optimum%206%20und%208.pdf" title="Die Datei Optimum 6 und 8.pdf herunterladen">Optimum 6 und 8.pdf <span class="size">(887,2 KiB)</span></a>

Ich weiß nicht ob der href Pfad überhaupt richtig ist. Zumindest wird mir lediglich das HTML als "PDF" ausgeliefert und nicht das PDF selbst.

Defcon0 commented 3 years ago

Bei mir geht es wunderbar. Kann es sein, dass der Ordner nicht public ist? Dieser Parameter wird auch von Contao so gesetzt und dann ausgewertet (siehe Core Download-Element).

Olli commented 3 years ago

Das war der erste Punkt auf den ich geguckt habe - ja er ist public gesetzt. Bei einem Core Contao Downloadelement wird zwar auch so eine URL erzeugt aber da funktioniert der Download. Warum wird da eigentlich kein direkter Link in /files/ erzeugt?

Defcon0 commented 3 years ago

Da man so auch an Dateien kommt, die nicht public sind, also nicht durch den direkten Link aufzurufen sind. Bspw. interne Bereiche sind so ein Anwendungsfall. kannst du den Link direkt aufrufen? Teste es bitte mal mit einer Datei ohne Sonderzeichen (%206%20).

Olli commented 3 years ago

Ich habe es mit einer Datei ohne Leerzeichen und äöü getestet. Der Link direkt aufgerufen schickt eine leere Datei (mit den Webdevtools angeschaut). Der content-type ist allerdings PDF.

Defcon0 commented 3 years ago

Mhm, bei mir geht es tadellos in diversen Projekten. Kannst du dich dort mal reindebuggen? Mir fehlt ob des Jahresendes aktuell die Zeit, sry.

Olli commented 3 years ago

Ich habe in meiner Testinstall jetzt das alles so nachgestellt (auch obige Versionen) und einen PDF Download gestartet. Selbes Ergebnis. Hast du auch wirklich das PDF im Viewer angeschaut oder nur den Download Dialog gesehen? :_)

Olli commented 3 years ago

Ich kenne mich mit Contao nicht so aus. Wo muss ich denn schauen um heraus zu finden, wo die Dinge verarbeitet werden die per "?file=" an eine URL Übergeben werden?

Defcon0 commented 3 years ago

Ah, Fehler gefunden. Dein obiger Hinweis war zielführend ;-) Ich fixe es mal und gebe dann noch mal Bescheid

Defcon0 commented 3 years ago

OK, das war gut versteckt :-)

Es scheint dann zum Fehler zu kommen, wenn vor der Download-Umleitung bereits Content im ob ausgegeben war. Man muss also vorher cleanen.

Kannst du bitte in vendor/heimrichhannot/contao-utils-bundle/src/Twig/DownloadExtension.php

das hier

try {
                Controller::sendFileToBrowser($requestedFile);
            } catch (ResponseException $e) {
                $e->getResponse()->send();
            }

durch das hier ersetzen

try {
                Controller::sendFileToBrowser($requestedFile);
            } catch (ResponseException $e) {
                ob_clean();
                $e->getResponse()->send();
            }

und schauen, ob es dann bei dir geht?

Olli commented 3 years ago

Genau das war es. Download funktioniert. Thx.

Defcon0 commented 3 years ago

Fixed in 2.186.0

Olli commented 3 years ago

Ok interessanterweise ist das bei mir lokal gefixt, bei All-Inkl läuft es auch aber auf dem Stratohosting nicht. Kannst du dir einen Grund dafür vorstellen? All-Inkl geht auch nicht.

Olli commented 3 years ago

Zusatzinfo: Ich habe lokal den Debugmodus deaktiviert. Jetzt gibt es selbigen oben beschrieben Fehler auch lokal. Vielleicht hilft der Umstand weiter.

Defcon0 commented 3 years ago

Ja, es liegt am Prod-Mode in Verbindung mit dem Senden des BinaryResponse im Kontext eines Twig-Renderings. Wenn ich denselben Code in die ContentElement-Klasse packe, geht er. Es scheint im Twig-Rendering irgendetwas gemacht zu werden, was dazu führt. Ich vermute, dass Twig im Prod-Mode Exceptions (wie bspw. unsere ResponseException, die das Binary enthält) abfängt, damit man nichts sehen kann. Es werden vielleicht dann alle Exception-Ausgaben erst mal genullt. Daher ist dann auch die Datei leer.

Damit wäre ein Downloaden im Prod-Mode aus einem Twig-Filter heraus quasi unmöglich.

@koertho Hast du noch eine Idee, was man noch versuchen könnte?

Du könntest alternativ aus dem

https://github.com/heimrichhannot/contao-inserttagcollection-bundle

den download-Inserttag in twig nutzen:

{{ '{{download::files/media/something.pdf}}'|raw }}

Der funktioniert (gerade getestet in 4.4 dev und prod), da es dort nicht im Twig-Render-Kontext passiert.

Olli commented 3 years ago

Das ist etwas betrüblich. Wie bekomme ich die UUID in den Inserttag? Der wird doch in Binärform im Template zur Verfügung gestellt und der Inserttag nimmt den als String.

Defcon0 commented 3 years ago

Es gibt ja im Reader/List das Konzept von formatierten Feldern (findest du in der Leser- bzw. Listenkonfiguration). Wenn du da dein Feld, was eine UUID enthält, mit in die Liste der formatierten Felder aufnimmst und dann nicht mit raw.documents sondern mit documents darauf zugreifst, sollte es den Pfad enthalten.

Ansonsten könnten wir auch den InsertTag erweitern, sodass er auch UUIDs nimmt. Aber versuche es erst mal so.

Olli commented 3 years ago

Aha danke :) Daran hatte ich gar nicht gedacht.

Olli commented 3 years ago

Das funktioniert so wie gedacht. Ich weiß jetzt nicht ob ich das Ticket schließen soll. Zumindest wäre ein Hinweis über dieses Issue in der Readme vielleicht angemessen.

Defcon0 commented 3 years ago

Was man machen könnte: man könnte das inserttag bundle als suggest in die composer.json nehmen und wenn man den betroffenen twig filter nutzt, als text ausgeben, dass man das inserttag bundle installieren muss, wenn man den tag nutzen will. aktuell funktioniert er ja ohnehin nicht richtig. Dann könnte man im twig filter einfach den inserttag zurückgeben.

@koertho Was meinst du?

koertho commented 3 years ago

Ich würde den Twig-Tag reparieren. Aber dass muss ich mir in Ruhe anschauen, wie das überhaupt umgesetzt ist. Wir lassen das Issue offen, in die Readme muss dazu nichts, dafür sind Issues ja da.

Defcon0 commented 3 years ago

OK, dann schau mal, ob du vllt noch eine Lösung findest, die nicht das Umwandeln in einen InsertTag im Hintergrund beinhaltet. Wird durch den Twig-Rendering-Prozess sicher schwierig. Oder ich habe was übersehen ;-)

koertho commented 3 years ago

Also ich habe es in Contao 4.4.55 mit utils bundle 2.188.0 und 4.9.10 mit utils bundle 2.188.1 getestet und es ging sowohl im dev und prod mode.

So sieht es aus, habe es in ein ce_text.html.twig template drin gehabt (über twig-support-bundle):

{{ 'files/media/test/loremipsum.pdf' | download | raw }}
{{ '46cfd7a9-0d25-11eb-a53b-c4651626d7c0' | download | raw }}
Olli commented 3 years ago

Naja aus der Datenbank kommt aber kein '46cfd7a9-0d25-11eb-a53b-c4651626d7c0' (also ein String) sondern eine binäre Representation davon. kann man sich denn bei der Schleife

{% for document in raw.documents | deserialize(true) %}

oder der

{% for document in documents | deserialize(true) %}

bei "document" auch eine String Representation der GUID zuweisen lassen?

Defcon0 commented 3 years ago

Nee, das geht aktuell nicht. Dafür müsste man einen neuen Twig-Filter schreiben. documents enthält in raw ein Array der binären UUIDs und im normalen Fall ein Array der generierten Pfade.