[ZBX-8195] httptest.update can create scenario steps without name Created: 2014 May 10  Updated: 2017 May 30  Resolved: 2014 Jun 06

Status: Closed
Project: ZABBIX BUGS AND ISSUES
Component/s: API (A)
Affects Version/s: 2.2.3
Fix Version/s: 2.2.4rc1, 2.3.1

Type: Incident report Priority: Minor
Reporter: Ivo Kurzemnieks Assignee: Oleg Egorov (Inactive)
Resolution: Fixed Votes: 0
Labels: api, httpstep, httptest, validation
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Duplicate
duplicates ZBX-3783 Proper API validation Reopened

 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.

 Comments   
Comment by Ivo Kurzemnieks [ 2014 May 10 ]

(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

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

oleg.egorov CLOSED

Comment by Krists Krigers (Inactive) [ 2014 May 19 ]

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

Comment by Oleg Egorov (Inactive) [ 2014 May 20 ]

(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

kristsk RESOLVED.

oleg.egorov

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

Better change to no permission/not exist message.

REOPEN

kristsk RESOLVED in r45779.

oleg.egorov CLOSED

Comment by Oleg Egorov (Inactive) [ 2014 May 20 ]

(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

kristsk RESOLVED in r45686.

oleg.egorov

Possible execute with name = null

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

REOPEN

kristsk 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

kristsk RESOLVED in r45779.

oleg.egorov CLOSED

Comment by Oleg Egorov (Inactive) [ 2014 May 20 ]

(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

kristsk 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

kristsk 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

kristsk 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

kristsk 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

kristsk RESOLVED in r45852.

oleg.egorov CLOSED

Comment by Ivo Kurzemnieks [ 2014 May 20 ]

(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]

kristsk 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

kristsk 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

kristsk RESOLVED in r45779.

oleg.egorov CLOSED

Comment by Oleg Egorov (Inactive) [ 2014 May 22 ]

(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

kristsk RESOLVED in r45737. Added step URL validation.

oleg.egorov CLOSED

Comment by Oleg Egorov (Inactive) [ 2014 May 28 ]

(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

Comment by Oleg Egorov (Inactive) [ 2014 May 28 ]

(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]

kristsk RESOLVED in r45967.

oleg.egorov CLOSED

Comment by Oleg Egorov (Inactive) [ 2014 May 29 ]

TESTED

Comment by Krists Krigers (Inactive) [ 2014 May 30 ]

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

oleg.egorov CLOSED

Comment by Krists Krigers (Inactive) [ 2014 May 30 ]

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

Comment by Pavels Jelisejevs (Inactive) [ 2014 Jun 05 ]

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

kristsk RESOLVED.

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

Generated at Thu Apr 25 08:01:02 EEST 2024 using Jira 9.12.4#9120004-sha1:625303b708afdb767e17cb2838290c41888e9ff0.