|
| 1 | +<?php |
| 2 | +/* |
| 3 | + +-------------------------------------------------------------------------+ |
| 4 | + | Copyright (C) 2004-2026 The Cacti Group | |
| 5 | + | | |
| 6 | + | This program is free software; you can redistribute it and/or | |
| 7 | + | modify it under the terms of the GNU General Public License | |
| 8 | + | as published by the Free Software Foundation; either version 2 | |
| 9 | + | of the License, or (at your option) any later version. | |
| 10 | + +-------------------------------------------------------------------------+ |
| 11 | + | Cacti: The Complete RRDtool-based Graphing Solution | |
| 12 | + +-------------------------------------------------------------------------+ |
| 13 | +*/ |
| 14 | + |
| 15 | +/* |
| 16 | + * Source-level guards for the data_input_data column-expansion bugs in |
| 17 | + * change_data_template() (lib/template.php) and api_data_source_duplicate() |
| 18 | + * (lib/api_data_source.php). |
| 19 | + * |
| 20 | + * Background. The data_input_data table on develop carries five identity |
| 21 | + * columns (data_input_field_id, data_template_data_id, data_template_id, |
| 22 | + * local_data_id, host_id) plus the value pair (t_value, value). Two |
| 23 | + * write paths copy rows from the template (parent) row into the child: |
| 24 | + * |
| 25 | + * - change_data_template() in lib/template.php |
| 26 | + * - api_data_source_duplicate() in lib/api_data_source.php |
| 27 | + * |
| 28 | + * Two regressions were live in the develop branch: |
| 29 | + * |
| 30 | + * (1) change_data_template() copied $item['data_template_id'], |
| 31 | + * $item['local_data_id'], $item['host_id'] from the parent row |
| 32 | + * verbatim. For template rows those are 0, so child data_input_data |
| 33 | + * rows ended up with local_data_id = 0 and host_id = 0, orphaned |
| 34 | + * from the new data source. The fix reads host_id from |
| 35 | + * data_local for the new $local_data_id and stamps the child row |
| 36 | + * with the function's own ($data_template_id, $local_data_id, |
| 37 | + * $host_id) instead of the parent's. |
| 38 | + * |
| 39 | + * (2) api_data_source_duplicate() expanded the INSERT column list to |
| 40 | + * seven (data_input_field_id, data_template_data_id, |
| 41 | + * data_template_id, local_data_id, host_id, t_value, value) but |
| 42 | + * left the placeholder list at four (?, ?, ?, ?), while still |
| 43 | + * binding seven values. db_execute_prepared() rejects the |
| 44 | + * mismatch, so duplicating a data source or template silently |
| 45 | + * produced no data_input_data rows. |
| 46 | + * |
| 47 | + * Both bugs cascade into the same downstream symptom TheWitness reported |
| 48 | + * for 1.2.31: data sources created from a template (or duplicated from |
| 49 | + * an existing one) end up without populated data_input_data, which |
| 50 | + * breaks update_poller_cache() and leaves poller_item empty. |
| 51 | + * |
| 52 | + * The 1.2.x branch is unaffected. Its data_input_data schema only |
| 53 | + * carries the four columns (data_input_field_id, data_template_data_id, |
| 54 | + * t_value, value) and both call sites already match. |
| 55 | + */ |
| 56 | + |
| 57 | +$templateSource = file_get_contents(__DIR__ . '/../../lib/template.php'); |
| 58 | +$apiDataSourceSource = file_get_contents(__DIR__ . '/../../lib/api_data_source.php'); |
| 59 | + |
| 60 | +/** |
| 61 | + * Extract the body of one PHP function definition out of a source blob. |
| 62 | + * Caller supplies the function name; returns the matched body or '' if |
| 63 | + * the definition is not found. |
| 64 | + */ |
| 65 | +function _extract_function_body(string $source, string $name): string { |
| 66 | + $pattern = '/^function\s+' . preg_quote($name, '/') . '\b[^{]*\{.*?^\}/sm'; |
| 67 | + if (!preg_match($pattern, $source, $m)) { |
| 68 | + return ''; |
| 69 | + } |
| 70 | + return $m[0]; |
| 71 | +} |
| 72 | + |
| 73 | +test('change_data_template stamps child data_input_data rows with the new identity tuple', function () use ($templateSource) { |
| 74 | + $body = _extract_function_body($templateSource, 'change_data_template'); |
| 75 | + expect($body)->not->toBe('', 'change_data_template() must be defined in lib/template.php'); |
| 76 | + |
| 77 | + /* The function must look up the child host_id from data_local using |
| 78 | + * the new $local_data_id. The pre-fix code did not do this and |
| 79 | + * relied on $item['host_id'] from the parent row, which is 0 on a |
| 80 | + * template. */ |
| 81 | + expect($body) |
| 82 | + ->toContain('SELECT host_id', 'must look up host_id from data_local') |
| 83 | + ->toContain('FROM data_local', 'must look up host_id from data_local') |
| 84 | + ->toContain('[$local_data_id]', 'lookup must key on the new local_data_id'); |
| 85 | + |
| 86 | + /* The REPLACE INTO must use the function's own identity tuple |
| 87 | + * ($data_template_id, $local_data_id, $host_id) for the three |
| 88 | + * identity columns that previously came from the parent row. */ |
| 89 | + $replacePos = strpos($body, 'REPLACE INTO data_input_data'); |
| 90 | + expect($replacePos)->not->toBeFalse('REPLACE INTO data_input_data must remain present'); |
| 91 | + |
| 92 | + $insertSlice = substr($body, $replacePos, 800); |
| 93 | + |
| 94 | + /* Column count and placeholder count must match. */ |
| 95 | + expect(substr_count($insertSlice, '?')) |
| 96 | + ->toBe(7, 'data_input_data REPLACE must bind seven placeholders'); |
| 97 | + |
| 98 | + /* The seven bound values must include the function's own variables, |
| 99 | + * not just $item[...] readbacks of the parent row. */ |
| 100 | + expect($insertSlice) |
| 101 | + ->toContain('$data_template_id,', 'bind the function-scope $data_template_id') |
| 102 | + ->toContain('$local_data_id,', 'bind the function-scope $local_data_id') |
| 103 | + ->toContain('$host_id,', 'bind the freshly-fetched $host_id'); |
| 104 | + |
| 105 | + /* The pre-fix bind list pulled these from $item; that pattern must |
| 106 | + * not survive in the fixed code for the identity columns. */ |
| 107 | + expect(strpos($insertSlice, "\$item['local_data_id']"))->toBeFalse( |
| 108 | + 'must not bind $item[local_data_id] for the child row (template default is 0)' |
| 109 | + ); |
| 110 | + expect(strpos($insertSlice, "\$item['host_id']"))->toBeFalse( |
| 111 | + 'must not bind $item[host_id] for the child row (template default is 0)' |
| 112 | + ); |
| 113 | +}); |
| 114 | + |
| 115 | +test('api_data_source_duplicate INSERT column count matches placeholder count', function () use ($apiDataSourceSource) { |
| 116 | + $body = _extract_function_body($apiDataSourceSource, 'api_data_source_duplicate'); |
| 117 | + expect($body)->not->toBe('', 'api_data_source_duplicate() must be defined in lib/api_data_source.php'); |
| 118 | + |
| 119 | + $insertPos = strpos($body, 'INSERT IGNORE INTO data_input_data'); |
| 120 | + expect($insertPos)->not->toBeFalse('INSERT IGNORE INTO data_input_data must remain present'); |
| 121 | + |
| 122 | + $insertSlice = substr($body, $insertPos, 800); |
| 123 | + |
| 124 | + /* Placeholder count must match the seven-column form the caller |
| 125 | + * already binds. The pre-fix code had four placeholders against |
| 126 | + * seven bound values; db_execute_prepared() rejects that. */ |
| 127 | + expect(substr_count($insertSlice, '?')) |
| 128 | + ->toBe(7, 'INSERT IGNORE must bind seven placeholders to match the seven-column list'); |
| 129 | + |
| 130 | + /* All seven columns must be named in the INSERT list. */ |
| 131 | + foreach (['data_input_field_id', 'data_template_data_id', 'data_template_id', 'local_data_id', 'host_id', 't_value', 'value'] as $col) { |
| 132 | + expect($insertSlice)->toContain($col, "INSERT must list column $col"); |
| 133 | + } |
| 134 | +}); |
| 135 | + |
| 136 | +test('1.2.x schema regression check: develop INSERT must not silently drop columns', function () use ($apiDataSourceSource, $templateSource) { |
| 137 | + /* If anyone re-narrows the develop INSERT/REPLACE column lists back |
| 138 | + * to the 1.2.x four-column form (data_input_field_id, |
| 139 | + * data_template_data_id, t_value, value) the column / placeholder |
| 140 | + * count check above will still pass on length but the cross-source |
| 141 | + * grammar drifts. Pin the seven-column shape explicitly so a |
| 142 | + * narrowing change has to delete the columns from the SQL string |
| 143 | + * rather than just re-shrink the bind array. */ |
| 144 | + foreach ([$templateSource, $apiDataSourceSource] as $source) { |
| 145 | + $pos = strpos($source, '(data_input_field_id, data_template_data_id, data_template_id, local_data_id, host_id, t_value, value)'); |
| 146 | + expect($pos)->not->toBeFalse('seven-column data_input_data write must remain present'); |
| 147 | + } |
| 148 | +}); |
0 commit comments