Diverse Probleme mit Payone

[bennyborn]

Hallo zusammen,

habe soeben einen Isotope-Shop mit Payone angebunden, dabei bin ich auf zwei doofe Probleme gestoßen.

Payone übermittelt die Parameter mod und id per POST anstatt GET. Das ist eigentlich ja kein Problem. Blöderweise gibt es aber auch einen Payone-internen Parameter der id heißt und somit den anderen überschreibt. Ich glaube hier müsste man alternativ zu mod und ideinfach noch auf "gepräfixte" Varianten wie z.B. contao_iso_mod und contao_iso_pay prüfen.

Weiterhin wird in der Payone.php in Zeile 141 der Wert eines jeden Formularfelds URL-encoded was aber bei der successurl und backurl Probleme macht. Payone baut daraus dann eine falsche Weiterleitung zusammen und versucht den User auf sowas wie https://secure.pay1.de/frontend/http%3A%2F%2Fmeinshop.de%2Fkasse%2Ffailed.html zu schicken.

[Toflar]

Das 1. Problem verstehe ich nicht ganz? Muss bei Payone die Callback-URL eingegeben werden? Weil gemäss Code liefern wir die ja nicht mit :) Warum also sollte da ein Problem entstehen?

Das 2. Problem ist ein Problem von Payone. POST-Daten mit Content-Type application/x-www-form-urlencoded müssen per Definition URL-encoded gesendet werden. Sonst könnte ja euer Server nicht wissen, wo ein Key-Value-Paar anfängt und wo es aufhört.

[bennyborn]

Zu Problem 1: Jupp, Payone benötigt in den Einstellungen die Callback-URL ansonsten kann Isotope ja nicht nachvollziehen ob die Zahlung erfolgreich war. Wird ja auch in Zeile 29 (https://github.com/isotope/core/blob/master/system/modules/isotope/library/Isotope/Model/Payment/Payone.php#L29) geprüft :) Und wie gesagt der Parameter id ist schon von Payone selbst in deren Response vorbelegt :/

Zu Problem 2: Das stimmt zwar aber ich glaube nicht das wir Payone dazu bekommen können das zu fixen, ich denke hier sollten wir einfach selbst tätig werden damit es funktioniert und ohne URL-Encoding klappt es :)

[Toflar]

Problem 1: Also ich versuche zu verstehen: Die Code-Stelle die du nennst hat damit ja nichts zu tun. Das wird in der postsale.php gemacht. Und so wie ich das sehe meinst du, es kommt zu einem Problem weil Payone an domain.de/system/modules/isotope/postsale.php?mod=pay&id=15 POST-Werte schickt, die ebenfalls einen Parameter id enthalten (auch wenn der Isotope eigentlich gar nicht interessiert) und somit gewinnt der Body-Parameter ($_POST) gegenüber dem Query-Parameter ($_GET). Richtig?

Problem 2: Was soll ich dazu sagen? Wieso sollte Payone kein Interesse haben, fundamentale Fehler in HTTP-Handling zu beheben? Ich kann's schon ändern, auch wenn mich das extrem stört...(#KANNSTEMACHEN). Was genau denn, nur die beiden URL's oder soll jegliches URL-Encoding weg?

[bennyborn]

Problem 1: Damit wollte ich nur verdeutlichen das Payone auf Postsale angewiesen ist. Ansonsten weis man ja nicht ob das Payment erfolgreich war oder nicht :) Payone schickt nur an domain.de/system/modules/isotope/postsale.php und packt sämtliche GET-Parameter in den POST-Body. Sprich id und mod werden nicht per GET übertragen sondern nur per POST, jedoch befindet sich im POST schon ein Parameter id der den der Postsale überschreibt. Du liegst also im Grunde richtig :)

Und warum sollte Isotope bzw postsale.php der ID-Parameter nicht interessieren? https://github.com/isotope/core/blob/master/system/modules/isotope/postsale.php#L81 fehlt id oder mod ist der Postsale-Request ungültig ;)

Problem 2: Naja, aus dem selben Grund warum sie GET-Parameter einfach mal in den POST-Body packen - sie sind der Meinung das es so gemacht werden sollte :D Würde mich (und andere bestimmt auch) freuen wenn Du das umsetzen könntest :) Es müssen lediglich successurl und backurl aus dem Encoding rausgenommen werden, der Rest muss so bleiben.

[Toflar]

Ich habe zwei Commits die du testen musst. Der erste ist hier referenziert und bezieht sich auf das URL-Encoding der Success- und Back-URLs.

Der zweite ist der folgende: https://github.com/isotope/core/commit/d1a8e7f1a3bd3ea600ca1c603ff2644845a5b4fb Er ermöglicht das dynamische Überschreiben der Parameter mod und id. Für Payone habe ich jetzt den Parameter param verwendet, welcher gemäss Doku für eigene Dinge verwendet werden kann. Insofern musst du jetzt auch nichts mehr übergeben, sondern kannst einfach nur domain.de/system/modules/isotope/postsale.php verwenden.

[aschempp]

ich gehe davon aus das ist erledigt, wurde mit Isotope 2.1.3 ja so ausgeliefert.

[Toflar]

Nein, nicht erledigt. Der zweite Teil muss noch getestet werden.

[bennyborn]

Ich habe leider aktuell keinen Payone-Account um das ganze zu testen. Habe zwar schon bzgl einem Testaccount nachgefragt aber die Jungs lassen sich Zeit...

[seaneble]

Eine Verständnisfrage: Ich habe ein ähnliches Problem, weil im Postsale Request folgende Parameter vorkommen: id, id[1] und id[2].

Die Input-Klasse von Contao liefert nach der Frage nach \Input::post('id') nicht etwa den Wert des ersten Parameters aus, sondern baut aus dem zweiten und dritten Parameter einen Array:

array(2) {
  [1]=>
  string(8) "sku-1"
  [2]=>
  string(10) "surcharge0"
}

Ich frage mich an der Stelle nun, ob mir mit Yanicks Patch geholfen wird, oder ob die Input-Klasse von Contao eine Macke hat, denn aus zwei unterschiedlichen String-Parametern ein Array zu bauen ist auch etwas gewagt. Oder müssten die eckigen Klammern einfach auf PAYONE-Seite als %5B und %5C abgeschickt werden? PAYONE-Zugang habe ich mehr als genug, ich bin also bereit, alles zu testen, was hilft…

[Toflar]

a) Ja der Feature Branch sollte helfen, bitte testen :) b) wenn du id[1] und id[2] als Parameter hast, dann ist das ja ein Array und somit korrektes Verhalten von der Input-Klasse

[seaneble]

Soweit ich inzwischen weiß, kann man der Input-Klasse von Contao so viele Vorwürfe nicht machen, da gemäß RFC3986 eckige Klammern gar nicht erlaubt sind. Mein Telefonat mit PAYONE ergab, dass man sehr bedauert, den Standard falsch gelesen zu haben, das sei aber nun so. Und den Code von Yanick habe ich inzwischen gelesen und denke, das wird für mich passen. Ich teste gleich mal.

[seaneble]

So, mit #1168 funktioniert dein Code, Yanick.

[Toflar]

Danke für's Feedback. Dann kann ich ja den Branch mergen und das Ticket schliessen.

[Toflar]

Merged https://github.com/isotope/core/commit/de39051e6161d7f6c593899729e1b0fd24c47f2a.