Banküberweisung Zeilenumbruch

[RabbitZzZ]

Hallo,

immer wenn ich im Banküberweisungsmodul beim Konto einen Zeilenumbruch einfüge, ist dieser nach dem Speichern verschwunden.
Auch HTML Codes werden entfernt. Gibt es irgendwie eine Möglichkeit die Kontodaten auf mehrere Zeilen aufzuteilen?

Danke!

[Occam]

Eigentlich sollte das beim Speichern aber nicht passieren.

EDIT: Habe es gerade nochmal in der Datenbank überprüft. Du hast recht. Obwohl das aus dem Quellcode nicht ersichtlich ist, werden alle HTML-Steuerzeichen aus dem Text entfernt, bevor er als Variable BANK_WIRE_DETAILS in der Datenbank abgespeichert wird. Fügt man manuell ein <br> in den Text ein, dann wird sogar alles, was danach kommt, beim Speichern abgeschnitten.

Das solltest du als Bug melden.

Edited November 11, 2019 by Occam

[RabbitZzZ]

Ah ok, also ist es ein Bug. Ich erinnere mich nämlich auch, dass es schon mal funktioniert hatte.

[Occam]

Es liegt an einer foreach-Schleife in der Funktion updateValue in der Datei /classes/Configuration.php. (Oder genauer gesagt, an der Aufsplittung einer Funktion in zwei, bei der bestimmte Informationen anscheinend verloren gehen.)

Wenn du nicht warten willst, bis der Bug behoben wird, dann kopiere einfach die gesamte Funktion aus der Version thirty bees 1.0.8 und ersetze damit die gleichnamige Funktion in thirty bees 1.1.0. Dann ist der Bug schon mal zumindest außer Kraft gesetzt und es funktioniert wieder.

Edited November 12, 2019 by Occam

[RabbitZzZ]

@Occam vielen Dank! Ich habe es eben schon auf Git gesehen und behoben.

[Occam]

Da hätte ich vieleicht auch als erstes nachgucken sollen! Und wo genau? Denn ich finde nur das hier: https://github.com/thirtybees/thirtybees/commit/8e89acf9061e88bc106658891c8bd5abb83eeb9c#diff-f44483a142f09ca47bd59224eb070bfb
Und damit hat sich eigentlich erst der Bug eingenistet.

Hast du mal einen Link für mich?

Edited November 12, 2019 by Occam

[RabbitZzZ]

Da https://github.com/thirtybees/bankwire/commit/914010b77f152f7fc487c5bb116a314eb7597f83

[datakick]

https://github.com/thirtybees/thirtybees/commit/d6ca9f6c334cb03174eeb7b75006a5c4fa589c0d

[Occam]

I checked this once more, and it seems that if you omit the else-branch in function function updateValue of Configuration.php completely, the corresponding change in bankwire.php, though explicit, seems  to be superfluous.

    public static function updateValue($key, $values, $html = false, $idShopGroup = null, $idShop = null)
    {
        if (!is_array($values)) {
            $values = [$values];
        }

        // sanitize values
        foreach ($values as &$value) {
            if (! is_numeric($value)) {
                if ($html) {
                    // if html values are allowed, just purify html code
                    $value = Tools::purifyHTML($value);
                //} else {
                    // if html values are not allowed, strip tags
                   // $value = strip_tags(Tools::nl2br($value));
                }
            }
        }

I did not find out yet the reason why the boolean variable $html seems to be true by default.

[datakick]

That's true. If we were to remove the else branch, then Configuration::updateValue($key, $value) call would not remove html tags from values stored into configuration table. 

And that would be a HUGE security bug.

In other words -- if programmer wants to store html code, he need to be very explicit about it. He must evaluate the context in which data are provided, and decide if it's safe to allow html code to enter the system or not. He needs to know where the data will go, and how it will impact rendered output of these pages.

[Occam]

Maybe, but I guess you missed the point:

2 hours ago, Occam said:

the boolean variable $html seems to be true by default.

Even with

Configuration::updateValue('BANK_WIRE_DETAILS', Tools::getValue('BANK_WIRE_DETAILS'), false);

in bankwire.php the string will be stored formatted in tb_configuration. The parameter seems to be always $html = true when you call this function.

[datakick]

12 minutes ago, Occam said:

Maybe, but I guess you missed the point:

Even with

Configuration::updateValue('BANK_WIRE_DETAILS', Tools::getValue('BANK_WIRE_DETAILS'), false);

in bankwire.php the string will be stored formatted in tb_configuration. The parameter seems to be always $html = true when you call this function.

No, it's  not. I've just tested it. 

I tried to save this string:

Hello <strong>there</strong>

With false, it's saved as 

Hello there

With true, it's saved correctly:

Hello <strong>there</strong>

Note that you have to re-load the page, because this module (like most of them) shows last $POSTed value, instead of what was actually saved into db. That can be, of course, considered as another bug. This is probably why you thought it is always called with `true` parameter.

 

[datakick]

Let's fix this newly discovered bug, where we are at this topic:

https://github.com/thirtybees/bankwire/commit/51e3afb916fdc83111628ca04de254dbbc0a2674

[Occam]

1 hour ago, datakick said:

Note that you have to re-load the page, because this module (like most of them) shows last $POSTed value, instead of what was actually saved into db.

Well, I always doublechecked: the database entry AND the reloaded module page.

[datakick]

5 minutes ago, Occam said:

Well, I always doublechecked: the database entry AND the reloaded module page.

Then you have some override installed, or modified core code. 

There is actually a test that verifies behavior of this method, and it runs on every git commit.

[Occam]

14 hours ago, datakick said:

Then you have some override installed, or modified core code.

No, it is a fresh 1.1.0, upgraded to Bleeding Edge. The only modifications I made were your proposals.

Technically spoken: With reference to my tests in tb itself I would not dispute the validity of your test, but the reliability.

Edited November 15, 2019 by Occam

[Occam]

Update ... and just for the record:

a) $html = false (default value) --> removes all visible html tags of a given textarea, incl. added <br> for a linefeed ( 
 ). But it does not affect a hard carriage return ( 
 
). Every new line you start in a textarea field will be treated as a linebreak, no matter if you finish the previuous line with a RETURN (linebreak) or a SHIFT RETURN (linefeed).

b) $html = true --> keeps all visible html tags of a given textarea and changes every <br> to <br /> for compatibility reasons.

Cases a) and b) Linebreaks (&#13; &#10;) in a textarea will always be maintained in the database field (after the bug fix above). The current function updateValue() will not remove linebreaks in a textarea.

Case a) Each html tag will be removed before storing in the database.

Case b) All html tags incl. <br> will be kept in the database.

==> With the current modifications the bankwire details and address seem to work properly. Thank you, @datakick. I was just puzzled because the line breaks did not disappear when setting $html to default like before the bug fix.

Edited November 15, 2019 by Occam