Improve logging on some backup validation errors.

This commit is contained in:
Greyson Parrelli
2025-02-14 11:40:28 -05:00
parent c1ce4ba80d
commit 16309d87cd
6 changed files with 107 additions and 18 deletions

View File

@@ -10,6 +10,8 @@ import org.signal.libsignal.messagebackup.MessageBackup
import org.signal.libsignal.messagebackup.ValidationError
import org.thoughtcrime.securesms.database.SignalDatabase
import org.thoughtcrime.securesms.keyvalue.SignalStore
import org.thoughtcrime.securesms.recipients.Recipient
import org.thoughtcrime.securesms.recipients.RecipientId
import org.thoughtcrime.securesms.util.isStory
import org.thoughtcrime.securesms.util.isStoryReaction
import org.whispersystems.signalservice.api.backup.MessageBackupKey
@@ -40,11 +42,36 @@ object ArchiveValidator {
} catch (e: IOException) {
ValidationResult.ReadError(e)
} catch (e: ValidationError) {
val sentTimestamp = "\\d{10,}+".toRegex().find(e.message ?: "")?.value?.toLongOrNull()
ValidationResult.ValidationError(
exception = e,
messageDetails = sentTimestamp?.let { fetchMessageDetails(it) } ?: emptyList()
)
if (e.message?.contains("have the same phone number") == true) {
val recipientIds = """RecipientId\((\d+)\)""".toRegex()
.findAll(e.message ?: "")
.map { it.groupValues[1] }
.mapNotNull { it.toLongOrNull() }
.map { RecipientId.from(it) }
.toList()
val recipientIdA = recipientIds.getOrNull(0)
val recipientIdB = recipientIds.getOrNull(1)
val e164A = recipientIdA?.let { Recipient.resolved(it).e164.orElse("UNKNOWN") }.let { "KEEP_E164::$it" }
val e164B = recipientIdB?.let { Recipient.resolved(it).e164.orElse("UNKNOWN") }.let { "KEEP_E164::$it" }
ValidationResult.RecipientDuplicateE164Error(
exception = e,
details = DuplicateRecipientDetails(
recipientIdA = recipientIds.getOrNull(0),
recipientIdB = recipientIds.getOrNull(1),
e164A = e164A,
e164B = e164B
)
)
} else {
val sentTimestamp = "\\d{10,}+".toRegex().find(e.message ?: "")?.value?.toLongOrNull()
ValidationResult.MessageValidationError(
exception = e,
messageDetails = sentTimestamp?.let { fetchMessageDetails(it) } ?: emptyList()
)
}
}
}
@@ -74,10 +101,14 @@ object ArchiveValidator {
sealed interface ValidationResult {
data object Success : ValidationResult
data class ReadError(val exception: IOException) : ValidationResult
data class ValidationError(
val exception: org.signal.libsignal.messagebackup.ValidationError,
data class MessageValidationError(
val exception: ValidationError,
val messageDetails: List<MessageDetails>
) : ValidationResult
data class RecipientDuplicateE164Error(
val exception: ValidationError,
val details: DuplicateRecipientDetails
) : ValidationResult
}
data class MessageDetails(
@@ -97,4 +128,11 @@ object ArchiveValidator {
val originalMessageId: Long,
val isLatestRevision: Boolean
)
data class DuplicateRecipientDetails(
val recipientIdA: RecipientId?,
val recipientIdB: RecipientId?,
val e164A: String?,
val e164B: String?
)
}

View File

@@ -151,10 +151,14 @@ class InternalBackupPlaygroundViewModel : ViewModel() {
val message = when (result) {
is ArchiveValidator.ValidationResult.ReadError -> "Failed to read backup file!"
ArchiveValidator.ValidationResult.Success -> "Validation passed!"
is ArchiveValidator.ValidationResult.ValidationError -> {
is ArchiveValidator.ValidationResult.MessageValidationError -> {
Log.w(TAG, "Validation failed! Details: ${result.messageDetails}", result.exception)
"Validation failed :( Check the logs for details."
}
is ArchiveValidator.ValidationResult.RecipientDuplicateE164Error -> {
Log.w(TAG, "Validation failed with a duplicate recipient! Details: ${result.details}", result.exception)
"Validation failed :( Check the logs for details."
}
}
_state.value = _state.value.copy(statusMessage = message)
}

View File

@@ -226,11 +226,17 @@ class BackupMessagesJob private constructor(
return BackupFileResult.Retry
}
is ArchiveValidator.ValidationResult.ValidationError -> {
is ArchiveValidator.ValidationResult.MessageValidationError -> {
Log.w(TAG, "The backup file fails validation! Message: ${result.exception.message}, Details: ${result.messageDetails}")
ArchiveUploadProgress.onValidationFailure()
return BackupFileResult.Failure
}
is ArchiveValidator.ValidationResult.RecipientDuplicateE164Error -> {
Log.w(TAG, "The backup file fails validation with a duplicate recipient! Message: ${result.exception.message}, Details: ${result.details}")
ArchiveUploadProgress.onValidationFailure()
return BackupFileResult.Failure
}
}
stopwatch.split("validate")

View File

@@ -277,10 +277,14 @@ object LinkDeviceRepository {
Log.w(TAG, "[createAndUploadArchive] Failed to read the file during validation!", result.exception)
return LinkUploadArchiveResult.BackupCreationFailure(result.exception)
}
is ArchiveValidator.ValidationResult.ValidationError -> {
is ArchiveValidator.ValidationResult.MessageValidationError -> {
Log.w(TAG, "[createAndUploadArchive] The backup file fails validation! Details: ${result.messageDetails}", result.exception)
return LinkUploadArchiveResult.BackupCreationFailure(result.exception)
}
is ArchiveValidator.ValidationResult.RecipientDuplicateE164Error -> {
Log.w(TAG, "[createAndUploadArchive] The backup file fails validation with a duplicate recipient! Details: ${result.details}", result.exception)
return LinkUploadArchiveResult.BackupCreationFailure(result.exception)
}
}
stopwatch.split("validate-backup")

View File

@@ -22,8 +22,8 @@ object Scrubber {
* Supposedly, the shortest international phone numbers in use contain seven digits.
* Handles URL encoded +, %2B
*/
private val E164_PATTERN = Pattern.compile("(\\+|%2B)(\\d{7,15})")
private val E164_ZERO_PATTERN = Pattern.compile("\\b0(\\d{10})\\b")
private val E164_PATTERN = Pattern.compile("(KEEP_E164::)?(\\+|%2B)(\\d{7,15})")
private val E164_ZERO_PATTERN = Pattern.compile("\\b(KEEP_E164::)?0(\\d{10})\\b")
/** The second group will be censored.*/
private val CRUDE_EMAIL_PATTERN = Pattern.compile("\\b([^\\s/])([^\\s/]*@[^\\s]+)")
@@ -104,17 +104,29 @@ object Scrubber {
private fun CharSequence.scrubE164(): CharSequence {
return scrub(this, E164_PATTERN) { matcher, output ->
output
.append("E164:")
.append(hash(matcher.group(2)))
if (matcher.group(1) != null && matcher.group(1)!!.isNotEmpty()) {
output
.append("KEEP_E164::")
.append((matcher.group(2) + matcher.group(3)).censorMiddle(2, 2))
} else {
output
.append("E164:")
.append(hash(matcher.group(3)))
}
}
}
private fun CharSequence.scrubE164Zero(): CharSequence {
return scrub(this, E164_ZERO_PATTERN) { matcher, output ->
output
.append("E164:")
.append(hash(matcher.group(1)))
if (matcher.group(1) != null && matcher.group(1)!!.isNotEmpty()) {
output
.append("KEEP_E164::")
.append(("0" + matcher.group(2)).censorMiddle(2, 2))
} else {
output
.append("E164:")
.append(hash(matcher.group(2)))
}
}
}
@@ -204,6 +216,15 @@ object Scrubber {
}
}
private fun String.censorMiddle(leading: Int, trailing: Int): String {
val totalKept = leading + trailing
if (this.length < totalKept) {
return "*".repeat(this.length)
}
val middle = "*".repeat(this.length - totalKept)
return this.take(leading) + middle + this.takeLast(trailing)
}
private fun scrub(input: CharSequence, pattern: Pattern, processMatch: MatchProcessor): CharSequence {
val output = StringBuilder(input.length)
val matcher: Matcher = pattern.matcher(input)

View File

@@ -74,6 +74,22 @@ class ScrubberTest(private val input: String, private val expected: String) {
"Longest number +155556789012345",
"Longest number E164:<90596>"
),
arrayOf(
"An E164 number KEEP_E164::+15551234567",
"An E164 number KEEP_E164::+1********67"
),
arrayOf(
"A UK number KEEP_E164::+447700900000",
"A UK number KEEP_E164::+4*********00"
),
arrayOf(
"A Japanese number KEEP_E164::08011112222",
"A Japanese number KEEP_E164::08*******22"
),
arrayOf(
"A Japanese number (KEEP_E164::08011112222)",
"A Japanese number (KEEP_E164::08*******22)"
),
arrayOf(
"One more than longest number +1234567890123456",
"One more than longest number E164:<78d5b>6"