You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Add Cisco NBAR application reports with reusable history summaries - #295
Depends on #294 for the bundled DNS library’s PHP 8.4 compatibility. This branch includes that prerequisite commit.
Add a Cisco Apps (NBAR) tab with Top applications, Conversations and Connections, IPv4/IPv6 and endpoint filters, observation dates, per-capture feed status, an application-volume chart and account-specific capture/network/display options. A setup dialog documents a Cisco 1100 Series NetFlow v9 example; the integration does not configure routers.
Hourly and daily summaries are shared across selections. Partial boundaries and invalidated revisions fall back to raw records. Connection pages and exact counts are requested separately with bounded account-session caches. Candidate pages and grouped fallback pages use the same deterministic tie order before and after pagination. Exact-count sources guard their checkpoint coverage and revisions at execution time, including checkpoint extensions and late exports. Retention is configurable up to 30 days and constrains report timespans.
NBAR timespan requests bypass legacy Flow Reports session storage for every preset. Conversations exclude multicast endpoints in both IP families. Conversation report information is translated through gettext, with a formatted retention placeholder and escaped translated output. Feed-query errors are distinguished from empty captures. Retention cleanup drains repeated batches across all retention levels within a shared ten-second budget, before summary maintenance.
Zero-length template fields are rejected and invalidate the affected cached template until a valid refresh arrives. Template state is bounded to 256 templates / 8,192 field definitions globally and 64 templates / 2,048 fields per exporter across all source IDs, with LRU eviction and 30-minute inactivity expiry. Numeric data widths are checked before conversion (counters 8 bytes, protocol 1, ports 2, uptime timestamp 4); options scopes remain separate. Chart grouping uses the complete learned-name/fallback-label expression, keeping unnamed application IDs in separate slices.
Actual exporter addresses are preserved through collection and summary keys; listener allowlists are not treated as exporter identities. Legacy counters are retained, but previously merged multi-exporter rows cannot be attributed retrospectively. Query failures render explicit errors. Observation checkpoint seeds are retryable and the DNS budget regression runs in a fresh process.
NBAR maintenance launches outside the poller, with one registered supervisor and a direct PHP child bounded to 90 seconds using PHP’s argv-array process API, independent of the Cacti timeout-helper signature. MariaDB hourly builds also establish a verified 20-second statement deadline and restore the previous value. MySQL hourly builds run only in the supervised child.
Validation:
Latest suite: 355 tests / 2,892 assertions pass on PHP 8.3 with the minimum supported Cacti 1.2.29 core. Changed-line coverage passes at 100% (1,585/1,585 measured lines); manifest and translation gates pass.
Isolated MariaDB 11.8.9: migration of the prior schema plus 96 raw/summary, exact-count and page-two comparisons pass against 2,628 retained fixture records.
A separate disposable database passes fresh/repeat schema creation, real packet collection from four exporters across three captures, application names with wildcard/CIDR/comma-list allowlists, hourly/daily aggregation, and deferred counts after checkpoint extension and late export.
Released-core compatibility: actual startup and a one-second timeout pass with the official Cacti 1.2.29 and current 1.2.x poller helpers loaded. The NBAR supervisor no longer calls their differing exec_with_timeout APIs. Its own tests cover quoted paths, exit codes, startup failures, bounded stdout/stderr draining and actual child termination. Previous PHP 8.2/8.4 runs covered the preceding revision; those temporary runtimes were removed, so this latest pass was PHP 8.3.
Recheck: generated fallback SQL and the PHP comparator match an independent ordering for 2,048 tied connections across four exporters and 21 pages, including missing names and the next-page sentinel (SQLite compatibility executor). The actual Cacti process helper terminates a five-second PHP child at a one-second test deadline.
Earlier compatibility work covered MariaDB 10.11 and Linux PHP 8.2–8.4, retention at 7/30 days, Safari navigation/resizing and seven themes. Those earlier runs preceded these review corrections. This is not a full accessibility audit or a production speedup claim.
Limitations: application decoding currently supports NetFlow v9, not IPFIX application records. MariaDB supplies verified DML statement deadlines for daily materialization and observation backfills. On MySQL 8 these workers defer safely, while reports continue through hourly/raw sources and exact counts use the SELECT execution-time hint; the MySQL path is unit-tested, not validated against a live MySQL server in this revision. The latest corrections have local regression coverage (including real Cacti timespan handling and generated multicast-predicate execution with a SQLite compatibility executor); their live MariaDB rerun is pending because the test terminal is unavailable. Upstream GitHub Actions still requires maintainer approval before the full Ubuntu/poller workflow can run.
Latest Copilot summary correction
Commit ef317f6 preserves header=false when restoring the last saved FlowView section, so Cacti fragment requests retain header suppression across the second redirect. Request regressions cover both saved sections, simultaneous headercontent=true, and ordinary full-page navigation. The latest PHP 8.3/Cacti 1.2.29 harness passes 355 tests / 2,892 assertions, the configured changed-line coverage gate passes at 100% (1,585/1,585 included lines), and manifest/gettext/whitespace checks pass. This does not replace pending live integration and full upstream CI.
Handle all NBAR timespan requests before legacy session storage
flowview.php:100
Changing an ordinary Cisco Apps timespan sends report=nbar through flowview_request_vars(), which stores it and the preset in the legacy sess_fv_<query> filter session. This overwrites Flow Reports state and can restore an invalid legacy report identifier on a later request. Handle every NBAR gettimespan request before that session-storage path, not just preset 1000; flowview_gettimespan() already validates the numeric preset itself.
Drain expired rows within the maintenance cleanup deadline
nbar_common.php:78
Each maintenance run deletes at most 50,000 expired raw rows, even if time remains. With 60,000 distinct rows expiring per five-minute poll, the backlog grows by at least 10,000 rows per run indefinitely, so retention does not bound storage growth. Hourly and daily tables likewise receive only one fixed-size batch. Drain additional batches within a cleanup deadline, or schedule sufficient cleanup throughput, while preserving the supervised worker's time bound.
Exclude IPv4 and IPv6 multicast destinations
nbar_conversations.php:46
Requiring exactly one internal endpoint still admits multicast destinations. For example, 192.0.2.10 → 239.255.255.250 with 192.0.2.0/24 configured is counted as an external upload; IPv6 ff00::/8 traffic has the same problem. This contradicts the report's stated multicast exclusion. Exclude IPv4 224.0.0.0/4 and IPv6 ff00::/8 endpoints before grouping, and add regression cases for both families.
Distinguish freshness query failures from missing NBAR data
nbar_feed_status.php:45
A failed freshness query returns false, which becomes minute 0 here. The capture then displays both “No recent NBAR” and “No retained NBAR records,” misrepresenting a database failure as missing exporter data. Handle failure separately from an empty result and display a translated status-load error for that capture; update the gettext template and cover the failure branch.
Every NBAR gettimespan request now bypasses legacy Flow Reports session storage, not only retention preset 1000. Regression cases cover ordinary, custom, invalid and retention requests while preserving the prior legacy session.
Retention cleanup repeats bounded deletion batches fairly across raw/hourly/daily tables until empty or a shared ten-second budget expires. Cleanup runs before summary preparation inside the supervised worker. Tests cover multiple full batches, empty tables, failed deletes and deadline expiry.
Conversation predicates exclude IPv4 224.0.0.0/4 and IPv6 ff00::/8 at either endpoint. Executed the generated predicates against multicast/unicast addresses in both directions (SQLite compatibility executor; not a new MariaDB run).
Failed feed freshness queries render a translated status-load error for that capture, while other capture statuses still display. They no longer render missing-data messages. Updated gettext template and regression coverage.
321 tests / 2,727 assertions pass on PHP 8.2, 8.3 and 8.4; measured changed-line coverage is 100% (1,503/1,503), and manifest/translation gates pass. The two older open threads refer to exporter joins and unsupported DML deadlines: current records/summaries carry the actual exporter address, and unsupported daily/backfill materialization defers safely while MySQL hourly aggregation runs in the supervised child. Those fixes and their prior isolated MariaDB evidence remain documented in the threads. Latest server/browser reruns remain pending; production/router configuration was not changed.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
Unresolved correctness and collector-availability issues need fixes, while schema and concurrent maintenance changes require maintainer review with pending live validation.
Group applications by the full fallback label expression
nbar_chart.php:48
GROUP BY name resolves to the joined n.name column rather than the computed fallback label. When names have not arrived, distinct application IDs collapse into one NULL-name group, attributing their combined bytes to an arbitrary application when permissive grouping is enabled. Group by the full COALESCE expression so unnamed applications retain separate chart slices.
Resolved the review's fallback chart-grouping finding in fad9b03: GROUP BY now uses the complete COALESCE(NULLIF(n.name,''),CONCAT('Application ',d.app_id)) expression instead of the ambiguous name alias. Unnamed/empty-name applications retain separate slices, while equal learned labels still combine. In addition to the query regression, executed the actual generated chart SQL with two missing names, an empty name, matching learned names across exporters and a different exporter-specific name behind a wildcard listener: all five labels and byte totals matched (SQLite compatibility executor, not a new MariaDB run).
Oversized numeric fields cause quadratic conversion work
nbar_collector.php:144
Templates permit numeric fields thousands of bytes wide. Passing such a counter to fv_nbar_uint() repeatedly expands and scans a decimal string, causing quadratic conversion work before the database can reject the value. An allowed exporter can stall both NBAR and legacy v9 processing because this decoder runs first. Reject oversized numeric fields before conversion and invalidate the cached template. Keep options-scope fields separate from data-field width checks, and add an oversized-counter regression.
Resolved the review's oversized-numeric-field finding in 3c69e49 and 628bec1. Data templates reject fields wider than their supported numeric types before extraction/conversion: counters up to 8 bytes, protocol 1, ports 2 and uptime timestamp 4. Invalid definitions evict their cached template. fv_nbar_uint itself rejects inputs wider than 64 bits as a defensive bound. Options scopes bypass data-counter width checks, and options records without names no longer fall through into flow-counter decoding. Regressions cover 4,096-byte fields, the individual type-width boundaries, invalidating an existing cache, a valid IPv6-width options scope and options without an application name.
Report information paragraph bypasses gettext localization
nbar_conversations.php:86
The second Report information paragraph is literal English, so it cannot follow the selected locale. Unlike the translated text at line 60, it never reaches gettext. Wrap it in __() with the flowview domain, substitute the retention value through a placeholder, and escape the result. After applying this change, refresh locales/po/cacti.pot only.
Resolved the review's Report information localization finding in d3dc902. The entire second conversation-information paragraph now passes through __() with the flowview domain, retention is substituted with a %d placeholder, and the complete translated result is escaped. Updated locales/po/cacti.pot only; existing translated .po files are unchanged. The rendering regression supplies a translated string containing markup and verifies both retention substitution and escaped output. Full suite: 347 tests / 2,843 assertions on PHP 8.2–8.4, changed-line coverage 100% (1,556/1,556).
The reason will be displayed to describe this comment to others. Learn more.
🟡 Changes recommended
Maintenance cannot start on supported Cacti 1.2.29; the broader collector and schema changes also warrant human review and updated integration validation.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
Cross-cutting collector, schema and worker changes need human review, with latest live integration reruns pending and a redirect correction outstanding.
The section-restore redirect drops header=false. The existing delete-filter redirect at includes/functions.php:650 reaches this branch when a last section is saved, but the second redirect loses header suppression. This can return full page chrome where Cacti expects an AJAX fragment. Preserve header=false in the Location URL, and cover this redirect case in the request tests.
Addressed the previously missed redirect finding in Copilot’s latest review in ef317f6.
The section-restore redirect now preserves header=false from the incoming request. This keeps the existing delete-filter → restore-saved-section path in fragment mode instead of returning full page chrome. Added request regressions for saved Flow Reports and Cisco Apps sections, simultaneous headercontent=true, and ordinary navigation without header suppression. The request fixture records the actual redirect target used by the branch.
Validation: 355 tests / 2,892 assertions pass on PHP 8.3 with the minimum Cacti 1.2.29 harness; configured changed-line coverage gate 100% (1,585/1,585 included lines); manifest, gettext and whitespace checks pass. Latest live database/browser rerun and full upstream Actions remain pending. The exporter-identity thread remains open with its earlier implementation and resolution explanation; no reviewer-thread status changed in this pass.
Fixed a live Safari layout failure in d9626cf. A collapsed Last seen column wrapped dates one character per line, making connection rows about 240px tall. Tables now use automatic layout, observation-date columns have a 136px minimum and do not wrap, and overflow stays in the report wrapper. Resizable initialization validates saved width arrays, enforces readable column minima and makes the restored table wide enough for its columns instead of crushing the final column.
The running installation uses older inline assets; equivalent CSS fixes were applied to its Connections/Conversations renderers after backing them up. Safari visibly changed from oversized rows to compact rows with both date columns readable. That live check validates the CSS correction; the newer PR-only saved-width normalization was syntax-checked but has not been deployed as part of a full live candidate upgrade. Local PHP 8.3/Cacti 1.2.29 suite: 355 tests / 2,892 assertions; JavaScript syntax, manifest and whitespace checks pass. No collector, database or router configuration changed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Depends on #294 for the bundled DNS library’s PHP 8.4 compatibility. This branch includes that prerequisite commit.
Add a Cisco Apps (NBAR) tab with Top applications, Conversations and Connections, IPv4/IPv6 and endpoint filters, observation dates, per-capture feed status, an application-volume chart and account-specific capture/network/display options. A setup dialog documents a Cisco 1100 Series NetFlow v9 example; the integration does not configure routers.
Hourly and daily summaries are shared across selections. Partial boundaries and invalidated revisions fall back to raw records. Connection pages and exact counts are requested separately with bounded account-session caches. Candidate pages and grouped fallback pages use the same deterministic tie order before and after pagination. Exact-count sources guard their checkpoint coverage and revisions at execution time, including checkpoint extensions and late exports. Retention is configurable up to 30 days and constrains report timespans.
NBAR timespan requests bypass legacy Flow Reports session storage for every preset. Conversations exclude multicast endpoints in both IP families. Conversation report information is translated through gettext, with a formatted retention placeholder and escaped translated output. Feed-query errors are distinguished from empty captures. Retention cleanup drains repeated batches across all retention levels within a shared ten-second budget, before summary maintenance.
Zero-length template fields are rejected and invalidate the affected cached template until a valid refresh arrives. Template state is bounded to 256 templates / 8,192 field definitions globally and 64 templates / 2,048 fields per exporter across all source IDs, with LRU eviction and 30-minute inactivity expiry. Numeric data widths are checked before conversion (counters 8 bytes, protocol 1, ports 2, uptime timestamp 4); options scopes remain separate. Chart grouping uses the complete learned-name/fallback-label expression, keeping unnamed application IDs in separate slices.
Actual exporter addresses are preserved through collection and summary keys; listener allowlists are not treated as exporter identities. Legacy counters are retained, but previously merged multi-exporter rows cannot be attributed retrospectively. Query failures render explicit errors. Observation checkpoint seeds are retryable and the DNS budget regression runs in a fresh process.
NBAR maintenance launches outside the poller, with one registered supervisor and a direct PHP child bounded to 90 seconds using PHP’s argv-array process API, independent of the Cacti timeout-helper signature. MariaDB hourly builds also establish a verified 20-second statement deadline and restore the previous value. MySQL hourly builds run only in the supervised child.
Validation:
Limitations: application decoding currently supports NetFlow v9, not IPFIX application records. MariaDB supplies verified DML statement deadlines for daily materialization and observation backfills. On MySQL 8 these workers defer safely, while reports continue through hourly/raw sources and exact counts use the SELECT execution-time hint; the MySQL path is unit-tested, not validated against a live MySQL server in this revision. The latest corrections have local regression coverage (including real Cacti timespan handling and generated multicast-predicate execution with a SQLite compatibility executor); their live MariaDB rerun is pending because the test terminal is unavailable. Upstream GitHub Actions still requires maintainer approval before the full Ubuntu/poller workflow can run.
Latest Copilot summary correction
Commit ef317f6 preserves
header=falsewhen restoring the last saved FlowView section, so Cacti fragment requests retain header suppression across the second redirect. Request regressions cover both saved sections, simultaneousheadercontent=true, and ordinary full-page navigation. The latest PHP 8.3/Cacti 1.2.29 harness passes 355 tests / 2,892 assertions, the configured changed-line coverage gate passes at 100% (1,585/1,585 included lines), and manifest/gettext/whitespace checks pass. This does not replace pending live integration and full upstream CI.