Laravel-auditing: Binary data in model causes JsonEncodingException

[SebastianS90]

| Q | A
| ----------------- | ---
| Bug? | yes, at least for my usecase
| New Feature? | maybe?
| Framework | Laravel
| Framework version | 5.6.22
| Package version | 7.0.0
| PHP version | 7.2.5

Actual Behaviour

Auditing a model that holds binary data results in a JsonEncodingException.

Expected Behaviour

The audit should convert the binary data before casting it to json.

Steps to Reproduce

I have a MyModel that has some attribute foo wich is stored in binary form in the database. The model comes with getFooAttribute and setFooAttribute methods that convert from binary to string and back.

When creating a MyModel, then the following exception occurs:

Illuminate\\Database\\Eloquent\\JsonEncodingException(code: 0): Unable to encode attribute [new_values] for model [App\\Models\\Audit] to JSON: Malformed UTF-8 characters, possibly incorrectly encoded. at PATH/vendor/laravel/framework/src/Illuminate/Database/Eloquent/JsonEncodingException.php:33)
[stacktrace]
#0 PATH/vendor/laravel/framework/src/Illuminate/Database/Eloquent/Concerns/HasAttributes.php(655): Illuminate\\Database\\Eloquent\\JsonEncodingException::forAttribute(Object(App\\Models\\Audit), 'new_values', 'Malformed UTF-8...')
#1 PATH/vendor/laravel/framework/src/Illuminate/Database/Eloquent/Concerns/HasAttributes.php(559): Illuminate\\Database\\Eloquent\\Model->castAttributeAsJson('new_values', false)
#2 PATH/vendor/laravel/framework/src/Illuminate/Database/Eloquent/Model.php(231): Illuminate\\Database\\Eloquent\\Model->setAttribute('new_values', Array)
#3 PATH/vendor/laravel/framework/src/Illuminate/Database/Eloquent/Model.php(153): Illuminate\\Database\\Eloquent\\Model->fill(Array)
#4 PATH/vendor/laravel/framework/src/Illuminate/Database/Eloquent/Model.php(294): Illuminate\\Database\\Eloquent\\Model->__construct(Array)
#5 PATH/vendor/laravel/framework/src/Illuminate/Database/Eloquent/Builder.php(1068): Illuminate\\Database\\Eloquent\\Model->newInstance(Array)
#6 PATH/vendor/laravel/framework/src/Illuminate/Database/Eloquent/Builder.php(756): Illuminate\\Database\\Eloquent\\Builder->newModelInstance(Array)
#7 PATH/vendor/laravel/framework/src/Illuminate/Database/Eloquent/Model.php(1504): Illuminate\\Database\\Eloquent\\Builder->create(Array)
#8 PATH/vendor/laravel/framework/src/Illuminate/Database/Eloquent/Model.php(1516): Illuminate\\Database\\Eloquent\\Model->__call('create', Array)
#9 [internal function]: Illuminate\\Database\\Eloquent\\Model::__callStatic('create', Array)
#10 PATH/vendor/owen-it/laravel-auditing/src/Drivers/Database.php(31): call_user_func(Array, Array)
#11 PATH/vendor/owen-it/laravel-auditing/src/Auditor.php(81): OwenIt\\Auditing\\Drivers\\Database->audit(Object(App\\Models\\MyModel))
#12 PATH/vendor/laravel/framework/src/Illuminate/Support/Facades/Facade.php(221): OwenIt\\Auditing\\Auditor->execute(Object(App\\Models\\MyModel))
#13 PATH/vendor/owen-it/laravel-auditing/src/AuditableObserver.php(50): Illuminate\\Support\\Facades\\Facade::__callStatic('execute', Array)
#14 [internal function]: OwenIt\\Auditing\\AuditableObserver->created(Object(App\\Models\\MyModel))

Possible Solutions

The Auditable currently reads attributes directly from $this->attributes, bypassing any accessor (code).
This is desirable in many cases, where the accessor just beautifies the attribute.

But in cases where the stored data is binary, we need to do some adjustments. This can be

• to use the accessor, assuming that it provides a valid utf-8 string or
• to just base64 encode the data.

How about adding a new property $auditBinary = ['foo'] to MyModel that instructs to base64 encode the foo attribute for auditing? The Audit::getFormattedValue() function should then decode the data again.

Workaround (if you don't want to fix it)

Use transformAudit to sanitize old_values and new_values. But this cannot be easily integrated in Audit::getModified().

[quetzyg]

What do you exactly mean when you say that using the transformAudit() method doesn't integrate easily with the getModified()?

[quetzyg]

It would be great some feedback on this.

[SebastianS90]

Sorry for the delay.

transformAudit is a method in the Auditable model. There you can sanitize any binary data to an utf-8 string which can be json-encoded. This class is also the right place to do so, because which data is in binary form depends on the model itself.

However, the getModified method is part of the Audit model. It expects the data to be stored in such a way that the Auditable model's getXyzAttribute accessors cleanly apply. If we store the binary data in some other form this is no longer the case. The implementation of getModified needs to know which data has been encoded and reverse that encoding. Therefore I'm proposing an additional attribute on the Auditable model.

Even if we argue that the user of getModified might be able to work around, another really useful feature is auditable transition. This feature makes use of getModified.

[quetzyg]

I'll see what can be done.

[fernandocoronatomf]

I have the same issue and what I am doing is converting the binary to string on the transformAudit()

Does that make sense??

[SebastianS90]

Yes, but then you cannot use Auditable Transition for that model any more.

[fernandocoronatomf]

Thank you @SebastianS90 for the quick answer.
I will stick to that for now until we come up with something better.
Thank you

[quetzyg]

Hey @SebastianS90,

Please try branch feat/attribute-encoder to see if it solves your binary data issue.

You basically have to define in your model, the attribute you wish to be encoded, like so:

protected $attributeModifiers = [
    'binary_attribute_name' => Base64Encoder::class,
];

Then, when you get the data back via $audit->getModified(), you should get it decoded.

See the Pull Request here: #437

[quetzyg]

Feedback would be appreciated, @SebastianS90.
Thanks!

[SebastianS90]

I'll try it out on the weekend, too busy right now.

[SebastianS90]

@quetzyg thank you so much, #437 fixes the issue for me :smile:

[quetzyg]

Brilliant! Will close this issue, then.