Deleting "Distribution" Info corrupts whole database

[mmatthiesencsc]

Metashare allows to remove Distribution information on an existing article and as a consequence the whole database gets corrupted and makes the whole site unusable.

To repeat (make sure you have a full database backup)
Site: metashare.csc.fi
Article: http://metashare.csc.fi/repository/browse/samples-of-spoken-finnish/642b58defccc11e18b49005056be118e3444ea5bb1dd46a5a4ca4829e93da406/
Login as editor, click edit resource
Remove "Distribution" with the [x]
Click Save
-> you get an internal server error 500.
Now try to lookup any other resource on the site. Matches are found, but trying to lookup the article results in internal server errors.

Workaround: The only quick fix is to restore the database prior to the failed operation.

This error was discovered when a user deleted the Distribution information and added new Distribution information. It seems that the deletion alone leaves the database in an undefined state.

Fix suggestion: It should not be allowed to save the article with no Distribution information. Deletion and immediate addition should be possible.

[mmatthiesencsc]

Here's the very first error with some Cookie information removed:

<WSGIRequest
GET:<QueryDict: {}>,
POST:<QueryDict: {}>,
COOKIES:(removed),
META:{'CSRF_COOKIE': '(removed)',
'DOCUMENT_ROOT': '(removed)',
'GATEWAY_INTERFACE': 'CGI/1.1',
'HTTP_ACCEPT': 'text/html,application/xhtml+xml,application/xml;q=0.9,/;q=0.8',
'HTTP_ACCEPT_ENCODING': 'gzip, deflate',
'HTTP_ACCEPT_LANGUAGE': 'en-US,en;q=0.5',
'HTTP_CONNECTION': 'keep-alive',
'HTTP_COOKIE': '(removed)',
'HTTP_HOST': 'metashare.csc.fi',
'HTTP_USER_AGENT': 'Mozilla/5.0 (Windows NT 6.1; WOW64; rv:30.0) Gecko/20100101 Firefox/30.0',
'PATH_INFO': u'/admin/repository/resourceinfotype_model/24/images/favicon.ico/',
'PATH_TRANSLATED': '/v/META-SHARE-3.0.1/metashare//admin/repository/resourceinfotype_model/24/images/favicon.ico/',
'QUERY_STRING': '',
'REDIRECT_STATUS': '200',
'REDIRECT_URI': '/metashare.fcgi/admin/repository/resourceinfotype_model/24/images/favicon.ico/',
'REMOTE_ADDR': '193.166.84.61',
'REMOTE_PORT': '52719',
'REQUEST_METHOD': 'GET',
'REQUEST_URI': '/admin/repository/resourceinfotype_model/24/images/favicon.ico/',
'SCRIPT_FILENAME': '/v/META-SHARE-3.0.1/metashare/metashare.fcgi',
'SCRIPT_NAME': u'',
'SERVER_ADDR': '86.50.27.125',
'SERVER_NAME': 'metashare.csc.fi',
'SERVER_PORT': '80',
'SERVER_PROTOCOL': 'HTTP/1.1',
'SERVER_SOFTWARE': 'lighttpd',
'wsgi.errors': <flup.server.fcgi_base.TeeOutputStream object at 0x7fdca8ba3d90>,
'wsgi.input': <flup.server.fcgi_base.InputStream object at 0x7fdca8bcbf10>,
'wsgi.multiprocess': False,
'wsgi.multithread': True,
'wsgi.run_once': False,
'wsgi.url_scheme': 'http',
'wsgi.version': (1, 0)}>

[mmatthiesencsc]

No comment in 3,5 months? Is this bug database maintained in any way?

[zeehio]

As far as I know nobody is taking care of these issues. This bug is very easy to reproduce, so with some knowledge of django and the metashare code it should not be that hard to fix. Unfortunately, there is no one with this knowledge anymore around here I think.

As maintainer of the metashare.talp.cat node, I have been working in my free time on an upgrade of the metashare code to depend on django-1.4. The aim is to address some security issues of django-1.3 and keep dependencies as upgraded as possible. If I come across or I am able find the code where the fix to this issue should be applied I will take care of it, but I can't promise that I will be able to fix this issue, nor that my pull request will be revised and merged.

[jsteffen]

I would be happy to review and merge your pull request.
Thanks for the effort!

[mmatthiesencsc]

Hi Sergio, thanks for your comment. Unfortunately I don't know Django and certainly not Metashare's inner workings. But I'd be happy to help with testing, if that helps.

[zeehio]

So far my only progress has been in testing further the issue:

1. Create a resource
2. Start to modify the existing resource
3. Delete (clicking on the "X") the Distribution information
4. Confirm clicking "Yes, I am sure" in the pop-up.

Note how this popup warns that not only the distribution information is going to be deleted, but also that the resource that depends on it will be removed too. We don't need to click "Save", the resource has already been deleted and the user probably did not want that. The easiest fix would be to disable the removal of the distribution information, or at least to not make appear the "X" symbol next to the distribution information, so the user is force to edit the model preventing the inconsistency. I do not know enough django to know where should I apply the patch though... I'll keep looking

[mmatthiesencsc]

Hello Sergio, thanks for further looking into this. Is there anyone from the Metashare team who could assisst? Jörg? The problem is that this bug truly messes things up and I have a production server I'd rather not use for testing. Cheers, Martin

[jsteffen]

Hi Martin,
sorry, I have to pass. As I already explained to Sergio, all core developers of META-SHARE at DFKI are no longer available. I'm the last person at DFKI who had worked on META-SHARE, but I lack the required expertise to fix this kind of bugs. Apart from that, there are no longer resources available (at least at DFKI, probably at all) to spend on META-SHARE. Still, I'm trying to find some time to check the merge request of Sergio for issue #751.
Best regards, Jörg

[zeehio]

Hi,

I just committed to my metashare fork a patch that solves this issue. It's added in the pull request for django-1.4 support, that will be reviewed by @jsteffen at some point.

The patch is very simple (click on the commit to see it): When the editor asks the model if it can be deleted, if the model is distributionInfoType then the editor does not show the "delete" button.

The patch is right, as in "it solves the issue" without known side effects (I can't think of a side effect caused by this change and all the existing tests pass).

The patch is wrong because it is the related_modeladmin.has_delete_permission(request) that should return False in this case, instead of having to patch the editor.

Unfortunately, I don't know enough django to address this issue better, so unless someone comes with a better solution I would suggest merging this one. It may be a bit messy (I am slightly coupling the editor to the models) but I believe it is well documented.

[mmatthiesencsc]

Hi Sergio! I remember writing a big thank you a while a go, when I applied your patch and it works as advertised! I must have somehow forgotten to press "Comment". Anyway, I consider this issue solved, thanks again!
Martin