Fixed crash when auditing on binary data #202
Conversation
| @@ -19,15 +17,21 @@ def get_field_value(obj, field): | |||
| :return: The value of the field as a string. | |||
| :rtype: str | |||
| """ | |||
| raw_value = getattr(obj, field.name, None) | |||
| if value is not None and settings.USE_TZ and not timezone.is_naive(value): | ||
| value = timezone.make_naive(value, timezone=timezone.utc) | ||
| except ObjectDoesNotExist: | ||
| value = field.default if field.default is not NOT_PROVIDED else None | ||
| elif isinstance(raw_value, bytes): | ||
| if len(raw_value) > 100: |
There was a problem hiding this comment.
I'm not saying this is a magic number, but it would be helpful to have a comment regarding this (and possibly make it a variable since it is referenced multiple times in this function scope).
There was a problem hiding this comment.
I'm actually quite confused why this value is being truncated, and why at 100 bytes. What is this doing? This PR doesn't link an issue explaining the problem.
There was a problem hiding this comment.
It's being truncated because the audit log can't by default store a full copy of binary files that can be gigabytes in size.
There was a problem hiding this comment.
Would it be worth (possible even?) storing a hash of the binary data instead of the first 100 bytes?
There was a problem hiding this comment.
Hmm.. that's an idea for sure. The advantage is that you can actually check if some data is the exact data. The downsides are that you don't get something immediately useful in the log, and that if you don't have the binary data you want to compare to anymore then the hash is useless.
There was a problem hiding this comment.
I would worry about the speed of hashing very large binary data. Say for a 1GB file, I'd expect at least 5 seconds for a hash to be generated if the implementation was purely C.
| if value is not None and settings.USE_TZ and not timezone.is_naive(value): | ||
| value = timezone.make_naive(value, timezone=timezone.utc) | ||
| except ObjectDoesNotExist: | ||
| value = field.default if field.default is not NOT_PROVIDED else None | ||
| elif isinstance(raw_value, bytes): | ||
| if len(raw_value) > 100: | ||
| return repr(raw_value[:100]) + '[truncated {} bytes]'.format(len(raw_value) - 100) |
There was a problem hiding this comment.
| return repr(raw_value[:100]) + '[truncated {} bytes]'.format(len(raw_value) - 100) | |
| return repr(f"{raw_value[:100]}...[truncated {len(raw_value) - 100} bytes]") |
No description provided.