getkirby / kirby

Kirby's core application folder
https://getkirby.com
Other
1.27k stars 167 forks source link

Quotation marks get deleted upon saving structure field #4493

Open fabianmichael opened 2 years ago

fabianmichael commented 2 years ago

Description

When nesting nested single and double straight quotes (' and ") into a text field within a structue field, the outer quotes get removed. Upon save.

Expected behavior
Quotes should be saved exactly as entered.

To reproduce

Setup any page with a structure field or use provided test case: https://github.com/fabianmichael/kirby-quotation-mark-bug

  1. Go to the panel and enter a test string with nested quotation mark into one of the fields within the strucutre field "This is a 'test' text."
  2. Save and reload the panel
  3. The outer quotes go removed when the content was saved.

Your setup

Kirby Version: 3.7.0.2 Your system: Tested on various macOS computers and Linux servers.

rasteiner commented 2 years ago

Go home Spyc, you're drunk... https://onlinephp.io/c/99fd8

The issue is reproduceable with Spyc (the yaml lib Kirby is using) with this code:

<?php

$stuff = [
    [
        "string" => '"quotes were here"'
    ]
];

$yaml = Spyc::YAMLDump($stuff);
$obj = Spyc::YAMLLoadString($yaml);

var_dump($obj);

/* Output:
array(1) {
  [0]=>
  array(1) {
    ["string"]=>
    string(16) "quotes were here"
  }
}
*/
rasteiner commented 2 years ago

Could be solved by adding " to the list of characters that cause a string to be encoded by a literal block, here.

Something like


                strpos($value, "&") !== false ||
                strpos($value, "'") !== false ||
+               strpos($value, "\"") !== false ||
                strpos($value, "!") === 0 ||
                substr($value, -1, 1) == ':'

See patched Spyc here: https://onlinephp.io/c/24996

edit: funnily, the patch doesn't work with the test string "This is a 'test' text.". 🤷

lukasbestle commented 2 years ago

Thanks for testing @rasteiner. If it's already caused by Spyc, we unfortunately cannot fix this in the short term. We have plans to replace Spyc with a more robust YAML library. I have added this issue to our list in https://github.com/getkirby/kirby/pull/4058.