github-starred/android
2
0
Fork 0
mirror of https://github.com/bitwarden/android.git synced 2026-03-25 07:41:55 -05:00
Code Issues 144 Packages Projects Releases 118 Wiki Activity

Android app fails to sync due to malformed data - all other platforms work #1059

New Issue
Closed
opened 2025-11-26 22:38:03 -06:00 by GiteaMirror · 13 comments
GiteaMirror commented 2025-11-26 22:38:03 -06:00
Owner
Copy Link

Originally created by @Lockszmith-GH on GitHub (Jul 9, 2020).

Originally assigned to: @cscharf on GitHub.

Describe the Bug

Web vault, command line, Firefox addon, Chrome extension and Windows application all function properly.
Android app refuses to sync.
Login is granted, but sync never succeeds.

If synced in the past, all data remains accessible, edits are synced back to server.
However new items and edits from other sources are not synced.

Steps To Reproduce

Not sure about reproduction, as I'm not 100% sure how the data crept in.
However, using the debugger, I found 3 items that broke the CipherData constructor.

In my case specifically: response.Login was null and the LoginData constructor threw the exception.

I replace SyncCiphersAsync with the following code in order to identify the culprits:

        private async Task SyncCiphersAsync(string userId, List<CipherResponse> response)
        {
            System.Diagnostics.Trace.WriteLine("SyncCiphersAsync");
            var badBunch = response.Where(c => null == c.Login && null == c.Card && null == c.Identity && null == c.SecureNote).ToDictionary(
                    c => c.Id,
                    c => {
                        try
                        {
                            var test = new CipherData(c, userId);
                        }
                        catch (Exception e)
                        {
                            System.Diagnostics.Trace.WriteLine(
                                    new StringBuilder()
                                    .AppendLine("ERROR SyncCiphersAsync: new CipherData threw an exception!")
                                    .Append("Id: ").AppendLine(c.Id)
                                    .Append("Name: ").AppendLine(c.Name)
                                    .AppendLine().Append("Exception: ").AppendLine(e.Message)
                                    .AppendLine("StackTrace:").AppendLine(e.StackTrace)
                                    .ToString()
                                );
                            try
                            {
                            }
                            catch { }
                        }
                        return c;
                    }
                );
            var ciphers = response.Where(c => null != c.Login || null != c.Card || null != c.Identity || null != c.SecureNote ).ToDictionary(
                    c => c.Id,
                    c => new CipherData(c, userId)
                );

            await _cipherService.ReplaceAsync(ciphers);
        }

this helped me pin point the 'broken' item ids, then using the bw get item <id> cli tool I identified the items, and after reviewing removed them.

Expected Result

Sync should never fail, especially if it doesn't fail on any other platform.
At the very least, if an item breaks sync, an item id should be logged somewhere (server or client side).

A possible (and very simple) solution would be to just engulf the switch in the CipherData constructor with try...catch, as the 'default' case is OK with assigning nothing.

Actual Result

Sync just isn't completing, no error, no info.

Environment

  • Device: Android, any - tested on VM, OnePlus 3, OnePlus 6, Samsung S8+
  • Operating system: docker on linux (installed via docker scripts a long time ago)
  • Build Version: 2.5.0 (3093), although it's been this way for a while.
  • Is this a Beta release? N

Additional Context

There are quite a few Sync related issues here, some are related to bitwarden_rs, this is not the case for me.

As always, you probably have the best password vault solution in the world right now, mainly because of the open-source nature of it.
Thank for maintaining it, and making it a reality.

Originally created by @Lockszmith-GH on GitHub (Jul 9, 2020). Originally assigned to: @cscharf on GitHub. ## Describe the Bug Web vault, command line, Firefox addon, Chrome extension and Windows application all function properly. Android app refuses to sync. Login is granted, but sync never succeeds. If synced in the past, all data remains accessible, edits are synced back to server. However new items and edits from other sources are not synced. ## Steps To Reproduce Not sure about reproduction, as I'm not 100% sure how the data crept in. However, using the debugger, I found 3 items that broke the [CipherData](https://github.com/bitwarden/mobile/blob/c5f158f1cfa60e33bf8856b3902596cf6f3554a6/src/Core/Models/Data/CipherData.cs#L12) constructor. In my case specifically: `response.Login` was null and the [LoginData constructor](https://github.com/bitwarden/mobile/blob/c5f158f1cfa60e33bf8856b3902596cf6f3554a6/src/Core/Models/Data/CipherData.cs#L31) threw the exception. I replace [SyncCiphersAsync](https://github.com/bitwarden/mobile/blob/3c18fd7636333a1dfac8d13ce4da6408b3a91783/src/Core/Services/SyncService.cs#L340) with the following code in order to identify the culprits: ``` cs private async Task SyncCiphersAsync(string userId, List<CipherResponse> response) { System.Diagnostics.Trace.WriteLine("SyncCiphersAsync"); var badBunch = response.Where(c => null == c.Login && null == c.Card && null == c.Identity && null == c.SecureNote).ToDictionary( c => c.Id, c => { try { var test = new CipherData(c, userId); } catch (Exception e) { System.Diagnostics.Trace.WriteLine( new StringBuilder() .AppendLine("ERROR SyncCiphersAsync: new CipherData threw an exception!") .Append("Id: ").AppendLine(c.Id) .Append("Name: ").AppendLine(c.Name) .AppendLine().Append("Exception: ").AppendLine(e.Message) .AppendLine("StackTrace:").AppendLine(e.StackTrace) .ToString() ); try { } catch { } } return c; } ); var ciphers = response.Where(c => null != c.Login || null != c.Card || null != c.Identity || null != c.SecureNote ).ToDictionary( c => c.Id, c => new CipherData(c, userId) ); await _cipherService.ReplaceAsync(ciphers); } ``` this helped me pin point the 'broken' item ids, then using the `bw get item <id>` cli tool I identified the items, and after reviewing removed them. ## Expected Result Sync should never fail, especially if it doesn't fail on any other platform. At the very least, if an item breaks sync, an **item id** should be logged somewhere (server or client side). A possible (and very simple) solution would be to just engulf the [switch](https://github.com/bitwarden/mobile/blob/c5f158f1cfa60e33bf8856b3902596cf6f3554a6/src/Core/Models/Data/CipherData.cs#L28) in the CipherData constructor with `try...catch`, as the '[default](https://github.com/bitwarden/mobile/blob/c5f158f1cfa60e33bf8856b3902596cf6f3554a6/src/Core/Models/Data/CipherData.cs#L42)' case is OK with assigning nothing. ## Actual Result Sync just isn't completing, no error, no info. ## Environment - Device: Android, any - tested on VM, OnePlus 3, OnePlus 6, Samsung S8+ - Operating system: docker on linux (installed via docker scripts a long time ago) - Build Version: 2.5.0 (3093), although it's been this way for a while. - Is this a Beta release? N ## Additional Context There are quite a few Sync related issues here, some are related to bitwarden_rs, this is not the case for me. As always, you probably have the best password vault solution in the world right now, mainly because of the open-source nature of it. Thank for maintaining it, and making it a reality.
GiteaMirror commented 2025-11-26 22:38:03 -06:00
Author
Owner
Copy Link

@Lockszmith-GH commented on GitHub (Jul 9, 2020):

Until the client is patched (if that's what devs decide), here is a quick way to get rid of troublesome entries.

Using the bw cli on Windows and using PowerShell the code below will help identify potential problem items, and help remove them.
'Bad' items are deemed any items you have that does not have values at all for it's username and passwords.

Before removal, the code below will preserve an export of all of your data first, and a list of the removed items before it actually removes them.

Use with extreme caution

  • Final NOTE: in order to actually allow the removal, you will need to uncomment the deletion command (remove the # from the beginning of the line)
# saving bw vault, and removing invalid entries

$pw = (Get-Credential -Message "bw password" -UserName 'nevermind').GetNetworkCredential().password

bw export "$pw" --format json --output .\bw_1.json ## The only way to export Unicode chars properly
(bw list organizations | convertfrom-json) | % { Write-Host "`nExporting org: $($_.id) $($_.name)..."; bw export "$pw" --format json --organizationid $_.id --output ./bw_$($_.id) }

$vault = bw list items | convertfrom-json
$bad = $vault | Where-Object { $_.type -eq 1 -and $_.login.password -eq $null -and $_.login.passwordRevisionDate -eq $null -and $_.login.totp -eq $null -and $_.login.username -eq $null -and $_.login.uris -eq $null } | Select-Object -Property organizationId,Id | Group-Object -Property organizationid
$bad | Export-Clixml -Path ./bad_BW.clixml

# delete bad entries, the line is commented to prevent accidental deletion.
# $bad.group.id | % { bw delete item "$_" --permanent}

bw sync

Remove-Variable pw # this will 'forget' the password you entered.
@Lockszmith-GH commented on GitHub (Jul 9, 2020): Until the client is patched (if that's what devs decide), here is a quick way to get rid of troublesome entries. Using the `bw cli` on Windows and using PowerShell the code below will help identify potential problem items, and help remove them. 'Bad' items are deemed any items you have that does not have values at all for it's username and passwords. Before removal, the code below will preserve an export of all of your data first, and a list of the removed items before it actually removes them. **Use with extreme caution** * Final NOTE: in order to actually allow the removal, you will need to uncomment the deletion command (remove the # from the beginning of the line) ``` powershell # saving bw vault, and removing invalid entries $pw = (Get-Credential -Message "bw password" -UserName 'nevermind').GetNetworkCredential().password bw export "$pw" --format json --output .\bw_1.json ## The only way to export Unicode chars properly (bw list organizations | convertfrom-json) | % { Write-Host "`nExporting org: $($_.id) $($_.name)..."; bw export "$pw" --format json --organizationid $_.id --output ./bw_$($_.id) } $vault = bw list items | convertfrom-json $bad = $vault | Where-Object { $_.type -eq 1 -and $_.login.password -eq $null -and $_.login.passwordRevisionDate -eq $null -and $_.login.totp -eq $null -and $_.login.username -eq $null -and $_.login.uris -eq $null } | Select-Object -Property organizationId,Id | Group-Object -Property organizationid $bad | Export-Clixml -Path ./bad_BW.clixml # delete bad entries, the line is commented to prevent accidental deletion. # $bad.group.id | % { bw delete item "$_" --permanent} bw sync Remove-Variable pw # this will 'forget' the password you entered. ```
GiteaMirror commented 2025-11-26 22:38:04 -06:00
Author
Owner
Copy Link

@addisonbeck commented on GitHub (Jul 10, 2020):

Thanks for all the information here!

In regards to this:

'Bad' items are deemed any items you have that does not have values at all for it's username and passwords.

Is that what the bad items you removed looked like? I'm not able to recreate this scenario just by having a login with a blank username & password. Was there anything else irregular about these items? Was their 'Name' field also empty?

@addisonbeck commented on GitHub (Jul 10, 2020): Thanks for all the information here! In regards to this: > 'Bad' items are deemed any items you have that does not have values at all for it's username and passwords. Is that what the bad items you removed looked like? I'm not able to recreate this scenario just by having a login with a blank username & password. Was there anything else irregular about these items? Was their 'Name' field also empty?
GiteaMirror commented 2025-11-26 22:38:04 -06:00
Author
Owner
Copy Link

@Lockszmith-GH commented on GitHub (Jul 10, 2020):

The 'bad items' from the cli was a larger set then those I found in the Android debugger.
It's hard to pin point what exactly got malformed, but for some client gets a null for the login object (in the Android app it is typed as LoginData class) which contained Username, Password, Totp, PasswordRevisionDate and list of Uris.
The PowerShell code just identifies were all of these values are blank.

In practice, it is possible for all to be empty, and the entry still be valid for sync with Android, but in my particular case, instead of an empty object, the value returned was invalid and the object was completely missing.

I have cleared the culprits from my Db, so can't validate anymore right now.
But the solution I proposed (putting a try-catch around the switch block) will at least cause Android client to function exactly like the rest of the platform clients.

@Lockszmith-GH commented on GitHub (Jul 10, 2020): The 'bad items' from the cli was a larger set then those I found in the Android debugger. It's hard to pin point what exactly got malformed, but for some client gets a null for the login object (in the Android app it is typed as LoginData class) which contained Username, Password, Totp, PasswordRevisionDate and list of Uris. The PowerShell code just identifies were all of these values are blank. In practice, it is possible for all to be empty, and the entry still be valid for sync with Android, but in my particular case, instead of an empty object, the value returned was invalid and the object was completely missing. I have cleared the culprits from my Db, so can't validate anymore right now. But the solution I proposed (putting a try-catch around the switch block) will at least cause Android client to function exactly like the rest of the platform clients.
GiteaMirror commented 2025-11-26 22:38:04 -06:00
Author
Owner
Copy Link

@Lockszmith-GH commented on GitHub (Jul 10, 2020):

I can create a pull request, I just want to get developer's general approval on the concept of the solution.

@Lockszmith-GH commented on GitHub (Jul 10, 2020): I can create a pull request, I just want to get developer's general approval on the concept of the solution.
GiteaMirror commented 2025-11-26 22:38:04 -06:00
Author
Owner
Copy Link

@addisonbeck commented on GitHub (Jul 10, 2020):

Sure, go ahead! I was hoping to figure out how this happened in the first place & maybe reproduce so we can try and stop this kind of malformed data from making it far enough to need to get catched on sync, but it looks like that might not be feasible, and your solution is worth implementing regardless for any similar issues.

@addisonbeck commented on GitHub (Jul 10, 2020): Sure, go ahead! I was hoping to figure out how this happened in the first place & maybe reproduce so we can try and stop this kind of malformed data from making it far enough to need to get catched on sync, but it looks like that might not be feasible, and your solution is worth implementing regardless for any similar issues.
GiteaMirror commented 2025-11-26 22:38:04 -06:00
Author
Owner
Copy Link

@kspearrin commented on GitHub (Jul 10, 2020):

@Lockszmith Are you using bitwarden_rs?

@kspearrin commented on GitHub (Jul 10, 2020): @Lockszmith Are you using bitwarden_rs?
GiteaMirror commented 2025-11-26 22:38:05 -06:00
Author
Owner
Copy Link

@Lockszmith-GH commented on GitHub (Jul 11, 2020):

no, although I did see this happen with bitwarden_rs (which a friend is running) and on my own bitwarden-vanilla self-hosted server.
In both vaults the sync broke at the same location.
The solution above fixed it for both.

I'll prep the PR.

@Lockszmith-GH commented on GitHub (Jul 11, 2020): no, although I did see this happen with bitwarden_rs (which a friend is running) and on my own bitwarden-vanilla self-hosted server. In both vaults the sync broke at the same location. The solution above fixed it for both. I'll prep the PR.
GiteaMirror commented 2025-11-26 22:38:05 -06:00
Author
Owner
Copy Link

@cscharf commented on GitHub (Jul 13, 2020):

Merged PR adds corrective behavior to the crash, keeping this open to try to reproduce the cause of that bad data in the first place and take corrective action at the source.

@cscharf commented on GitHub (Jul 13, 2020): Merged PR adds corrective behavior to the crash, keeping this open to try to reproduce the cause of that bad data in the first place and take corrective action at the source.
GiteaMirror commented 2025-11-26 22:38:06 -06:00
Author
Owner
Copy Link

@cscharf commented on GitHub (Jul 13, 2020):

@Lockszmith , can you specify how those entries were originally added to your vault? Did you use the import tool, the CLI, browser extension, or web vault to add them (or another client)?

@cscharf commented on GitHub (Jul 13, 2020): @Lockszmith , can you specify how those entries were originally added to your vault? Did you use the import tool, the CLI, browser extension, or web vault to add them (or another client)?
GiteaMirror commented 2025-11-26 22:38:06 -06:00
Author
Owner
Copy Link

@cscharf commented on GitHub (Jul 23, 2020):

Closing issue for now, would love to get more information however on at least the source of the bad data, whether or not it was imported, from where, etc.

@cscharf commented on GitHub (Jul 23, 2020): Closing issue for now, would love to get more information however on at least the _source_ of the bad data, whether or not it was imported, from where, etc.
GiteaMirror commented 2025-11-26 22:38:06 -06:00
Author
Owner
Copy Link

@Lockszmith-GH commented on GitHub (Jul 23, 2020):

@cscharf

@Lockszmith , can you specify how those entries were originally added to your vault? Did you use the import tool, the CLI, browser extension, or web vault to add them (or another client)?

Sorry somehow missed your questions earlier - I believe it was via import, but unsure, because I noticed the sync failing weeks after the fact

@Lockszmith-GH commented on GitHub (Jul 23, 2020): @cscharf > @Lockszmith , can you specify how those entries were originally added to your vault? Did you use the import tool, the CLI, browser extension, or web vault to add them (or another client)? Sorry somehow missed your questions earlier - I believe it was via import, but unsure, because I noticed the sync failing weeks after the fact
GiteaMirror commented 2025-11-26 22:38:06 -06:00
Author
Owner
Copy Link

@cscharf commented on GitHub (Jul 23, 2020):

No worries and thanks for the additional info... Do you remember what source you imported from (another pw manager, csv, etc.)?

@cscharf commented on GitHub (Jul 23, 2020): No worries and thanks for the additional info... Do you remember what source you imported from (another pw manager, csv, etc.)?
GiteaMirror commented 2025-11-26 22:38:06 -06:00
Author
Owner
Copy Link

@Lockszmith-GH commented on GitHub (Jul 23, 2020):

My first import (a couple of years ago) was LastPass - don't think I had issues there, since then only exports from web vault.

@Lockszmith-GH commented on GitHub (Jul 23, 2020): My first import (a couple of years ago) was LastPass - don't think I had issues there, since then only exports from web vault.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
app:authenticator
app:password-manager
bug
bug-passkey
customer-support
duplicate
enhancement
good first issue
help wanted
needs-reply
needs-response
pull-request
Mirrored from GitHub Pull Request
 question
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
GiteaMirror ninjasurge
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: github-starred/android#1059
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?