Register application/x-kdbx the right way #124

kesselb · 2019-12-07T20:21:52Z

Thanks for your app 👍 Some people at nextcloud/server (e.g. nextcloud/server#17217) and also over here are reporting issues. *.kdbx files are not opened with the app 😞

This pull request should fix most of these issues and also make the manual changes to mimetypemapping.json superfluous (at least on my setup).

I'm not a heavy user of keeweb. We probably need some testers ;)

- Change x-application/kdbx to application/x-kdbx (as in viewer.js) - Fix call to updateFilecache (extension is requested not a pattern) - Inject IMimeTypeLoader via DI Signed-off-by: Daniel Kesselberg <[email protected]>

- Fix call to updateFilecache (extension is requested not a pattern) - Inject IMimeTypeLoader via DI Signed-off-by: Daniel Kesselberg <[email protected]>

Signed-off-by: Daniel Kesselberg <[email protected]>

I'm not 100% sure about this change. It should not be necessary to call getAllMappings to register a new mime type. Probably a definition with config/mimetypemapping.json will overwrite this definition later. App initialization should be cheap. Expensive jobs like getAllMappings are bad. Signed-off-by: Daniel Kesselberg <[email protected]>

kesselb · 2019-12-07T20:23:33Z

keeweb/lib/Migration/AddMimetypeToFilecache.php

+    public function run(IOutput $output) {
+        // And update the filecache for it.
+        $mimetypeId = $this->mimeTypeLoader->getId('application/x-kdbx');
+        $this->mimeTypeLoader->updateFilecache('kdbx', $mimetypeId);


updateFilecache is looking for a extension. A pattern like %.kdbx do not work.

kesselb · 2019-12-07T20:26:32Z

keeweb/lib/Migration/AddMimetypeToFilecache.php


-    // And update the filecache for it.
-    $mimetypeId = $mimeTypeLoader->getId('x-application/kdbx');


nextcloud-keeweb/keeweb/js/viewer.js

Line 21 in 380e78f

mime: 'application/x-kdbx',

Do not work because they mimetype is created as x-application/kdbx over here.

maurerle · 2019-12-08T11:39:46Z

can confirm this is working (tested in newly setup nc-docker with nc 17).
executing select * from oc_mimetypes; contains application/x-kdbx as it should.
And selecting kdbx opens the keeweb application.

this should definitly get merged

maurerle · 2019-12-08T12:39:16Z

New files or edited files are not detected correctly.

Use this patch, existing file is recognized correctly.
Add a new kdbx file.
New kdbx file is not available to open with keeweb

As the mimetypes are correctly in the database it seems that there is something wrong with detecting the mimetype correctly.
Running php occ maintenance:mimetype:update-db --repair-filecache resolves this until the next save.

Could you look into that?

Filesystem loads the app also for request via dav. This is required to assign the mime type. Signed-off-by: Daniel Kesselberg <[email protected]>

- Register mime type before app initialization - Skip the app initialization for dav requests Signed-off-by: Daniel Kesselberg <[email protected]>

kesselb · 2019-12-08T13:37:17Z

Thanks @maurerle 👍

Make sense to me. Keeweb app is not loaded for dav requests and therefore the mime type (.kdbx => application/x-kdbx) unknown. Added the app to dav requests (types: filesystem). Nextcloud caches the types information in oc_appconfig table. You probably have to modify the types row for keeweb and add filesystem.

kesselb · 2019-12-08T13:38:46Z

keeweb/appinfo/app.php

+$mimeTypeDetector = \OC::$server->getMimeTypeDetector();
+$mimeTypeDetector->registerType('kdbx', 'application/x-kdbx', 'application/x-kdbx');
+
+if (\OC::$REQUESTEDAPP === 'dav') {


cc @rullzer @nickvergessen does that make sense? ;) For a dav request we need to announce the mime type to nextcloud. The app itself is not required. I'm trying to save some resources by skipping the app initialization.

maurerle · 2019-12-08T15:02:33Z

This works! 😄
So types-filesystem loads this app with every filesystem or dav request. And loading the app runs the mimetype detection?

So when doing it through mimetypemapping.json this process is done by the core mimetype detector right?

kesselb · 2019-12-08T15:08:59Z

So types-filesystem loads this app with every filesystem or dav request. And loading the app runs the mimetype detection?

Loading the app tells nextcloud that files with kdbx extension should have the mime type application/x-kdbx. For dav request we don't initialize the app to save resources.

nextcloud-keeweb/keeweb/appinfo/app.php

Lines 17 to 23 in e0ee3d9

    
           $mimeTypeDetector = \OC::$server->getMimeTypeDetector(); 
        
           $mimeTypeDetector->registerType('kdbx', 'application/x-kdbx', 'application/x-kdbx'); 
        
           if (\OC::$REQUESTEDAPP === 'dav') { 
        
               /** For dav requests it should be enough to register the mime type and skip the rest of the app initialization. */ 
        
               return; 
        
           }

So when doing it through mimetypemapping.json this process is done by the core mimetype detector right?

Yes. A entry in mimetypemapping.json tells nextcloud that files with kdbx extension should have the mime type application/x-kdbx.

It's required to call getAllMappings before registerType otherwise the default mappings are not loaded. Signed-off-by: Daniel Kesselberg <[email protected]>

kesselb · 2019-12-09T15:29:12Z

Nextcloud 18 will contain kdbx => application/x-kdbx by default. We should keep this approach until 17 is EOL or release different version fors 17/18.

jhass · 2020-01-19T12:30:06Z

So with this we can drop the instructions from the readme right?

Sorry for not responding earlier here. To be honest I don't know enough about Nextcloud to fully verify this is sane, but it sounds like you know what you're doing here. So I'm sure it won't be any more broken than it is right now, so I'll merge this for now. Btw if you're interested in (co-) maintaining... ;)

kesselb · 2020-01-21T14:00:06Z

Thanks for merging 👍

you know what you're doing her

I hope 🙈

Btw if you're interested in (co-) maintaining

Sorry 😞 I don't have enough time for co-maintaining a app. Luckily there seems to be some progress with the documentation for developers so it should be easier to keep up with the latest changes and better instructions how to migrate code. I think that most of the issues reports here should be solved with this PR. That will hopefully reduce your workload. Ping me if you're stuck somehow. I'll try to have a look.

kesselb added 4 commits December 7, 2019 21:09

Register mime type on install via repair step.

4d1fdaf

- Change x-application/kdbx to application/x-kdbx (as in viewer.js) - Fix call to updateFilecache (extension is requested not a pattern) - Inject IMimeTypeLoader via DI Signed-off-by: Daniel Kesselberg <[email protected]>

Remove mime type on uninstall via repair step.

9e25ca7

- Fix call to updateFilecache (extension is requested not a pattern) - Inject IMimeTypeLoader via DI Signed-off-by: Daniel Kesselberg <[email protected]>

$mimeTypeLoader is not used

7904a7c

Signed-off-by: Daniel Kesselberg <[email protected]>

kesselb commented Dec 7, 2019

View reviewed changes

This was referenced Dec 7, 2019

mimetypes not updating nextcloud/server#17217

Closed

Update Mimetype Dectection #105

Closed

maurerle approved these changes Dec 8, 2019

View reviewed changes

kesselb added 2 commits December 8, 2019 14:20

Register app for as filesystem

8411f39

Filesystem loads the app also for request via dav. This is required to assign the mime type. Signed-off-by: Daniel Kesselberg <[email protected]>

Register mime type before app initialization

e0ee3d9

- Register mime type before app initialization - Skip the app initialization for dav requests Signed-off-by: Daniel Kesselberg <[email protected]>

kesselb commented Dec 8, 2019

View reviewed changes

Add getAllMappings again

6d6ae18

It's required to call getAllMappings before registerType otherwise the default mappings are not loaded. Signed-off-by: Daniel Kesselberg <[email protected]>

kesselb mentioned this pull request Dec 8, 2019

Add mimetype for keepass database. nextcloud/server#18287

Merged

jhass merged commit 6c0b6ea into jhass:master Jan 19, 2020

jhass mentioned this pull request Jan 19, 2020

Update link for mimetypes configuration #123

Closed

PriceChild mentioned this pull request Jan 20, 2020

Add application/x-kdbx mimetype bpellin/keepassdroid#352

Merged

kesselb deleted the fix/register-mime-type branch January 21, 2020 14:00

This was referenced Feb 17, 2020

App/Plugin API for mimetypes nextcloud/server#9192

Closed

Don't load app for dav requests #135

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Register application/x-kdbx the right way #124

Register application/x-kdbx the right way #124

kesselb commented Dec 7, 2019 •

edited

Loading

kesselb Dec 7, 2019

kesselb Dec 7, 2019

maurerle commented Dec 8, 2019 •

edited

Loading

maurerle commented Dec 8, 2019

kesselb commented Dec 8, 2019

kesselb Dec 8, 2019 •

edited

Loading

maurerle commented Dec 8, 2019

kesselb commented Dec 8, 2019

kesselb commented Dec 9, 2019

jhass commented Jan 19, 2020

kesselb commented Jan 21, 2020


		// And update the filecache for it.
		$mimetypeId = $mimeTypeLoader->getId('x-application/kdbx');

Register application/x-kdbx the right way #124

Register application/x-kdbx the right way #124

Conversation

kesselb commented Dec 7, 2019 • edited Loading

kesselb Dec 7, 2019

Choose a reason for hiding this comment

kesselb Dec 7, 2019

Choose a reason for hiding this comment

maurerle commented Dec 8, 2019 • edited Loading

maurerle commented Dec 8, 2019

kesselb commented Dec 8, 2019

kesselb Dec 8, 2019 • edited Loading

Choose a reason for hiding this comment

maurerle commented Dec 8, 2019

kesselb commented Dec 8, 2019

kesselb commented Dec 9, 2019

jhass commented Jan 19, 2020

kesselb commented Jan 21, 2020

kesselb commented Dec 7, 2019 •

edited

Loading

maurerle commented Dec 8, 2019 •

edited

Loading

kesselb Dec 8, 2019 •

edited

Loading