artefactual / archivematica-storage-service

Archivematica storage service
http://www.archivematica.org
GNU Affero General Public License v3.0
34 stars 43 forks source link

Catch ValueError from literal_eval #231

Open helenst opened 7 years ago

helenst commented 7 years ago

Settings can be uuids, and some uuids are valid Python expressions, so running literal_eval on a settings value might raise a ValueError (to reject an expression) rather than SyntaxError

Done here in Jisc, needs porting to core. https://github.com/JiscRDSS/archivematica-storage-service/pull/14

Related to #211

ross-spencer commented 6 years ago

Seeing this error with my database in its current form:

select * from administration_settings;

id,name,value
1,default_transfer_source,[u'7d4f7847-1da6-4e0e-b5a8-1af1b9eb7cc2']
2,default_aip_storage,[u'e9c219df-40ed-44b5-90c3-f385d7057fdc']
3,default_dip_storage,[u'cc959ec7-6a61-485a-9668-441e0ada4ae3']
4,default_backlog,[u'c49c798d-b1b8-4066-a1ca-e554bb549081']
5,default_recovery,[u'9e3bf287-fc58-4066-ab87-f820711ab4cf']
6,default_TS_location,7d4f7847-1da6-4e0e-b5a8-1af1b9eb7cc2
7,default_AS_location,e9c219df-40ed-44b5-90c3-f385d7057fdc
8,default_DS_location,cc959ec7-6a61-485a-9668-441e0ada4ae3
9,default_BL_location,c49c798d-b1b8-4066-a1ca-e554bb549081
10,default_AR_location,9e3bf287-fc58-4066-ab87-f820711ab4cf

The result is that it impacts my ability to bring up the administration page of the SS Dashboard.

image

@sevein has highlighted this maybe the line causing the issue: https://github.com/artefactual/archivematica-storage-service/blob/stable/0.11.x/storage_service/locations/models/location.py#L159

And also notes, it might be an idea to move set_setting and the others into the Setting model, if there’s business logic for a model it should be in the model.

This function also exists to wrap some literals when needed: https://github.com/JiscRDSS/archivematica-storage-service/blob/cdb02e251842fe90d6384695c062c6f3141c32c9/storage_service/common/utils.py#L73

NB. There is a workaround/fix PR in the RDSS issue referenced above.

ross-spencer commented 6 years ago

Workaround

Access individual urls under Admin, e.g: