Make Binary behaviour extensible which might also be used for mysql adapter

[yhabteab]

This now makes it possible to extend the Binary behaviour and to additionally apply other behaviours to the mysql adapter as well, such as the IP behaviour of the x509 module.

fixes Icinga/icingaweb2-module-x509#172

[lippserd]

Let's discuss this PR tomorrow, please.

[lippserd]

Please update PR title and description as this change actually allows for subclassing the behavior.

[yhabteab]

We discussed offline with @nilmerg and agreed not to consider all characters that are not prefixed with \x as hex number. The x509 module already does this correctly by displaying in the search bar (when searching for the subject hash) as hex prefixed with \x. So, since icingadb-web will need a new release soon, binary data should be displayed there as hex number with \x prefixed as well. And thus makes the rewriteCondition implementation obsolete and is dropped, which never actually worked properly for what it was intended.

[lippserd]

agreed not to consider all characters that are not prefixed with \x as hex number

This is bytea hex format. \x$whatever is valid binary. Only ctype_xdigit() is safe. But I don't see why we want to support hex strings for inserts and updates.

binary data should be displayed ... as hex number with \x

No. Didn't even notice that this is already the case in the certificate monitoring module. If you really want to show that this is a hex string, prefix it with 0x. Though I strongly suggest not do that. If you cant see that the suggestions are hex strings, \x or 0x won't help you further. Also, this would require to add the prefix for manual values. And we would have to strip them in the behavior and still check whether it's a hex string since the prefix may just be the first characters of a binary string.

rewriteCondition ... which never actually worked properly for what it was intended

Please elaborate. Using hex strings worked perfectly before with the recent fixes I made in this PR. Of course only without the \x prefix.

[lippserd]

#105 (comment)

[nilmerg]

This is bytea hex format. \x$whatever is valid binary. Only ctype_xdigit() is safe.

bytea is a PostgreSQL type, which uses \xHEX by chance to represent binary as printable string. If you search for the \x prefix, you find nothing which hints at this being used strictly for binary strings, besides PostgreSQL of course.
ctype_xdigit is not safe. It only tells you that all characters are hexadecimal. Though, if you ask ctype_print it tells you it's ASCII. I know, I know, it's the same, but it shows why ctype_xdigit is not safe.

We need a prefix. Be it \x or 0x. To being able to independently detect that we have to decode hex in order to get the binary representation.

[yhabteab]

Please elaborate. Using hex strings worked perfectly before with the recent fixes I made in this PR. Of course only without the \x prefix.

Yes, the unit tests did run successfully, but in which use cases would rewriteCondition be useful at all? Binary#toDb() is always called before rewriteCondtion() and thus gets the already transformed(or binary for mysql adapter) data, which results in ctype_xdigit() always returning false.

[lippserd]

bytea is a PostgreSQL type, which uses \xHEX by chance to represent binary as printable string. If you search for the \x prefix, you find nothing which hints at this being used strictly for binary strings, besides PostgreSQL of course.

Yes, the unit tests did run successfully, but in which use cases would rewriteCondition be useful at all? Binary#toDb() is always called before rewriteCondtion() and thus gets the already transformed(or binary for mysql adapter) data, which results in ctype_xdigit() always returning false.

My point was that \x$whatever is valid binary data where hex2bin() must not be called just because it starts with \x. The same goes for any other prefix, e.g. 0x. That's why toDb() must not try to do any hex transformation.

Nothing must stop me from inserting hex values like 76616c7565 ('value') or 0x76616c7565. Now it's true that we would have a problem if we just used ctype_xdigit() to do the hex to binary transformation for filters. A prefix makes sense here. Still, we would have to remove the prefix and use ctype_xidigit() to check whether the remainders are hex digits. For example, 0xfoobar is valid binary and not hexadecimal. If I would've inserted 0x76616c7565 my filter must be 0x0x76616c7565 then where no transformation takes place. ctype_print() is of no use. You're thinking of binary data being unprintable characters only but that is just not the case.

[nilmerg]

Let's discuss this in person. Should prevent any further misunderstandings.......

[nilmerg]

Didn't test, though.

[lippserd]

Just for the records, do we use a prefix now or not? I have no problem not using a prefix. In the rare case that actual hex values are stored in the database and filters are composed manually, the user has to hex hex 😆. But this is fine for me.

@yhabteab Don't forget to adjust X.509 to not add the \x prefix for value suggestions in the search bar.

[yhabteab]

Just for the records, do we use a prefix now or not?

Only for PostgreSQL adapter.