search

yiisoft / yii2-debug

Debug Extension for Yii 2
http://www.yiiframework.com
BSD 3-Clause "New" or "Revised" License
202 stars 150 forks source link

On serialization of data #390

Closed Sarke closed 5 years ago

Sarke commented 5 years ago

To follow up on the #377 issue, which has been discussed a few times. Currently the workaround for this serialization uses this implementation:

https://github.com/yiisoft/yii2/blob/e1f6761dfd9eba1ff1260cd37b04936aaa4959b5/framework/helpers/BaseVarDumper.php#L192-L221

This creates an unusual format, and I think it was meant to be evaled perhaps?

$output = 'unserialize(' . var_export(serialize($var), true) . ')';

The drawback of this workaround is that objects get quite ugly:

image

This really nerfs the dump panel, since many times objects are dumped for debugging, but we can't trust them to not have closures somewhere in them, so they end up looking like the above.

Personally, I have worked around this by using the https://github.com/opis/closure to serialize them instead, so I end up with a much nicer looking dump (also using the symfony/vardumper):

image

So, what's my point?

I think we should revisit how the data is serialized for the LogTarget, and I wonder if the 'unserialize(' . var_export(serialize($var), true) . ')'; format is really intended to be unserialized at some point? Would we want to depend on an external library like opis/closure?

An alternative, at least for the DumpPanel, would be to dump the data before it is serialized, so it's already a string and safe, and can then be displayed raw when viewed in the panel.

A second related issue I discovered was how the debug data is saved. Many panels use the same log data (DbPanel, DumpPanel, LogPanel, ProfilingPanel, RouterPanel) yet each panel saves it's own part of that, creating duplicate data and larger debug dumps. Would it not make sense to serialize it once and then have those panels read from and filter the single source of data on read? Instead of filtering and serializing duplicates on write during the request.

Thoughts?

samdark commented 5 years ago

Depending on opis/closure for this case sounds fine to me.