search

Kunzisoft / KeePassDX

Lightweight vault and password manager for Android, KeePassDX allows editing encrypted data in a single file in KeePass format and fill in the forms in a secure way.
https://www.keepassdx.com/
GNU General Public License v3.0
4.32k stars 261 forks source link

[2.9.14_Beta] Save a database with many binaries is long #926

Closed Vstory closed 3 years ago

Vstory commented 3 years ago

Describe the bug After selecting an item in the selection mode (auto-fill), it is always writing data.

To Reproduce

  1. Enter the application
  2. Select KeePassDx Pro to fill in the password
  3. After selecting an item in the selection mode (auto-fill)
  4. save
  5. After that, the application will be in the state of writing to the database.

KeePass Database

KeePassDX (please complete the following information):

Android (please complete the following information):

Additional context

Screenshot_2021-03-14-01-18-47-454_com kunzisoft keepass pro

J-Jamet commented 3 years ago

If the problem is that the saving is too long, it has been corrected in the issue https://github.com/Kunzisoft/KeePassDX/issues/923

Can you install the latest beta 2.9.14 and tell me if the problem is still visible?

Vstory commented 3 years ago

Can you install the latest beta 2.9.14 and tell me if the problem is still visible?

I don't know which version of 2.9.14 you are talking about. My current version is 2.9.14 (60). The problem is in this version.

KeePassDX (please complete the following information):

J-Jamet commented 3 years ago

OK, 2.9.14 code 60 is the latest current version. But I cannot reproduce the problem. The save is done correctly after each selection on my devices. For you approximately how long did a save take in version 2.9.13?

Vstory commented 3 years ago

For you approximately how long did a save take in version 2.9.13?

About 3~5 seconds.

I grabbed a logcat log and reproduced the above problem. The target application of the test is bin.mt.plus.canary .

logcat: all_2021-03-15_22-51-32.zip

J-Jamet commented 3 years ago

Thanks for the logs, it's very useful. It looks like there is an attempt to retrieve a null icon, I will try to fix the problem and upload an APK to you for testing.

03-15 22:50:47.458  2278  2311 I Timeline: Timeline: Activity_windows_visible id: ActivityRecord{a8af45e u0 com.kunzisoft.keepass.pro/com.kunzisoft.keepass.activities.GroupActivity t7899} time:4789729
03-15 22:50:47.469  2278  2303 I am_stop_activity: [0,125901446,com.kunzisoft.keepass.pro/com.kunzisoft.keepass.activities.AutofillLauncherActivity]
03-15 22:50:47.477 14797 14797 I am_on_stop_called: [0,com.kunzisoft.keepass.activities.AutofillLauncherActivity,STOP_ACTIVITY_ITEM,1]
03-15 22:50:47.559  2278  4946 I notification_expansion: [0|com.kunzisoft.keepass.pro|575|null|10184,0,0,7066,3901,0]
03-15 22:50:50.189 14797 10821 W System.err:    at com.kunzisoft.keepass.icons.IconDrawableFactory.getIconDrawable(IconDrawableFactory.kt:92)
03-15 22:50:50.189 14797 10821 W System.err:    at com.kunzisoft.keepass.icons.IconDrawableFactory.addToCustomCache(IconDrawableFactory.kt:211)
03-15 22:50:50.189 14797 10821 W System.err:    at com.kunzisoft.keepass.icons.IconDrawableFactory.access$addToCustomCache(IconDrawableFactory.kt:50)
03-15 22:50:50.189 14797 10821 W System.err:    at com.kunzisoft.keepass.icons.IconDrawableFactory$assignDatabaseIcon$1.invokeSuspend(IconDrawableFactory.kt:160)
03-15 22:50:50.196 14797 10792 W System.err:    at com.kunzisoft.keepass.icons.IconDrawableFactory.getIconDrawable(IconDrawableFactory.kt:92)
03-15 22:50:50.196 14797 10792 W System.err:    at com.kunzisoft.keepass.icons.IconDrawableFactory.addToCustomCache(IconDrawableFactory.kt:211)
03-15 22:50:50.196 14797 10792 W System.err:    at com.kunzisoft.keepass.icons.IconDrawableFactory.access$addToCustomCache(IconDrawableFactory.kt:50)
03-15 22:50:50.196 14797 10792 W System.err:    at com.kunzisoft.keepass.icons.IconDrawableFactory$assignDatabaseIcon$1.invokeSuspend(IconDrawableFactory.kt:160)
03-15 22:50:51.665  2278  2278 I sysui_multi_action: [757,128,758,5,759,8,793,11171,794,0,795,8006,796,575,806,com.kunzisoft.keepass.pro,857,com.kunzisoft.keepass.notif-13fe0499,858,3,947,0]
03-15 22:50:51.668  2278  2278 I notification_canceled: [0|com.kunzisoft.keepass.pro|575|null|10184,8,11171,8006,0,-1,-1,NULL]
03-15 22:50:51.698  3098  3098 D StatusBar: onNotificationRemoved key=0|com.kunzisoft.keepass.pro|575|null|10184 reason=8
03-15 22:50:51.705  3098  3098 D StatusBar: removeNotification StatusBarNotification(pkg=com.kunzisoft.keepass.pro user=UserHandle{0} id=575 tag=null key=0|com.kunzisoft.keepass.pro|575|null|10184: Notification(channel=com.kunzisoft.keepass.notification.channel.database pri=0 contentView=null vibrate=null sound=null defaults=0x0 flags=0x62 color=0xff43a047 actions=1 vis=SECRET))
03-15 22:50:51.705  3098  3098 D StatusBar:     pkgName=com.kunzisoft.keepass.pro appUid=10184 sdk=30 imp=3 sysApp=F priApp=F hasShown=F float=F keyguard=F peek=F fullscreen=F
03-15 22:50:51.706  3098  3098 D StatusBar: update app badge num: com.kunzisoft.keepass.pro/,num=0,isAllowed=false,userId=0
03-15 22:50:51.724 17422 17489 I Launcher.ApplicationsMessage: update com.kunzisoft.keepass.pro/ to null
Vstory commented 3 years ago

It looks like there is an attempt to retrieve a null icon, I will try to fix the problem and upload an APK to you for testing.

Ok. 😘 I suggest the problem feedback template, adding log upload options. This can probably locate the problem quickly.

J-Jamet commented 3 years ago

I suggest the problem feedback template, adding log upload options.

Yes I thought about it, the problem is that I have to code it because it's a feature like any other so it also fits into the debugging processes, and it's also more development time. The time to retrieve the logs is not yet long enough for the log retrieval feature to be more beneficial, but I may change my mind if there are too many issues that I can't reproduce.

J-Jamet commented 3 years ago

KeePassDX-2.9.14_beta_05.zip

Tell me if this new beta compilation solves the problem? ;)

Vstory commented 3 years ago

Tell me if this new beta compilation solves the problem? ;)

The problem is not resolved. all2021-03-16 0-56-23.zip

J-Jamet commented 3 years ago

I don't see any more errors related to the KeePassDX code, the only thing I see a little weird is this: 

03-16 00:56:37.909  2243  2286 I ActivityManager:   Force finishing activity ActivityRecord{fa5ca7a u0 com.kunzisoft.keepass.free/com.kunzisoft.keepass.activities.PasswordActivity t7951}
03-16 00:56:37.909  2243  2286 I am_finish_activity: [0,262523514,7951,com.kunzisoft.keepass.free/com.kunzisoft.keepass.activities.PasswordActivity,force-stop]
03-16 00:56:37.912  2243  2286 I ActivityManager:   Force finishing activity ActivityRecord{a9863b1 u0 com.kunzisoft.keepass.free/com.kunzisoft.keepass.activities.FileDatabaseSelectActivity t7951}
03-16 00:56:37.915  2243  2286 I am_finish_activity: [0,177759153,7951,com.kunzisoft.keepass.free/com.kunzisoft.keepass.activities.FileDatabaseSelectActivity,force-stop]
03-16 00:56:37.917  2243  2286 I ActivityManager:   Force finishing activity ActivityRecord{ed68147 u0 com.kunzisoft.keepass.free/com.kunzisoft.keepass.activities.PasswordActivity t7951}
03-16 00:56:37.919  2243  2286 I am_finish_activity: [0,248938823,7951,com.kunzisoft.keepass.free/com.kunzisoft.keepass.activities.PasswordActivity,force-stop]
03-16 00:56:37.922  2243  2949 W InputDispatcher: channel 'c608472 com.kunzisoft.keepass.free/com.kunzisoft.keepass.activities.GroupActivity (server)' ~ Consumer closed input channel or an error occurred.  events=0x9
03-16 00:56:37.922  2243  2949 E InputDispatcher: channel 'c608472 com.kunzisoft.keepass.free/com.kunzisoft.keepass.activities.GroupActivity (server)' ~ Channel is unrecoverably broken and will be disposed!
03-16 00:56:37.922  2243  2949 W InputDispatcher: channel 'adf95b3 com.kunzisoft.keepass.free/com.kunzisoft.keepass.activities.GroupActivity (server)' ~ Consumer closed input channel or an error occurred.  events=0x9
03-16 00:56:37.922  2243  2949 E InputDispatcher: channel 'adf95b3 com.kunzisoft.keepass.free/com.kunzisoft.keepass.activities.GroupActivity (server)' ~ Channel is unrecoverably broken and will be disposed!
03-16 00:56:37.922  2243  2949 W InputDispatcher: channel 'e267c9d com.kunzisoft.keepass.free/com.kunzisoft.keepass.activities.AutofillLauncherActivity (server)' ~ Consumer closed input channel or an error occurred.  events=0x9
03-16 00:56:37.922  2243  2949 E InputDispatcher: channel 'e267c9d com.kunzisoft.keepass.free/com.kunzisoft.keepass.activities.AutofillLauncherActivity (server)' ~ Channel is unrecoverably broken and will be disposed!
03-16 00:56:37.922  2243  2949 W InputDispatcher: channel 'fa004c2 com.kunzisoft.keepass.free/com.kunzisoft.keepass.activities.PasswordActivity (server)' ~ Consumer closed input channel or an error occurred.  events=0x9
03-16 00:56:37.922  2243  2949 E InputDispatcher: channel 'fa004c2 com.kunzisoft.keepass.free/com.kunzisoft.keepass.activities.PasswordActivity (server)' ~ Channel is unrecoverably broken and will be disposed!
03-16 00:56:37.922  2243  2949 W InputDispatcher: channel '7cc09a5 com.kunzisoft.keepass.free/com.kunzisoft.keepass.activities.FileDatabaseSelectActivity (server)' ~ Consumer closed input channel or an error occurred.  events=0x9
03-16 00:56:37.922  2243  2949 E InputDispatcher: channel '7cc09a5 com.kunzisoft.keepass.free/com.kunzisoft.keepass.activities.FileDatabaseSelectActivity (server)' ~ Channel is unrecoverably broken and will be disposed!
03-16 00:56:37.922  2243  2949 W InputDispatcher: channel '9fcf082 com.kunzisoft.keepass.free/com.kunzisoft.keepass.activities.PasswordActivity (server)' ~ Consumer closed input channel or an error occurred.  events=0x9
03-16 00:56:37.922  2243  2949 E InputDispatcher: channel '9fcf082 com.kunzisoft.keepass.free/com.kunzisoft.keepass.activities.PasswordActivity (server)' ~ Channel is unrecoverably broken and will be disposed!
03-16 00:56:37.923  2243  2286 I ActivityManager:   Force finishing activity ActivityRecord{21bd60e u0 com.kunzisoft.keepass.free/com.kunzisoft.keepass.activities.GroupActivity t7951}
03-16 00:56:37.923  2243  2286 I am_finish_activity: [0,35378702,7951,com.kunzisoft.keepass.free/com.kunzisoft.keepass.activities.GroupActivity,force-stop]

So I don't understand why your save during an autofill would be infinite. Have you tried with a new database? Do you have MIUI's special permissions enabled for background items? https://github.com/Kunzisoft/KeePassDX/wiki/AutoFill#set-up

Vstory commented 3 years ago

Have you tried with a new database?

The problem is still.

Do you have MIUI's special permissions enabled for background items? https://github.com/Kunzisoft/KeePassDX/wiki/AutoFill#set-up

There is no problem with MIUI permission configuration, because everything was normal before.

J-Jamet commented 3 years ago

I'm out of ideas. I don't know why you have this problem. I haven't changed the way the database actions dialog works. So if it has a different behavior, it might be because of a bad save, but in that case an exception would be raised and the dialog would stop, so it's not that. And it's the same behavior with a new database, so it doesn't come from the database itself but from the display. So maybe it's a library update that causes this behavior on your device, but I don't know which one.

J-Jamet commented 3 years ago

I made minor improvements and fixes based on my previous code. Can you tell me if this changes anything for you? KeePassDX-2.9.14_beta_06.zip

Vstory commented 3 years ago

I made minor improvements and fixes based on my previous code. Can you tell me if this changes anything for you?

No effect. The problem is still. all_2021-03-16_14-39-34.zip Can the reason be found in the damaged database?

Vstory commented 3 years ago

I thought it was an auto-fill problem. I wanted to avoid this problem. I directly used KeePassDX to write to the database, but found that it could not be saved. all_2021-03-16_15-11-30.zip

J-Jamet commented 3 years ago

There is no log that matches the internal code of KeePassDX. For me the problem comes from the configuration of the application in the system. Have you tried with another file manager? (ie: Material Files) If you try with version 2.9.13 with a different file manager, does the problem also happen? The logs indicate that the program is not whitelisted on your device so maybe a MIUI permission, but you told me it worked before, so I don't know.

Vstory commented 3 years ago

Have you tried with another file manager? (ie: Material Files)

I use com.android.documentsui, which is an application for Android to select files.

If you try with version 2.9.13 with a different file manager, does the problem also happen?

Versions before 2.9.14 do not have this problem.I have been using com.android.documentsui to select files.

I installed 2.9.13 and saved the data and everything was normal. The permissions are the same. all_2021-03-16_19-51-29.zip

The logs indicate that the program is not whitelisted on your device so maybe a MIUI permission, but you told me it worked before, so I don't know.

whitelisted？Where is this whitelisted? Give me some information and let me think about it.

(ie: Material Files)

Material Files: No effect, the problem remains. all_2021-03-16_19-44-13.zip

I tried to clear the com.android.documentsui data and select the file again, but the problem still exists.

J-Jamet commented 3 years ago

OK, thank you for all your tests.

The whitelist is what I see in your logs, I will not be able to say more. I think it's a list managed by Xiaomi that defines applications to block or not but if it works with another version it's not the problem.

I saw this error in your logs, which is a problem displaying icons, but there is no connection with an infinite save.

03-16 00:56:01.559 25192 26720 W System.err:    at com.kunzisoft.keepass.icons.IconDrawableFactory.getIconDrawable(IconDrawableFactory.kt:93)
03-16 00:56:01.559 25192 26720 W System.err:    at com.kunzisoft.keepass.icons.IconDrawableFactory.addToCustomCache(IconDrawableFactory.kt:216)
03-16 00:56:01.559 25192 26720 W System.err:    at com.kunzisoft.keepass.icons.IconDrawableFactory.access$addToCustomCache(IconDrawableFactory.kt:50)
03-16 00:56:01.559 25192 26720 W System.err:    at com.kunzisoft.keepass.icons.IconDrawableFactory$assignDatabaseIcon$1.invokeSuspend(IconDrawableFactory.kt:165)
03-16 00:56:01.563 25192 25303 W System.err:    at com.kunzisoft.keepass.icons.IconDrawableFactory.getIconDrawable(IconDrawableFactory.kt:93)
03-16 00:56:01.563 25192 25303 W System.err:    at com.kunzisoft.keepass.icons.IconDrawableFactory.addToCustomCache(IconDrawableFactory.kt:216)
03-16 00:56:01.563 25192 25303 W System.err:    at com.kunzisoft.keepass.icons.IconDrawableFactory.access$addToCustomCache(IconDrawableFactory.kt:50)
03-16 00:56:01.563 25192 25303 W System.err:    at com.kunzisoft.keepass.icons.IconDrawableFactory$assignDatabaseIcon$1.invokeSuspend(IconDrawableFactory.kt:165)

The logs also show that other apps try to do something with KeePassDX activity, but that's another problem.

03-16 15:44:51.596  2247  2292 I sysui_multi_action: [757,919,758,4,806,com.tencent.mm,871,com.tencent.mm.plugin.fts.ui.FTSMainUI,908,com.kunzisoft.keepass.free,1455,1,1456,-471028684]
03-16 15:44:54.022  2247  4338 I sysui_multi_action: [757,906,758,4,806,com.tencent.mm,871,com.tencent.mm.ui.chatting.ChattingUI,908,com.kunzisoft.keepass.free,1452,0,1456,-1829343517]
...
03-16 16:32:51.594  2247  4344 I sysui_multi_action: [757,906,758,4,806,com.coolapk.market,871,com.coolapk.market.view.feed.ReplyActivity,908,com.kunzisoft.keepass.free,1452,0,1456,1828282780]
...
03-16 17:25:28.477  2247  4610 I sysui_multi_action: [757,906,758,4,806,com.tencent.mobileqq,871,com.tencent.mobileqq.search.activity.ContactSearchComponentActivity,908,com.kunzisoft.keepass.free,1452,0,1456,1055331727]
03-16 17:25:30.961  2247  4767 I sysui_multi_action: [757,906,758,4,806,com.tencent.mobileqq,871,com.tencent.mobileqq.activity.ForwardRecentActivity,908,com.kunzisoft.keepass.free,1452,0,1456,-1097849612]

I see that too.

03-16 17:40:07.910  2247  3342 I am_kill : [0,1946,com.kunzisoft.keepass.free,906,empty #17]
03-16 17:40:07.957  2247  3710 I am_proc_died: [0,1946,com.kunzisoft.keepass.free,906,18]

So it is possible that the system kills the saving service (but the associated dialog box remains visible). This shouldn't happen because I'm using a foreground service that has a high priority, and unless the system is in vital memory overflow, it shouldn't kill this service. This is an assumption but I don't know why this behavior would happen only in this version?

Vstory commented 3 years ago

Icon, I imported an icon library file, which contains many icon files.

This was imported very early, and there has been no problem of not being able to save the database before.

J-Jamet commented 3 years ago

I'm not going to be able to understand why you have this behavior on your device on this new version if I don't have a way to reproduce it as well and if there are no KeePassDX errors in the logs. So I'm going to leave this issue open until someone can reproduce and understand the problem on their device.

If anyone else has this issue, please write a comment.

J-Jamet commented 3 years ago

Icon, I imported an icon library file, which contains many icon files.

This shouldn't change anything because even if there are new checks of the icon binaries when there is a saving, you told me that the behavior was the same for a newly created database (so without custom icon and without binary) so it can't come from that. :/

Have you tried with a new database?

The problem is still.

Vstory commented 3 years ago

Have you tried with a new database?

The problem is still.

In my opinion, using the old backup should be indistinguishable from the new database. I am not using a brand new database, but a backup on the PC.

I created a brand new database with KeePaaDX, and the problem did not exist when I tested it again. The difference in the database is probably the imported icon material. I'll test it in detail later.

J-Jamet commented 3 years ago

It changes everything in this case. It is possible that the system of checking binaries has a bug or is too long.

J-Jamet commented 3 years ago

OK, now that I know where to look, I think I know what's going on.

I changed the storage mode of the custom icons so that they are no longer directly binaries in RAM but individual files temporarily encrypted, this theoretically allows to avoid OOM if there are very big ones. But it must also be more time consuming to read when there are a lot of them because when writing the database, each encrypted icon file is read and decrypted to be re-encrypted and written in the database file. You thought that the save was not done or had failed but it was only longer because there are many accesses to the disk of your device to write each icon.

I will try to find a better solution to avoid a lot of disk access while keeping the advantage of avoiding OOM.

J-Jamet commented 3 years ago

In my opinion, using the old backup should be indistinguishable from the new database.

A database that comes from a backup or a new database are two different things, because many actions have been performed on a backup. On a new database, only the basic structure is created. My questions are there to eliminate assumptions when I can't reproduce a behavior. I am not a politician, opinions are worthless to fix a bug. :smile:

Vstory commented 3 years ago

I changed the storage mode of the custom icons so that they are no longer directly binaries in RAM but individual files temporarily encrypted, this theoretically allows to avoid OOM if there are very big ones. But it must also be more time consuming to read when there are a lot of them because when writing the database, each encrypted icon file is read and decrypted to be re-encrypted and written in the database file. You thought that the save was not done or had failed but it was only longer because there are many accesses to the disk of your device to write each icon.

There are indeed many documents. aegis-icons

Snipaste_2021-03-17_21-13-00

Vstory commented 3 years ago

You thought that the save was not done or had failed but it was only longer because there are many accesses to the disk of your device to write each icon.

I tried it, and it took more than 8 minutes to save. 🤦‍♂️

J-Jamet commented 3 years ago

I am improving the code to store only large items per file. It just takes some design time.

Vstory commented 3 years ago

I am improving the code to store only large items per file. It just takes some design time.

Thank you. ❤ I am currently using version 2.9.13.

J-Jamet commented 3 years ago

The changes will be available soon on the store with the version code 62. For the moment, all the icons will be loaded in RAM, I have prepared the code if there are big data but it is rather complicated to manage dynamically so I will do it in other versions. Let me know if it's OK for you.

Vstory commented 3 years ago

The changes will be available soon on the store with the version code 62. For the moment, all the icons will be loaded in RAM, I have prepared the code if there are big data but it is rather complicated to manage dynamically so I will do it in other versions. Let me know if it's OK for you.

No, it's still being saved. I will try to delete all the imported materials and try again.

The test version is 2.9.14(61): In a brand new database, import the same picture material to the database.Then create a new password and save it. The saving process is very fast, no different from 2.9.13.

Where is the problem? The reason for the size of the database? 😔

J-Jamet commented 3 years ago

The version to test is 2.9.14 code 62, version 61 still uses file-based icon storage. It's always been normal for it to save. But how long does it take with version code 62?

Vstory commented 3 years ago

The version to test is 2.9.14 code 62, version 61 still uses file-based icon storage.

No, it's still being saved. I have tried again and I am crawling logcat.

It's always been normal for it to save. But how long does it take with version code 62?

Recording time...

Vstory commented 3 years ago

It's always been normal for it to save. But how long does it take with version code 62?

More than 8 minutes. all2021-03-18 2-01-18.zip

J-Jamet commented 3 years ago

OK, I don't understand why. Can you create a new database, import all the icons inside so that the problem appears and upload the database here on github? This will allow me to develop with a database similar to yours.

Vstory commented 3 years ago

Can you create a new database, import all the icons inside so that the problem appears and upload the database here on github?

I have created a brand new database and tested it with 2.9.14(61), there is no problem, everything is normal.

No problem, do you still need this database?

This will allow me to develop with a database similar to yours.

KeePass-data (2).zip

J-Jamet commented 3 years ago

I have no interest in having a working database. I want one that replicates the problem you have without sensitive data in it. ~So put all the icons inside this new database you just created to reproduces the problem with it.~

OK. If that's not the problem, there must be something else that slows down the save... :/

Vstory commented 3 years ago

I have no interest in having a working database. I want one that replicates the problem you have without sensitive data in it. ~So put all the icons inside this new database you just created to reproduces the problem with it.~

OK. If that's not the problem, there must be something else that slows down the save... :/

I don't know the reason for the trigger, I use too much database content myself. Pictures, files and many other things. I will try my best.

J-Jamet commented 3 years ago

Otherwise I have another optimization to do, I will calculate the hash of a temporary file while it is being written. If you say you have many binaries, it is possible that the hash calculator is taking time to compare identical files. I make these changes and upload a new APK to you.

Vstory commented 3 years ago

Otherwise I have another optimization to do, I will calculate the hash of a temporary file while it is being written. If you say you have many binaries, it is possible that the hash calculator is taking time to compare identical files. I make these changes and upload a new APK to you.

Ok.

J-Jamet commented 3 years ago

KeePassDX-2.9.14_beta_08.zip In this APK, I put the files back as method of storing custom icons and I greatly improved the recognition of the file hash which is now done directly in the outpustream. Tell me if it changes anything for you?

Vstory commented 3 years ago

Tell me if it changes anything for you?

Very good !👍🏻 The problem has been solved by you! Thank you! :)

J-Jamet commented 3 years ago

OK, Nice! I'm still improving some code and uploading it to the store. (it will be version code 63) Finally, optimization is the most difficult at this level.

Difficult issue but we made it! ◕‿◕