ZABBIX BUGS AND ISSUES

Zabbix API item.update doesn't update item 'name' parameter.

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: 2.0.2, 2.0.4rc1, 2.1.0
  • Fix Version/s: 2.0.4rc1, 2.1.0
  • Component/s: API (A)
  • Labels:
  • Zabbix ID:
    NA

Description

Hi!

I tried to update Item params with item.update method of zabbix api, all params has changed succesfully, but 'name' field hasn't updated.
Here is my json requests to zabbix:
{code}
10: json_obj: {'params': {'itemid': u'30917', 'key_': 'failcounter[parts5.time_median]', 'name': 'Failcounter: \xd0\xa1\xd1\x82\xd0\xb0\xd1\x82. \xd1\x81\xd1\x80\xd0\xb5\xd0\xb4. \xd0\xb2\xd1\x80\xd0\xb5\xd0\xbc\xd1\x8f \xd0\xbe\xd1\x82\xd0\xb2\xd0\xb5\xd1\x82\xd0\xb0 \xd0\xbf\xd1\x80\xd0\xbe\xd0\xb5\xd0\xba\xd1\x82\xd0\xb0 parts5', 'description': 'Failcounter: \xd0\xa1\xd1\x82\xd0\xb0\xd1\x82. \xd1\x81\xd1\x80\xd0\xb5\xd0\xb4. \xd0\xb2\xd1\x80\xd0\xb5\xd0\xbc\xd1\x8f \xd0\xbe\xd1\x82\xd0\xb2\xd0\xb5\xd1\x82\xd0\xb0 \xd0\xbf\xd1\x80\xd0\xbe\xd0\xb5\xd0\xba\xd1\x82\xd0\xb0 parts5', 'delay': 300, 'trends': 30, 'value_type': 0, 'templateid': u'10307', 'type': 0, 'history': 14}, 'jsonrpc': '2.0', 'method': 'item.update', 'auth': u'2dcaf3d694a7aea4c656549728c6009e', 'id': 217}
20: Sending: {"params": {"itemid": "30917", "key_": "failcounter[parts5.time_median]", "name": "Failcounter: \u0421\u0442\u0430\u0442. \u0441\u0440\u0435\u0434. \u0432\u0440\u0435\u043c\u044f \u043e\u0442\u0432\u0435\u0442\u0430 \u043f\u0440\u043e\u0435\u043a\u0442\u0430 parts5", "description": "Failcounter: \u0421\u0442\u0430\u0442. \u0441\u0440\u0435\u0434. \u0432\u0440\u0435\u043c\u044f \u043e\u0442\u0432\u0435\u0442\u0430 \u043f\u0440\u043e\u0435\u043a\u0442\u0430 parts5", "delay": 300, "trends": 30, "value_type": 0, "templateid": "10307", "type": 0, "history": 14}, "jsonrpc": "2.0", "method": "item.update", "auth": "2dcaf3d694a7aea4c656549728c6009e", "id": 217}
10: Sending headers: {'Content-Type': 'application/json-rpc', 'User-Agent': 'python/zabbix_api'}
20: Response Code: 200
10: Response Body: {u'jsonrpc': u'2.0', u'result': {u'itemids': [u'30917']}, u'id': 217}
{code}

I'm using python api for making requests. Item 'failcounter[parts5.time_median]' is templated item, and it belongs to template 'Template_App_failcounter".

Activity

Hide
Oleg Egorov added a comment - - edited

RESOLVED in svn://svn.zabbix.com/branches/dev/ZBX-5648 r30619

Show
Oleg Egorov added a comment - - edited RESOLVED in svn://svn.zabbix.com/branches/dev/ZBX-5648 r30619
Hide
Toms added a comment - - edited

(1) This check should be performed in CItemGeneral::CheckInput(), to support check for discovery items, and item prototypes

Oleg Egorov RESOLVED IN svn://svn.zabbix.com/branches/dev/ZBX-5648 r30680

Toms CLOSED. Review my changes in r30688, if OK merge.

Show
Toms added a comment - - edited (1) This check should be performed in CItemGeneral::CheckInput(), to support check for discovery items, and item prototypes Oleg Egorov RESOLVED IN svn://svn.zabbix.com/branches/dev/ZBX-5648 r30680 Toms CLOSED. Review my changes in r30688, if OK merge.
Hide
Toms added a comment -

TESTED

Show
Toms added a comment - TESTED
Hide
Oleg Egorov added a comment -

Fixed in 2.0.4 r30708, 2.1.0 r30709

Show
Oleg Egorov added a comment - Fixed in 2.0.4 r30708, 2.1.0 r30709
Hide
Pavels Jelisejevs added a comment - - edited

(1) This fix must be documented in the 2.0 API changelog.

Oleg Egorov RESOLVED

Pavels Jelisejevs CLOSED.

Pavels Jelisejevs Don't forget to document the remaining fixes when you're done.

Show
Pavels Jelisejevs added a comment - - edited (1) This fix must be documented in the 2.0 API changelog. Oleg Egorov RESOLVED Pavels Jelisejevs CLOSED. Pavels Jelisejevs Don't forget to document the remaining fixes when you're done.
Hide
Pavels Jelisejevs added a comment - - edited

(2) The same check should be performed for graphs, triggers, screens (both templated and plain) and all of the prototypes.

Oleg Egorov RESOLVED IN svn://svn.zabbix.com/branches/dev/ZBX-5648-2 r30786

Pavels Jelisejevs CLOSED.

Show
Pavels Jelisejevs added a comment - - edited (2) The same check should be performed for graphs, triggers, screens (both templated and plain) and all of the prototypes. Oleg Egorov RESOLVED IN svn://svn.zabbix.com/branches/dev/ZBX-5648-2 r30786 Pavels Jelisejevs CLOSED.
Hide
Pavels Jelisejevs added a comment - - edited

(3) You've added a $prototype parameter to the CGraphGeneral::checkInput() method. It's not the best solution, if you need some specific validation rules for either prototypes or graphs, just extend this method in the corresponding class.

Please, pay extra attention to the check on line 146: it looks weird to me.

Oleg Egorov RESOLVED IN svn://svn.zabbix.com/branches/dev/ZBX-5648-2 r30798

Pavels Jelisejevs CLOSED.

Show
Pavels Jelisejevs added a comment - - edited (3) You've added a $prototype parameter to the CGraphGeneral::checkInput() method. It's not the best solution, if you need some specific validation rules for either prototypes or graphs, just extend this method in the corresponding class. Please, pay extra attention to the check on line 146: it looks weird to me. Oleg Egorov RESOLVED IN svn://svn.zabbix.com/branches/dev/ZBX-5648-2 r30798 Pavels Jelisejevs CLOSED.
Hide
Pavels Jelisejevs added a comment - - edited

(4) Don't use isset() to check the "templateid" property, because it will pass through NULLs. Use array_key_exists() instead.

Oleg Egorov RESOLVED IN svn://svn.zabbix.com/branches/dev/ZBX-5648-2 r30798

Pavels Jelisejevs CLOSED.

Show
Pavels Jelisejevs added a comment - - edited (4) Don't use isset() to check the "templateid" property, because it will pass through NULLs. Use array_key_exists() instead. Oleg Egorov RESOLVED IN svn://svn.zabbix.com/branches/dev/ZBX-5648-2 r30798 Pavels Jelisejevs CLOSED.
Hide
Pavels Jelisejevs added a comment - - edited

(5) A similar check has to be performed for template screens. There's a difference, though. In template screens, the "templateid" field refers to the template that the screen belongs to, not to the parent screen. So the "templateid" property must be set when calling create, but cannot be changed later.

Oleg Egorov RESOLVED IN svn://svn.zabbix.com/branches/dev/ZBX-5648-2 r30801

Pavels Jelisejevs CLOSED.

Show
Pavels Jelisejevs added a comment - - edited (5) A similar check has to be performed for template screens. There's a difference, though. In template screens, the "templateid" field refers to the template that the screen belongs to, not to the parent screen. So the "templateid" property must be set when calling create, but cannot be changed later. Oleg Egorov RESOLVED IN svn://svn.zabbix.com/branches/dev/ZBX-5648-2 r30801 Pavels Jelisejevs CLOSED.
Hide
Pavels Jelisejevs added a comment - - edited

(6) Since your fix affected the screen validation, may be you could also implement the correct validate methods for update and delete? See CService as an example. Keep in mind, that CTemplateScreen extends CScreen and already has some validation methods, maybe they can be unified.

Oleg Egorov RESOLVED IN svn://svn.zabbix.com/branches/dev/ZBX-5648-2 r30801
For 'update' and 'create' (not 'delete'), because in 'delete' exist only one check

Pavels Jelisejevs CLOSED.

Show
Pavels Jelisejevs added a comment - - edited (6) Since your fix affected the screen validation, may be you could also implement the correct validate methods for update and delete? See CService as an example. Keep in mind, that CTemplateScreen extends CScreen and already has some validation methods, maybe they can be unified. Oleg Egorov RESOLVED IN svn://svn.zabbix.com/branches/dev/ZBX-5648-2 r30801 For 'update' and 'create' (not 'delete'), because in 'delete' exist only one check Pavels Jelisejevs CLOSED.
Hide
Pavels Jelisejevs added a comment - - edited

(7) There's an error on the template configuration page.

Declaration of CTemplateScreen::validateCreate() should be compatible with that of CScreen::validateCreate() [api/classes/CTemplateScreen.php:25]
Declaration of CTemplateScreen::create() should be compatible with that of CScreen::create() [api/classes/CTemplateScreen.php:25]

Make sure you have strict standard errors enabled.

Oleg Egorov RESOLVED

Pavels Jelisejevs We always use type hinting for arrays, so instead of removing it, you need to add it where it's missing.

Oleg Egorov RESOLVED

Pavels Jelisejevs CLOSED.

Show
Pavels Jelisejevs added a comment - - edited (7) There's an error on the template configuration page. Declaration of CTemplateScreen::validateCreate() should be compatible with that of CScreen::validateCreate() [api/classes/CTemplateScreen.php:25] Declaration of CTemplateScreen::create() should be compatible with that of CScreen::create() [api/classes/CTemplateScreen.php:25] Make sure you have strict standard errors enabled. Oleg Egorov RESOLVED Pavels Jelisejevs We always use type hinting for arrays, so instead of removing it, you need to add it where it's missing. Oleg Egorov RESOLVED Pavels Jelisejevs CLOSED.
Hide
Pavels Jelisejevs added a comment - - edited

(8) I cannot create a plain screen:

Cannot set "templateid" for screen. [CScreen.create -> CScreen.validateCreate]

Oleg Egorov RESOLVED

Pavels Jelisejevs CLOSED.

Show
Pavels Jelisejevs added a comment - - edited (8) I cannot create a plain screen: Cannot set "templateid" for screen. [CScreen.create -> CScreen.validateCreate] Oleg Egorov RESOLVED Pavels Jelisejevs CLOSED.
Hide
Pavels Jelisejevs added a comment - - edited

(9) Some issues regarding screens:

1. In CScreen::validateCreate() we first need to check if the fields are correct, only then - if the screen doesn't exist.
2. In CScreen::validateUpdate() permission checks and field checks should be performed in separate loops.
3. Templateid is not validated when calling screen.update.

Oleg Egorov RESOLVED

Pavels Jelisejevs Please review my changes in r30877. If everything is OK, it can be closed.

Oleg Egorov CLOSED

Show
Pavels Jelisejevs added a comment - - edited (9) Some issues regarding screens: 1. In CScreen::validateCreate() we first need to check if the fields are correct, only then - if the screen doesn't exist. 2. In CScreen::validateUpdate() permission checks and field checks should be performed in separate loops. 3. Templateid is not validated when calling screen.update. Oleg Egorov RESOLVED Pavels Jelisejevs Please review my changes in r30877. If everything is OK, it can be closed. Oleg Egorov CLOSED
Hide
Pavels Jelisejevs added a comment - - edited

(10) In CTemplateScreen::validateUpdate():

1. templateid must be validated after permissions;
2. remove the old templateid check.

Oleg Egorov RESOLVED

Pavels Jelisejevs CLOSED.

Show
Pavels Jelisejevs added a comment - - edited (10) In CTemplateScreen::validateUpdate(): 1. templateid must be validated after permissions; 2. remove the old templateid check. Oleg Egorov RESOLVED Pavels Jelisejevs CLOSED.
Hide
Pavels Jelisejevs added a comment - - edited

(11) Some error message improvements:

1. When calling update methods, the error should read "Cannot update" instead of "Cannot set";
2. The error should always mention the name of the screen.

So, for create methods it should be: "Cannot set "templateid" for screen "%1$s".
For update methods: "Cannot update "templateid" for screen "%1$s".

Keep in mind, that the name of the screen may not be passed to update methods, so you may have to pull it out of the database using the CZBXAPI::extendObjects() methods or in some other way.

Oleg Egorov RESOLVED

Pavels Jelisejevs CLOSED.

Show
Pavels Jelisejevs added a comment - - edited (11) Some error message improvements: 1. When calling update methods, the error should read "Cannot update" instead of "Cannot set"; 2. The error should always mention the name of the screen. So, for create methods it should be: "Cannot set "templateid" for screen "%1$s". For update methods: "Cannot update "templateid" for screen "%1$s". Keep in mind, that the name of the screen may not be passed to update methods, so you may have to pull it out of the database using the CZBXAPI::extendObjects() methods or in some other way. Oleg Egorov RESOLVED Pavels Jelisejevs CLOSED.
Hide
Pavels Jelisejevs added a comment - - edited

(12) In CTrigger::checkInput() templateid check must be performed before the permission check.

Oleg Egorov RESOLVED

Pavels Jelisejevs Please review my changes in r30879.

Oleg Egorov CLOSED

Show
Pavels Jelisejevs added a comment - - edited (12) In CTrigger::checkInput() templateid check must be performed before the permission check. Oleg Egorov RESOLVED Pavels Jelisejevs Please review my changes in r30879. Oleg Egorov CLOSED
Hide
Pavels Jelisejevs added a comment - - edited

(13) In CGraph::checkInput():

1. Permissions need to be validated before calling parent::checkInput(). Don't forget, that before checking permissions you need to check that items and item IDs are valid.
2. We don't check permissions for super admins, so we don't need to make the API request for super admins.

Oleg Egorov RESOLVED

Pavels Jelisejevs Before using items IDs to validate permissions, we need to validate the items and item IDs themselves. That is, the check in CGraphGeneral::checkInput() lines 66 and 73 have to be performed before permission checks.

Also please review my changes in r30879.

Oleg Egorov RESOLVED

Pavels Jelisejevs itemids also needed to be validated. Please review my changes in r30894.

Oleg Egorov OK, CLOSED

Show
Pavels Jelisejevs added a comment - - edited (13) In CGraph::checkInput(): 1. Permissions need to be validated before calling parent::checkInput(). Don't forget, that before checking permissions you need to check that items and item IDs are valid. 2. We don't check permissions for super admins, so we don't need to make the API request for super admins. Oleg Egorov RESOLVED Pavels Jelisejevs Before using items IDs to validate permissions, we need to validate the items and item IDs themselves. That is, the check in CGraphGeneral::checkInput() lines 66 and 73 have to be performed before permission checks. Also please review my changes in r30879. Oleg Egorov RESOLVED Pavels Jelisejevs itemids also needed to be validated. Please review my changes in r30894. Oleg Egorov OK, CLOSED
Hide
Pavels Jelisejevs added a comment - - edited

(14) In CGraphPrototype::checkInput():

1. Permissions need to be validated before calling parent::checkInput(). Don't forget, that before checking permissions you need to check that items and item IDs are valid.

Oleg Egorov Duplicated in (13)

Pavels Jelisejevs Sorry, I meant CGraphPrototype.

Oleg Egorov RESOLVED

Pavels Jelisejevs Same problem as in (13).

Oleg Egorov RESOLVED

Pavels Jelisejevs CLOSED.

Show
Pavels Jelisejevs added a comment - - edited (14) In CGraphPrototype::checkInput(): 1. Permissions need to be validated before calling parent::checkInput(). Don't forget, that before checking permissions you need to check that items and item IDs are valid. Oleg Egorov Duplicated in (13) Pavels Jelisejevs Sorry, I meant CGraphPrototype. Oleg Egorov RESOLVED Pavels Jelisejevs Same problem as in (13). Oleg Egorov RESOLVED Pavels Jelisejevs CLOSED.
Hide
Alexander Vladishev added a comment - - edited

(15) Cannot Full clone a template: "Cannot set "templateid" for item."

Oleg Egorov RESOLVED

Pavels Jelisejevs Please review my changes in r30904.

Oleg Egorov CLOSED

Show
Alexander Vladishev added a comment - - edited (15) Cannot Full clone a template: "Cannot set "templateid" for item." Oleg Egorov RESOLVED Pavels Jelisejevs Please review my changes in r30904. Oleg Egorov CLOSED
Hide
Pavels Jelisejevs added a comment - - edited

(16) Almost forgot: this also has to be done for applications.

Oleg Egorov RESOLVED in svn://svn.zabbix.com/branches/dev/ZBX-5648-2 r30863

Pavels Jelisejevs Please review my changes in r30884.

Oleg Egorov CLOSED

Show
Pavels Jelisejevs added a comment - - edited (16) Almost forgot: this also has to be done for applications. Oleg Egorov RESOLVED in svn://svn.zabbix.com/branches/dev/ZBX-5648-2 r30863 Pavels Jelisejevs Please review my changes in r30884. Oleg Egorov CLOSED
Hide
Pavels Jelisejevs added a comment - - edited

(17) Cannot create a graph prototype:

Undefined variable: allowedItems [api/classes/CGraphPrototype.php:890]
Graph prototype must have at least one prototype. [CGraphGeneral.create -> CGraphPrototype.checkInput]

Oleg Egorov This error not exist in my last revision

RESOLVED

Pavels Jelisejevs CLOSED.

Show
Pavels Jelisejevs added a comment - - edited (17) Cannot create a graph prototype: Undefined variable: allowedItems [api/classes/CGraphPrototype.php:890] Graph prototype must have at least one prototype. [CGraphGeneral.create -> CGraphPrototype.checkInput] Oleg Egorov This error not exist in my last revision RESOLVED Pavels Jelisejevs CLOSED.
Hide
Oleg Egorov added a comment -

FIXED IN 2.0.4rc1 r30905, 2.1.0 (trunk) r30908

Show
Oleg Egorov added a comment - FIXED IN 2.0.4rc1 r30905, 2.1.0 (trunk) r30908

People

Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: