ZABBIX BUGS AND ISSUES
  1. ZABBIX BUGS AND ISSUES
  2. ZBX-8195

httptest.update can create scenario steps without name

    Details

      Description

      Doing httptest.update and not passing "name" in steps, will remove existing steps and create new step without name. Step then cannot be clicked and edited in frontend.

        Issue Links

          Activity

          Hide
          Ivo Kurzemnieks added a comment - - edited

          (1) Related issue with step name validation:

          • have a template linked to a host;
          • on template create a web scenario with steps
          • perform httptest.update for that template scenario and add steps without name

          it results in error "Item with key \"web.test.in[test,,bps]\" already exists.", but this happends only in 2.2

          Krists Krigers RESOLVED in r45617 - validation now does not allow to update step name to empty string.

          Oleg Egorov CLOSED

          Show
          Ivo Kurzemnieks added a comment - - edited (1) Related issue with step name validation: have a template linked to a host; on template create a web scenario with steps perform httptest.update for that template scenario and add steps without name it results in error "Item with key \"web.test.in [test,,bps] \" already exists.", but this happends only in 2.2 Krists Krigers RESOLVED in r45617 - validation now does not allow to update step name to empty string. Oleg Egorov CLOSED
          Hide
          Krists Krigers (Inactive) added a comment -

          Fixed validation in r45617, branch svn://svn.zabbix.com/branches/dev/ZBX-8195.

          Show
          Krists Krigers (Inactive) added a comment - Fixed validation in r45617, branch svn://svn.zabbix.com/branches/dev/ZBX-8195.
          Hide
          Oleg Egorov added a comment - - edited

          (2) String changes?
          New translatable string:

          • 'Web scenario step name cannot be empty.'
          • 'Web scenario step URL cannot be empty.'
          • 'If specified, web scenario step ID must belong to the web scenario.'

          Oleg Egorov

          "Web scenario step name cannot be empty."

          REOPEN

          Krists Krigers RESOLVED.

          Oleg Egorov

          'If specified, web scenario step ID must belong to the web scenario.'...

          Better change to no permission/not exist message.

          REOPEN

          Krists Krigers RESOLVED in r45779.

          Oleg Egorov CLOSED

          Show
          Oleg Egorov added a comment - - edited (2) String changes? New translatable string: 'Web scenario step name cannot be empty.' 'Web scenario step URL cannot be empty.' 'If specified, web scenario step ID must belong to the web scenario.' Oleg Egorov "Web scenario step name cannot be empty." REOPEN Krists Krigers RESOLVED. Oleg Egorov 'If specified, web scenario step ID must belong to the web scenario.'... Better change to no permission/not exist message. REOPEN Krists Krigers RESOLVED in r45779. Oleg Egorov CLOSED
          Hide
          Oleg Egorov added a comment - - edited

          (3) API request:

          {
                  "name": "Homepage check8",
                  "hostid": "10105",
                  "steps": [
                      {
                          "name": 0,
                          "url": "http://mycompany.com",
                          "status_codes": 200,
                          "no": 1
                      }
                  ]
              }
          

          Response

          {
              "jsonrpc": "2.0",
              "error": {
                  "code": -32602,
                  "message": "Invalid params.",
                  "data": "Web scenario step name can not be empty."
              },
              "id": 17
          }
          

          0 - valid name

          Krists Krigers RESOLVED in r45686.

          Oleg Egorov

          Possible execute with name = null

          {
              "httptestid": 15,
              "name": "Homepage check30",
              "hostid": "10105",
              "steps": [
                  {
                      "httpstepid": 9999,
                      "name": null
                  }
              ]
          }
          

          REOPEN

          Krists Krigers RESOLVED in r45740.

          Oleg Egorov

          {
              "httptestid": 13,
              "name": "Homepage check25",
              "hostid": "10105",
              "steps": [
                  {
                      "httpstepid": 21,
                      "name": 0
                  }
              ]
          }
          

          Again...

          Web scenario step name cannot be empty.

          URL have same problem

          REOPEN

          Krists Krigers RESOLVED in r45779.

          Oleg Egorov CLOSED

          Show
          Oleg Egorov added a comment - - edited (3) API request: { "name" : "Homepage check8" , "hostid" : "10105" , "steps" : [ { "name" : 0, "url" : "http: //mycompany.com" , "status_codes" : 200, "no" : 1 } ] } Response { "jsonrpc" : "2.0" , "error" : { "code" : -32602, "message" : "Invalid params." , "data" : "Web scenario step name can not be empty." }, "id" : 17 } 0 - valid name Krists Krigers RESOLVED in r45686. Oleg Egorov Possible execute with name = null { "httptestid" : 15, "name" : "Homepage check30" , "hostid" : "10105" , "steps" : [ { "httpstepid" : 9999, "name" : null } ] } REOPEN Krists Krigers RESOLVED in r45740. Oleg Egorov { "httptestid" : 13, "name" : "Homepage check25" , "hostid" : "10105" , "steps" : [ { "httpstepid" : 21, "name" : 0 } ] } Again... Web scenario step name cannot be empty. URL have same problem REOPEN Krists Krigers RESOLVED in r45779. Oleg Egorov CLOSED
          Hide
          Oleg Egorov added a comment - - edited

          (4)

          if (isset($step['httpstepid'])
          	? isset($step['name']) && ($step['name'] == '')
          	: !isset($step['name']) || ($step['name'] == '')) {
          	self::exception(ZBX_API_ERROR_PARAMETERS, _('Web scenario step name can not be empty.'));
          }
          

          Please use PHP coding guidelines
          http://zabbix.org/wiki/PHP_coding_guidelines

          Krists Krigers RESOLVED in r45686.

          Oleg Egorov Coding style problem still exist.

          Use something like:

          if ((isset($step['httpstepid']) && isset($step['name']) && $step['name'] === '')
          || (!isset($step['httpstepid']) && (!isset($step['name']) || $step['name'] === ''))) {
          self::exception(ZBX_API_ERROR_PARAMETERS, _('Web scenario step name can not be empty.')); 
          

          Please remove isValidStepName function.

          And more about coding styles

          if($stepName...
          

          After IF, we use space.

          if ($stepName...
          

          REOPEN

          Krists Krigers RESOLVED in r45724.

          Oleg Egorov

          Other problem

          Coding style

          if ((isset($step['httpstepid']) && isset($step['name']) && $step['name'] === '')
          	|| (!isset($step['httpstepid']) && (!isset($step['name']) || $step['name'] === ''))
          ) {
          

          Is incorrect.

          Use

          if ((isset($step['httpstepid']) && isset($step['name']) && $step['name'] === '')
          		|| (!isset($step['httpstepid']) && (!isset($step['name']) || $step['name'] === ''))) {
          

          REOPEN

          Krists Krigers RESOLVED in r45740.

          Oleg Egorov Coding style:

          			if ((isset($step['httpstepid']) && array_key_exists('url', $step) && $step['url'] == '')
          						|| (!isset($step['httpstepid']) && (!array_key_exists('url', $step) || $step['url'] == ''))) {
          

          You use 3 tabs, but should be 2

          REOPEN

          Krists Krigers RESOLVED in r45779.

          Oleg Egorov

          • Better use empty array, not null
          protected function checkSteps(array $httpTest, array $dbHttpTest = null) {
          
          • And as was discussed, please remove check for bool value in
          if ((isset($step['httpstepid']) && array_key_exists('name', $step) && (zbx_empty($step['name']) || ($step['name'] === false)))
          					|| (!isset($step['httpstepid']) && (!array_key_exists('name', $step) || zbx_empty($step['name']) || ($step['name'] === false)))) {
          ...
          
          if ((isset($step['httpstepid']) && array_key_exists('url', $step) && (zbx_empty($step['url']) || ($step['url'] === false)))
          					|| (!isset($step['httpstepid']) && (!array_key_exists('url', $step) || zbx_empty($step['url']) || ($step['url'] === false)))) {
          

          REOPEN

          Krists Krigers RESOLVED in r45852.

          Oleg Egorov CLOSED

          Show
          Oleg Egorov added a comment - - edited (4) if (isset($step['httpstepid']) ? isset($step['name']) && ($step['name'] == '') : !isset($step['name']) || ($step['name'] == '')) { self::exception(ZBX_API_ERROR_PARAMETERS, _('Web scenario step name can not be empty.')); } Please use PHP coding guidelines http://zabbix.org/wiki/PHP_coding_guidelines Krists Krigers RESOLVED in r45686. Oleg Egorov Coding style problem still exist. Use something like: if ((isset($step['httpstepid']) && isset($step['name']) && $step['name'] === '') || (!isset($step['httpstepid']) && (!isset($step['name']) || $step['name'] === ''))) { self::exception(ZBX_API_ERROR_PARAMETERS, _('Web scenario step name can not be empty.')); Please remove isValidStepName function. And more about coding styles if ($stepName... After IF, we use space. if ($stepName... REOPEN Krists Krigers RESOLVED in r45724. Oleg Egorov Other problem Coding style if ((isset($step['httpstepid']) && isset($step['name']) && $step['name'] === '') || (!isset($step['httpstepid']) && (!isset($step['name']) || $step['name'] === '')) ) { Is incorrect. Use if ((isset($step['httpstepid']) && isset($step['name']) && $step['name'] === '') || (!isset($step['httpstepid']) && (!isset($step['name']) || $step['name'] === ''))) { REOPEN Krists Krigers RESOLVED in r45740. Oleg Egorov Coding style: if ((isset($step['httpstepid']) && array_key_exists('url', $step) && $step['url'] == '') || (!isset($step['httpstepid']) && (!array_key_exists('url', $step) || $step['url'] == ''))) { You use 3 tabs, but should be 2 REOPEN Krists Krigers RESOLVED in r45779. Oleg Egorov Better use empty array, not null protected function checkSteps(array $httpTest, array $dbHttpTest = null ) { And as was discussed, please remove check for bool value in if ((isset($step['httpstepid']) && array_key_exists('name', $step) && (zbx_empty($step['name']) || ($step['name'] === false ))) || (!isset($step['httpstepid']) && (!array_key_exists('name', $step) || zbx_empty($step['name']) || ($step['name'] === false )))) { ... if ((isset($step['httpstepid']) && array_key_exists('url', $step) && (zbx_empty($step['url']) || ($step['url'] === false ))) || (!isset($step['httpstepid']) && (!array_key_exists('url', $step) || zbx_empty($step['url']) || ($step['url'] === false )))) { REOPEN Krists Krigers RESOLVED in r45852. Oleg Egorov CLOSED
          Hide
          Ivo Kurzemnieks added a comment - - edited

          (5) Related:
          update method passing empty array, results in success and frontend displays "Undefined index: stepscnt [httpconf.php:556 → CView->render() → include() in C:\Development\zabbix\frontends\php\include\views\configuration.httpconf.list.php:81]

          Krists Krigers RESOLVED in r45724. Added check and unset of empty "steps" array in update data normalization.

          Oleg Egorov

          API REQUEST

          {
              "httptestid": 18,
              "name": "Homepage check25",
              "hostid": "10105",
              "steps": [
                  {
                      "httpstepid": 9999,
                      "name": 0
                  }
              ]
          }
          

          httpstepid = 9999 - not exist

          Undefined index: stepscnt [httpconf.php:513 → CView->render() → include() in C:\xampp\htdocs\2.2\frontends\php\include\views\configuration.httpconf.list.php:83]

          REOPEN

          Krists Krigers RESOLVED in r45737.

          Oleg Egorov

          In update function

          		$dbCursor = DBselect('SELECT hs.httpstepid,hs.httptestid,hs.name'.
          				' FROM httpstep hs'.
          				' WHERE '.dbConditionInt('hs.httptestid', array_keys($dbHttpTests)));
          		while ($dbHttpStep = DBfetch($dbCursor)) {
          			$dbHttpTests[$dbHttpStep['httptestid']]['steps'][$dbHttpStep['httpstepid']] = $dbHttpStep;
          		}
          

          There we get all httpstepid

          Remove unnecessary SQL query

          REOPEN

          Krists Krigers RESOLVED in r45779.

          Oleg Egorov CLOSED

          Show
          Ivo Kurzemnieks added a comment - - edited (5) Related: update method passing empty array, results in success and frontend displays "Undefined index: stepscnt [httpconf.php:556 → CView->render() → include() in C:\Development\zabbix\frontends\php\include\views\configuration.httpconf.list.php:81] Krists Krigers RESOLVED in r45724. Added check and unset of empty "steps" array in update data normalization. Oleg Egorov API REQUEST { "httptestid" : 18, "name" : "Homepage check25" , "hostid" : "10105" , "steps" : [ { "httpstepid" : 9999, "name" : 0 } ] } httpstepid = 9999 - not exist Undefined index: stepscnt [httpconf.php:513 → CView->render() → include() in C:\xampp\htdocs\2.2\frontends\php\include\views\configuration.httpconf.list.php:83] REOPEN Krists Krigers RESOLVED in r45737. Oleg Egorov In update function $dbCursor = DBselect('SELECT hs.httpstepid,hs.httptestid,hs.name'. ' FROM httpstep hs'. ' WHERE '.dbConditionInt('hs.httptestid', array_keys($dbHttpTests))); while ($dbHttpStep = DBfetch($dbCursor)) { $dbHttpTests[$dbHttpStep['httptestid']]['steps'][$dbHttpStep['httpstepid']] = $dbHttpStep; } There we get all httpstepid Remove unnecessary SQL query REOPEN Krists Krigers RESOLVED in r45779. Oleg Egorov CLOSED
          Hide
          Oleg Egorov added a comment - - edited

          (6) Same problem exist for URL's

          empty "url" => SQL statement execution has failed \"INSERT INTO httptest (name,hostid,variables,httptestid) VALUES ('Homepage check test','10084','','100100000000055')\".
          

          Moved from ZBX-8194

          Krists Krigers RESOLVED in r45737. Added step URL validation.

          Oleg Egorov CLOSED

          Show
          Oleg Egorov added a comment - - edited (6) Same problem exist for URL's empty "url" => SQL statement execution has failed \ "INSERT INTO httptest (name,hostid,variables,httptestid) VALUES ('Homepage check test','10084','','100100000000055')\" . Moved from ZBX-8194 Krists Krigers RESOLVED in r45737. Added step URL validation. Oleg Egorov CLOSED
          Hide
          Oleg Egorov added a comment - - edited

          (7) Request:
          httptest.create with httpstepid and without name

          {
              "hostid": 10107,
              "name": "E7",
              "steps": [
                  {
                      "httpstepid": 91,
                      "url": "1",
                      "status_codes": 200,
                      "no": 1
                  }
              ]
          }
          

          Response:
          Item with key "web.test.in[E7,,bps]" already exists.

          MOVED TO ZBX-8268

          CLOSED

          Show
          Oleg Egorov added a comment - - edited (7) Request: httptest.create with httpstepid and without name { "hostid" : 10107, "name" : "E7" , "steps" : [ { "httpstepid" : 91, "url" : "1" , "status_codes" : 200, "no" : 1 } ] } Response: Item with key "web.test.in [E7,,bps] " already exists. MOVED TO ZBX-8268 CLOSED
          Hide
          Oleg Egorov added a comment - - edited

          (8) httptest.create

          {
              "hostid": 10107,
              "name": "E9",
              "steps": 1
          }
          

          OK result and...

          Undefined index: stepscnt [httpconf.php:522 → CView->render() → include() in C:\xampp\htdocs\ZBX-8195\frontends\php\include\views\configuration.httpconf.list.php:83]

          Krists Krigers RESOLVED in r45967.

          Oleg Egorov CLOSED

          Show
          Oleg Egorov added a comment - - edited (8) httptest.create { "hostid" : 10107, "name" : "E9" , "steps" : 1 } OK result and... Undefined index: stepscnt [httpconf.php:522 → CView->render() → include() in C:\xampp\htdocs\ZBX-8195\frontends\php\include\views\configuration.httpconf.list.php:83] Krists Krigers RESOLVED in r45967. Oleg Egorov CLOSED
          Hide
          Oleg Egorov added a comment -

          TESTED

          Show
          Oleg Egorov added a comment - TESTED
          Hide
          Krists Krigers (Inactive) added a comment - - edited

          (9) Made a branch for trunk merge svn://svn.zabbix.com/branches/dev/ZBX-8195-trunk. Committed changes in r45999.

          Oleg Egorov CLOSED

          Show
          Krists Krigers (Inactive) added a comment - - edited (9) Made a branch for trunk merge svn://svn.zabbix.com/branches/dev/ZBX-8195-trunk. Committed changes in r45999. Oleg Egorov CLOSED
          Hide
          Krists Krigers (Inactive) added a comment -

          Fixed validation for 2.2.4rc1 in r45982 and for 2.3.1 (trunk) in r46028.

          Show
          Krists Krigers (Inactive) added a comment - Fixed validation for 2.2.4rc1 in r45982 and for 2.3.1 (trunk) in r46028.
          Hide
          Pavels Jelisejevs (Inactive) added a comment - - edited

          (10) This needs to be noted in the API changelog.

          Krists Krigers RESOLVED.

          Pavels Jelisejevs If a fix affects multiple methods we write the entry without mentioning the methods. Otherwise OK, CLOSED.

          Show
          Pavels Jelisejevs (Inactive) added a comment - - edited (10) This needs to be noted in the API changelog. Krists Krigers RESOLVED. Pavels Jelisejevs If a fix affects multiple methods we write the entry without mentioning the methods. Otherwise OK, CLOSED.

            People

            • Assignee:
              Oleg Egorov
              Reporter:
              Ivo Kurzemnieks
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: