[ZBX-5900] incorrect message when validating maintenance periods Created: 2012 Nov 27 Updated: 2017 May 30 Resolved: 2013 Jan 23 |
|
Status: | Closed |
Project: | ZABBIX BUGS AND ISSUES |
Component/s: | API (A), Frontend (F) |
Affects Version/s: | None |
Fix Version/s: | 2.0.4rc1, 2.0.5rc1, 2.1.0 |
Type: | Incident report | Priority: | Minor |
Reporter: | richlv | Assignee: | Unassigned |
Resolution: | Fixed | Votes: | 0 |
Labels: | maintenance, regression | ||
Remaining Estimate: | Not Specified | ||
Time Spent: | Not Specified | ||
Original Estimate: | Not Specified | ||
Environment: |
trunk rev 31744. |
Description |
try to enter year 2039 in maintenance "Active till" area. it fails with message : the previous (and correct) message was : note that "Active till" fields are also cleared upon such error - they should stay the same user entered instead. broken by : ------------------------------------------------------------------------ A......... |
Comments |
Comment by Oleg Egorov (Inactive) [ 2012 Nov 28 ] |
FIXED IN svn://svn.zabbix.com/branches/dev/ZBX-5900 r31762 |
Comment by Toms (Inactive) [ 2012 Nov 28 ] |
(1) note that "Active till" fields are also cleared upon such error - they should stay the same user entered instead oleg.egorov RESOLVED IN r31844 tomtom CLOSED |
Comment by Toms (Inactive) [ 2012 Nov 28 ] |
(2) With year 2100 and above, frontend silently discards year change. This happens with other malformed field values as well. oleg.egorov RESOLVED IN r31844 tomtom CLOSED |
Comment by Toms (Inactive) [ 2012 Nov 28 ] |
(3)return ($time <= 2147464800 && $time != NULL); will not work with value 0, what theoretically could happen in future; I suggest using something more descriptive: return (is_numeric($time) && $time <= 2147464800); oleg.egorov I agree! RESOLVED IN r31772 tomtom CLOSED |
Comment by Toms (Inactive) [ 2012 Dec 05 ] |
(4) oleg.egorov RESOLVED r32049 |
Comment by Toms (Inactive) [ 2012 Dec 05 ] |
(5) if ((this.year+1) > 2038) { for whole numbers should be written as: if (this.year >= 2038) { oleg.egorov RESOLVED r32049 |
Comment by Toms (Inactive) [ 2012 Dec 05 ] |
(6) When entered invalid month (13) message "Active till" must be between 1970.01.01 and 2038.01.18. is shown. Should be something like: Invalid date ... Happens for other fields as well. oleg.egorov RESOLVED IN r32056 |
Comment by Toms (Inactive) [ 2012 Dec 05 ] |
(7) oleg.egorov RESOLVED IN r32056 tomtom Not fixed, error still exists oleg.egorov RESOLVED tomtom CLOSED |
Comment by Toms (Inactive) [ 2012 Dec 05 ] |
(8) Date part should be edited with single character value! oleg.egorov I don't understand your idea... tomtom as we discussed oleg.egorov RESOLVED tomtom CLOSED |
Comment by Toms (Inactive) [ 2012 Dec 05 ] |
(9) commented part seems like never executed in forms.inc.php if (isset($_REQUEST['add_timeperiod'])) { // $new_timeperiod_year = get_request('new_timeperiod_year'); // $new_timeperiod_month = get_request('new_timeperiod_month'); // $new_timeperiod_day = get_request('new_timeperiod_day'); // $new_timeperiod_hours = get_request('new_timeperiod_hour'); // $new_timeperiod_minutes = get_request('new_timeperiod_minute'); } elseif ($start_date > 0) { $new_timeperiod_year = date('Y', $start_date ); $new_timeperiod_month = date('m', $start_date ); $new_timeperiod_day = date('d', $start_date ); $new_timeperiod_hours = date('H', $start_date ); $new_timeperiod_minutes = date('i', $start_date ); } else { // $new_timeperiod_year = ''; // $new_timeperiod_month = ''; // $new_timeperiod_day = ''; // $new_timeperiod_hours = ''; // $new_timeperiod_minutes = ''; } camelCase should be used in this code part as well oleg.egorov Code executed in maintenance->periods. OR Undefined variable: new_timeperiod_day [include\forms.inc.php:1648] Eduards CLOSED tomtom REOPENED. camelCase should be used. oleg.egorov RESOLVED tomtom CLOSED |
Comment by Toms (Inactive) [ 2012 Dec 05 ] |
(10)
oleg.egorov RESOLVED IN r32056 tomtom REOPENED. Validate functions should return TRUE if date and time are valid!!! Review changes in r32163 oleg.egorov Changes - OK, but why need return true?
return ($year < 1970 || $year > 2038 || ($year == 2038 && (($month > 1) || ($month == 1 && $day > 18))));
If this expression is valid, function return "false", else "true" RESOLVED tomtom CLOSED. Valid is what we need, so it is good as well true is good, so, if valid, function should return true |
Comment by Toms (Inactive) [ 2012 Dec 05 ] |
(11) Services.php if ($_REQUEST['new_service_time']['type'] == SERVICE_TIME_TYPE_ONETIME_DOWNTIME) { if (validateZBXTime($_REQUEST['downtime_since_year'], $_REQUEST['downtime_since_month'], $_REQUEST['downtime_since_day'], $_REQUEST['downtime_since_hour'], $_REQUEST['downtime_since_minute'])) { throw new APIException(ZBX_API_ERROR_PARAMETERS, _s('Service time "%s" must be between 1970.01.01 and 2038.01.18.', _('From'))); } if (validateZBXTime($_REQUEST['downtime_till_year'], $_REQUEST['downtime_till_month'], $_REQUEST['downtime_till_day'], $_REQUEST['downtime_till_hour'], $_REQUEST['downtime_till_minute'])) { throw new APIException(ZBX_API_ERROR_PARAMETERS, _s('Service time "%s" must be between 1970.01.01 and 2038.01.18.', _('Till'))); } } It is not OK to rise API exceptions outside API. As well this part of functionality could be outside "try" block and implemented the same as elsewhere. oleg.egorov RESOLVED IN r32056 tomtom CLOSED |
Comment by Toms (Inactive) [ 2012 Dec 17 ] |
(12) we get error: Though "Active since" is smaller than "Active till" oleg.egorov RESOLVED tomtom CLOSED |
Comment by Toms (Inactive) [ 2012 Dec 20 ] |
(13) If entered date is 1/1/1970 00:00, then value is not stored. oleg.egorov RESOLVED in r32279 tomtom CLOSED |
Comment by Toms (Inactive) [ 2012 Dec 20 ] |
(14) in maintenance.php
}
else {
$activeSince = time();
}
never executes, but if it would happen time() should be rounded down to minutes oleg.egorov RESOLVED in r32275 tomtom CLOSED |
Comment by Toms (Inactive) [ 2012 Dec 20 ] |
Review my changes in r32266 oleg.egorov CLOSED |
Comment by Toms (Inactive) [ 2012 Dec 21 ] |
TESTED |
Comment by Oleg Egorov (Inactive) [ 2012 Dec 21 ] |
FIXED IN 2.0.5rc1 r32283 |
Comment by Oleg Egorov (Inactive) [ 2012 Dec 28 ] |
FIXED FOR TRUNK IN svn://svn.zabbix.com/branches/dev/ZBX-5900-trunk r32348 |
Comment by Toms (Inactive) [ 2012 Dec 28 ] |
(15) For trunk. $onClick unused in configuration.services.edit.php, but in general function createDateSelector() should be used as before, not replaced by inline code with modifications.
oleg.egorov FIXED IN r32381 tomtom CLOSED. Review my changes r32466 |
Comment by Pavels Jelisejevs (Inactive) [ 2013 Jan 02 ] |
(16) Please note the fix in the API changelog. oleg.egorov CLOSED |
Comment by Toms (Inactive) [ 2013 Jan 04 ] |
TESTED for trunk |
Comment by Oleg Egorov (Inactive) [ 2013 Jan 07 ] |
FIXED IN 2.1.0(trunk) r32489 |
Comment by richlv [ 2013 Jan 07 ] |
(17) the only entry in https://www.zabbix.com/documentation/2.0/manual/appendix/api/changes_2.0 is for 2.0.4, but apparently something changed for 2.0.5 as well. shouldn't it be mentioned in 2.0.5 section, too ? oleg.egorov RESOLVED <richlv> a bit vague, but better than before - CLOSED |
Comment by richlv [ 2013 Jan 15 ] |
see (17), reopening |
Comment by richlv [ 2013 Jan 15 ] |
this resulted in a regression : |
Comment by richlv [ 2013 Jan 18 ] |
see (15) before closing this issue - apparently those changes haven't been reviewed oleg.egorov Was reviewed CLOSED |
Comment by Anton Samets [ 2013 Feb 21 ] |
Guys, we have a problem with this issue. We are create zabbix maintenance by next (1 Jan 2010 - 1 Jan 2030): active_since = "1262304000" active_till = "1893456000" start_date = "1262304000" period = "621152000" z.maintenance.create({ #"groupids": [], "hostids": [hostid[0]['hostid']], "name": mntname, "maintenance_type": "0", "active_since": active_since, "active_till": active_till, "timeperiods": [ { "timeperiod_type": "0", "start_date": start_date, "period": period }], "description": "for bwc" }) Then we trying to put host in maintenance state: resupdate = z.maintenance.update({ "maintenanceid": maintenanceid, "name": mntname, "groupids": [], "hostids": bwcmnthosts, "timeperiods": [ { "timeperiod_type": "0", "start_date": start_date, "period": period }], "description": "for BWC" }) And got the next: File "/home/bwtools/opt/lib/python2.6/zabbix_api.py", line 307, in do_request raise ZabbixAPIException(msg, jobj['error']['code']) zabbix_api.ZabbixAPIException: (u'Error -32602: Invalid params., "Active since" must be between 1970.01.01 and 2038.01.18. while sending {"params": {"description": "for BWC", "timeperiods": [{"timeperiod_type": "0", "start_date": "1262304000", "period": "621152000"}], "maintenanceid": "17", "groupids": [], "hostids": ["14024", "14024", "13471"], "name": "BWC"}, "jsonrpc": "2.0", "method": "maintenance.update", "auth": "d32a3273513d623702c52684c8b9091a", "id": 7}', -32602) Can you help us? Because we didn't see any error on our side. |
Comment by Anton Samets [ 2013 Feb 21 ] |
This method was working in 2.0.4 |
Comment by Anton Samets [ 2013 Feb 21 ] |
we found that if you are trying to change time range via Frontend, you get error about incorrect data for days (something like " you entered 4 digits, but allowed not more that 3"). We are playing with date, made period not so high (from 01.01.2013 to 01.09.2015) and still we error on updating maintenance via zabbix API. |
Comment by Anton Samets [ 2013 Feb 21 ] |
So, the problem is in api/classes/CMaintenance.php: // validate maintenance active since //if (!validateUnixTime($maintenance['active_since'])) { // self::exception(ZBX_API_ERROR_PARAMETERS, _s('"%s" must be between 1970.01.01 and 2038.01.18.', _('Active since'))); //} // validate maintenance active till //if (!validateUnixTime($maintenance['active_till'])) { // self::exception(ZBX_API_ERROR_PARAMETERS, _s('"%s" must be between 1970.01.01 and 2038.01.18.', _('Active till'))); //} We think that something was broken in svn/2.0/frontends/php/include/validate.inc.php in function validateUnixTime. |
Comment by Anton Samets [ 2013 Feb 21 ] |
We have been trying to use: active_since = "1262304000" active_till = "1893456000" start_date = "1262304000" period = "621152000" without "", but it's still not work. |