Fix parsing unknown fields break record deserialisation #666
Conversation
- cleaned up decryptRecord()
| ) { | ||
| override fun equals(other: Any?): Boolean { | ||
| if (this === other) return true | ||
| if (javaClass != other?.javaClass) return false | ||
|
|
||
| other as KeeperFile | ||
|
|
||
| if (!fileKey.contentEquals(other.fileKey)) return false | ||
| if (fileUid != other.fileUid) return false | ||
| if (data != other.data) return false | ||
| if (url != other.url) return false | ||
| if (thumbnailUrl != other.thumbnailUrl) return false | ||
|
|
||
| return true | ||
| } | ||
|
|
||
| override fun hashCode(): Int { | ||
| var result = fileKey.contentHashCode() | ||
| result = 31 * result + fileUid.hashCode() | ||
| result = 31 * result + data.hashCode() | ||
| result = 31 * result + url.hashCode() | ||
| result = 31 * result + (thumbnailUrl?.hashCode() ?: 0) | ||
| return result | ||
| } | ||
| } |
There was a problem hiding this comment.
We have a bytearray in this data class which means we need to implement our own equals and hashcode
There was a problem hiding this comment.
No, we don't do equality checks on this class
There was a problem hiding this comment.
That hashcode is more important here - if this ever goes in a hashed data structure without it it won't work
| if (decryptedRecord != null) { | ||
| records.add(decryptedRecord) | ||
| } | ||
| decryptRecord(it, recordKey).onSuccess { keeperRecord -> records.add(keeperRecord) } |
There was a problem hiding this comment.
this is the magic part where if there's a failure it skips gracefully (i.e. onSuccess only gets called if we ge a successful Result)
| val decryptedRecordFiles = encryptedRecord.files?.map { recordFile -> | ||
| val fileKey = decrypt(recordFile.fileKey, recordKey) | ||
| val decryptedFile = decrypt(recordFile.data, fileKey) | ||
| KeeperFile( | ||
| fileKey, | ||
| recordFile.fileUid, | ||
| Json.decodeFromString(bytesToString(decryptedFile)), | ||
| recordFile.url, | ||
| recordFile.thumbnailUrl | ||
| ) | ||
| } ?: emptyList() |
There was a problem hiding this comment.
functional > imperative (plus why allocate when you don't need to?)
| ) | ||
| } ?: emptyList() | ||
|
|
||
| val recordData: KeeperRecordData = try { |
There was a problem hiding this comment.
Restructuring this ensures that there are only two non exception pathways out - either you get the record data you want, or you don't, and both are ok.
idimov-keeper
changed the base branch from
master
to
release/sdk/java/core/v16.6.6
This PR fixes an issue where deserialising a record with unknown fields breaks deserialisation of all records. With this change, an error message is displayed and the record is skipped.