Add defensive coding fixes to prevent index out of range errors #582
Conversation
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
This commit adds bounds checking in several locations where array/slice indexing could potentially cause panics: 1. report.go:520 - Add check that Values slices have elements before accessing [0] when looking up reference data 2. report.go:386 - Use cached memLatTableValues instead of calling getTableValues twice, ensuring consistency 3. dimm.go:628 - Add check that dimms[0] has sufficient elements before accessing BankLocatorIdx and LocatorIdx 4. report_tables.go:2036 - Add bounds checks before accessing DerivedSocketIdx, DerivedChannelIdx, and DerivedSlotIdx Values[0] 5. report_tables.go:1222-1224 - Store Split results and check length before accessing [0] to make code more defensive 6. system.go:105 - Add length check before accessing fields[len-2] to prevent panic on short slices 7. system.go:129 - Add bounds checks for both regex match results and fields slice before accessing indices These changes prevent potential panics when processing unexpected or malformed data while maintaining the existing error handling behavior. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
This commit adds defensive bounds checking in internal/common to prevent index out of range errors when processing system data: ### nic.go fixes: 1. Line 117 - Check bounds before accessing Split result when parsing model names with parentheses 2. Line 182 - Validate deviceFunc slice has elements before accessing [0] when building card identifiers from PCI addresses ### power.go fixes: 3. Lines 40, 60, 88 - Add bounds checks before accessing Split result [1] when parsing EPP-related MSR values from colon-separated output 4. Line 127 - Consolidate bounds checks for fieldValues array access 5. Lines 212, 220 - Add bounds checks before accessing row indices when interpreting ELC mode values These changes prevent panics when: - NIC model names have unexpected formats - PCI addresses are malformed - MSR register output has unexpected format - CSV data from ELC scripts has missing columns All changes maintain existing error handling behavior while making the code more robust against unexpected input formats. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Harper, Jason M <jason.m.harper@intel.com>
Signed-off-by: Harper, Jason M <jason.m.harper@intel.com>
Signed-off-by: Harper, Jason M <jason.m.harper@intel.com>
This commit adds defensive bounds checking in internal/report rendering functions to prevent index out of range errors when processing table data: ### render_excel.go fixes: 1. Line 72 - Add check that targetTableValues has elements before accessing [0] in renderXlsxTableMultiTarget 2. Line 208 - Check each field's Values length instead of assuming all fields have same structure as Fields[0] ### render_html.go fixes: 3. Line 306 - Add check that allTargetsTableValues has elements before accessing [0] in createHtmlReportMultiTarget 4. Line 599 - Add bounds check before accessing field.Values[0] when rendering non-row tables 5. Line 614 - Add check that tableValues has elements before accessing [0] in RenderMultiTargetTableValuesAsHTML These changes prevent panics when: - Rendering reports with empty target lists - Processing tables where fields have varying numbers of values - Generating multi-target HTML reports with no data All changes maintain existing rendering behavior while making the code more robust against edge cases with missing or incomplete data. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
This PR adds defensive bounds checking in several locations within
cmd/reportwhere array/slice indexing could potentially cause panics. These fixes address reported "index out of range" errors by adding proper validation before accessing array/slice elements.Changes
1. report.go:520 - Reference data lookup
.Values[0]without checking if Values slice has elements2. report.go:386 - Memory benchmark table access
getTableValues()twice, potentially accessing different datamemLatTableValuesvariable for consistency3. dimm.go:628 - 2D array access
dimms[0]has sufficient elements before accessingBankLocatorIdxandLocatorIdx4. report_tables.go:2036 - DIMM table renderer
.Values[0]without bounds checking5. report_tables.go:1222-1224 - String split access
6. system.go:105 - Filesystem fields parsing
fields[len(fields)-2]without length validationlen(fields) >= 2before accessing7. system.go:129 - Mount options parsing
Testing
go build ./cmd/report/...Impact
These defensive coding improvements will prevent panics when processing:
The changes are backward compatible and maintain the existing error handling patterns while making the code more robust.
Review Notes
This PR is marked as draft for initial review. All fixes follow the same defensive coding pattern: check bounds before accessing array/slice elements. No functional behavior changes, only added safety checks.