auditor icon indicating copy to clipboard operation
auditor copied to clipboard

Auditor fails to compute diff when a column of type `blob` is modified.

Open SanderVerkuil opened this issue 3 years ago • 4 comments

Q A
auditor version 2.0.2
PHP version 7.4.28
Database MySQL

Summary

Computing the diff breaks when using a resource column.

Current behavior

When the line:

[
  'diffs' => json_encode($data['diff'], JSON_THROW_ON_ERROR),
]

is called when the diff contains a column of type resource, a JSON exception is thrown stating that the type is not supported.

JsonException Type is not supported

How to reproduce

See my next PR which will introduce a failing test.

Expected behavior

I had hoped that this would, at least, not be failing.

SanderVerkuil avatar Apr 19 '22 18:04 SanderVerkuil

Thanks @SanderVerkuil for the report and the test. I'll try to make fix shortly.

DamienHarper avatar Apr 19 '22 19:04 DamienHarper

Well your PR already fixes the issue, thanks 👍🏽 When reading this issue I thought it only contains a failing test 😅 Anyway, it's merged now.

DamienHarper avatar Apr 19 '22 20:04 DamienHarper

I also replied to your question about blob field handling in the PR conversation

DamienHarper avatar Apr 19 '22 20:04 DamienHarper

I thought fixing it would be a bit more difficult, but the fix was quite simple as it was the same as the binary column type.

The question is more generic though, like, currently when the value is a resource, the diff will show resource#<resource-id>, which is fine if the data is indeed a binary file (or a blob) containing multiple megabytes of data. However, in some cases, the actual value is a couple of bytes, for instance when the blob type is used because the column contains unstructured data, like only 1 or 0, or a float/numeric value.

A possibility for this would be to do something like:

$stream = $value;
stream_rewind($stream);
$possibleValue= stream_get_contents($stream);
if (strlen($possibleValue) > (32 * 1024)) {
  $convertedValue = get_resource_type($value).'#'.get_resource_id($value);
} else {
  $convertedValue = $possibleValue;
}

though this is very inefficiënt because if the binary or blob data was in fact 2GiB of data, the whole file would be read into memory.

So, due to how Doctrine works, and how I mostly use these values, when I call setData('test data'), the new field will be 'test data', because it is not a string. So in that case the auditor will specify that the new data will be 'test data', and the old field will be <resource_type>#<resource_id>. Or am I seeing this differently?

SanderVerkuil avatar Apr 19 '22 23:04 SanderVerkuil