Core: ConversationDB extracts first character only from [notes] db entry with php 7.1.1

[sprnza]

Hi guys!

Required Information

• PHP version: 7.1.1
• PHP Telegram Bot version:
• Using MySQL database: yes
• Update Method: Webhook
• Self-signed certificate: yes
• RAW update (if available):

Error description

After a string has been added to a database using
this->conversation->notes['option']
If I later assign this value to a variable it contains of first symbol of the initial string only.
Say I have a string "List" in a $text variable.
I do
$this->conversation->notes['option'] = $text;
After an attempt to get it back using:
$param = $this->conversation->notes['option'];
$param gets "L".

[sprnza]

I saw you guys use json_encode to store ['notes']. I'm not sure if it will be useful for you but I've found this in PHP 7.0 -> PHP 7.1 migrating notes:

JSON encoding and decoding ¶

The serialize_precision ini setting now controls the serialization precision when encoding doubles.

Decoding an empty key now results in an empty property name, rather than _empty_ as a property name.
var_dump(json_decode(json_encode(['' => 1])));

The above example will output something similar to:

object(stdClass)#1 (1) {
[""]=>
int(1)
}

When supplying the JSON_UNESCAPED_UNICODE flag to json_encode(), the sequences U+2028 and U+2029 are now escaped.

[sprnza]

Oops, sorry it's not DB related. Just checked and see that the data is being gotten correctly. Keep investigating.

[sprnza]

Just double checked. It's definitely DB related issue. Please help!

[sprnza]

Aha! The [note] is not being encoded at all! The DB gets a state number only.

[thebigsmileXD]

I have to note some things here

After a string has been added to a database using
this->conversation->option['notes']
If I later assign this value to a variable it contains of first symbol of the initial string only.
Say I have a string "List" in a $text variable.
I do
$this->conversation->notes['option'] = $text;

Here you access notes['option'], before you used option['notes']

Is this intended?

[sprnza]

Sorry, it's a typo. I've fixed it.

[sprnza]

Here is how a row of a DB looked like before:
| 471 | 213322544 | 213322544 | stopped | mpc | {"option":"Play Artist","state":1}| 2016-11-17 17:47:56 | 2016-11-17 17:48:26 |
And now (/survery example command : Enter your surname step):
| 569 | 213322544 | 213322544 | active | survey | "1" | 2017-01-20 19:47:38 | 2017-01-20 19:47:41 |
Notes field receives just "1" - state number.

[sprnza]

This code:

<?php
    $option = [];
    $option['text'] = "test";
    $option['test'] = "text";
    $encoded = json_encode($option);
    echo $encoded."</br>";
    $version = phpversion();
    echo "PHP version: " . $version;
?>

returns this though:

{"text":"test","test":"text"}
PHP version: 7.1.1

[sprnza]

So the root cause of errors was a line #97 of ConversationDB.php changing it to
notes = [];
solves the issue.
Otherwise $notes was a string.

[sprnza]

I haven't found anything regarding this in Backward incompatible changes list though.

[noplanman]

@sprnza Thanks for reporting!

I can't reproduce this using PHP 7.1.0 (or any other version).
The /survey command works as expected for me.

Could you please post your whole command that makes use of the conversation feature?

[sprnza]

PHP 7.1.1 is one which causes the issue.
/survery doesn't work with it.

[sprnza]

I didn't have 7.1.0 installed though so I wasn't able to check with it. I've updated from 7.0.x.

[sprnza]

with notes = '""' gettype($notes) from Conversation.php right before jason_encode was returning "string".

[sprnza]

The only "String to Array"-thing I've found is this.

The empty index operator is not supported for strings anymore

Applying the empty index operator to a string (e.g. $str[] = $x) throws a fatal error instead of converting silently to array

But I wasn't getting a fatal error.

[noplanman]

with notes = '""' gettype($notes) from Conversation.php right before jason_encode was returning "string".

Yes, because the database field is a string, JSON encoded.

It seems the changes in 7.1.1 aren't related to this issue.

But I wasn't getting a fatal error.

Because there shouldn't be any code like that in the conversations (as far as I know!).

You have version 0.38.1 of the library, right? (didn't say in your initial comment)

[sprnza]

Yes, because the database field is a string, JSON encoded.

Before it's being encoded it should be an array right? I had it appeared as a string.

Because there shouldn't be any code like that in the conversations (as far as I know!).

$notes is being declared as a string in ConversationsDB.php. My PHP kung fu is not very good so I haven't found how's it being converted to an array.

You have version 0.38.1 of the library, right? (didn't say in your initial comment)

Actually no, IDK how to update keeping my changes untouched. (shame on me) I create my custom commands in Commands/ directory but I've made some changes in vendor/longman/telegram-bot/src/Commands/Command.php. How to keep them?

It seems the changes in 7.1.1 aren't related to this issue.

I've got the issue right after PHP got updated. Another packages that were updated:
[2017-01-19 18:56] [ALPM] upgraded imagemagick (6.9.7.3-1 -> 6.9.7.4-1) [2017-01-19 18:56] [ALPM] upgraded libtasn1 (4.9-2 -> 4.10-1) [2017-01-19 18:56] [ALPM] upgraded libcups (2.2.1-1 -> 2.2.2-1) [2017-01-19 18:56] [ALPM] upgraded libpulse (9.0-1 -> 10.0-1) [2017-01-19 18:56] [ALPM] upgraded libxkbcommon (0.7.0-1 -> 0.7.1-1) [2017-01-19 18:56] [ALPM] upgraded libxkbcommon-x11 (0.7.0-1 -> 0.7.1-1) [2017-01-19 18:56] [ALPM] upgraded noto-fonts (20161221-1 -> 20170112-1) [2017-01-19 18:56] [ALPM] upgraded mplayer (37857-1 -> 37916-1) [2017-01-19 18:56] [ALPM] upgraded nfs-utils (1.3.4-1 -> 2.1.1-1) [2017-01-19 18:56] [ALPM] warning: /etc/php/php.ini installed as /etc/php/php.ini.pacnew [2017-01-19 18:56] [ALPM] upgraded php (7.0.14-1 -> 7.1.1-1) [2017-01-19 18:56] [ALPM] upgraded php-apache (7.0.14-1 -> 7.1.1-1) [2017-01-19 18:56] [ALPM] upgraded php-apcu (5.1.7-1 -> 5.1.8-1) [2017-01-19 18:56] [ALPM] upgraded php-cgi (7.0.14-1 -> 7.1.1-1) [2017-01-19 18:56] [ALPM] upgraded php-gd (7.0.14-1 -> 7.1.1-1) [2017-01-19 18:56] [ALPM] upgraded php-intl (7.0.14-1 -> 7.1.1-1) [2017-01-19 18:56] [ALPM] upgraded php-mcrypt (7.0.14-1 -> 7.1.1-1) [2017-01-19 18:56] [ALPM] upgraded php-sqlite (7.0.14-1 -> 7.1.1-1) [2017-01-19 18:56] [ALPM] upgraded pulseaudio (9.0-1 -> 10.0-1) [2017-01-19 18:56] [ALPM] upgraded pulseaudio-bluetooth (9.0-1 -> 10.0-1) [2017-01-19 18:56] [ALPM] upgraded python2-cherrypy (8.8.0-1 -> 8.9.1-1) [2017-01-19 18:56] [ALPM] upgraded xorg-mkfontdir (1.0.7-5 -> 1.0.7-7)

[noplanman]

Before it's being encoded it should be an array right? I had it appeared as a string.

I see, yes, it should be $notes = '[]'; here.
(basically json_encode([]); => (string) '[]')

You're on an older version of the library and your /survey command isn't up to date.
The latest /survey command works due to this line, which creates the array if it isn't one.
(Admittedly, the correct place to fix it would be in ConversationsDB.php)

Now, to get your library up to date:
Move all custom commands into a separate folder and add that folder as a commands path using $telegram->addCommandsPath('/path/to/your/commands'); in your hook file.

What changes have you made to Command.php?
Maybe we can incorporate some directly into the core of the library?
Otherwise, save that file and move your changes to the updated one.

If you have any questions, just open a new issue to keep this one clean.

[sprnza]

Thanks @noplanman! I'll try to update it!

What changes have you made to Command.php?

My bot is private so I've added this:

 public function preExecute()
    {
    $allowedChats = array("213322544", "147378243", "240670959", "246956999");
    if (in_array($this->getMessage()->getChat()->getID(), $allowedChats)) {
        if ($this->need_mysql && !($this->telegram->isDbEnabled() && DB::isDbConnected())) {
            return $this->executeNoDb();
        }
        return $this->execute();
    } else {
    return $this->executeWrongChat();
    }
    }

    /**
     * Execute command
     *
     * @return Entities\ServerResponse
     */
    abstract public function execute();

// Execution if CHAT[ID] is wrong
    public function executeWrongChat()
    {
        //Preparing message
        $message = $this->getMessage();
        $chat_id = $message->getChat()->getId();

        $data = [
            'chat_id' => $chat_id,
            'text'    => 'Sorry the bot is private. Chat id is: ' . $chat_id,
        ];

        return Request::sendMessage($data);
    }

[noplanman]

Initially, the idea was to let developers decide how to use the notes field, so they can add whatever they like.
It does make sense to have it start off as an array though, for the exact reasons as you've described above, that the natural inclination is to have an array of notes there.

Even having it as an array by default, developers still have the freedom to do whatever they like 👍

[noplanman]

@sprnza 0.39.0 is out, NOW! :wink:

Remember to update your database too, check the structure.sql file to compare.