Skip to content

Conversation

@SimonFair
Copy link
Contributor

@SimonFair SimonFair commented Jan 23, 2026

Summary by CodeRabbit

  • Refactor

    • Reworked address parsing to a clearer, sentinel-aware component-by-component process.
    • Added explicit handling for the special all-empty address form to simplify output.
  • Bug Fixes

    • Stricter validation of hexadecimal address segments; invalid segments now return an error.
    • Correct handling of consecutive and trailing separators to avoid ambiguous parsing.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 23, 2026

Walkthrough

Reworks the unzero6 function in the network config script to use an explicit, sentinel-aware parser: maps leading :: to a sentinel, validates each hex component (1–4 hex digits), emits segments with proper separators, handles trailing ::, and returns error on invalid components.

Changes

Cohort / File(s) Summary
Network Configuration
etc/rc.d/rc.inet1.conf
Replaced a compact loop in unzero6 with an explicit parser that: maps leading :: to a sentinel (:-:), initializes separator M, iterates components, validates 1–4 hex digits, prints numeric segments with separators, restores sentinel to :, handles trailing ::, and exits with error (1) on invalid hex components.

Sequence Diagram(s)

(omitted — change is localized and does not introduce multi-component control flow requiring a sequence diagram)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I parsed the colons, one by one,
Mapped a double hop beneath the sun,
Checked each hex nibble, tidy and bright,
If one went astray, I halted the flight.
Hops fixed — now the address is right. 🥕

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically identifies the main change: fixing an IPv6 DNS issue related to trailing '::' notation in network configuration.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@etc/rc.d/rc.inet1.conf`:
- Around line 35-39: The change to unzero6() broke mid-address `::` handling by
adding an extra colon when `M` already contained ":" during the loop; revert the
loop to the original logic that iterates over ${A//:/ } and prints either "$M%x"
for non-"-" segments or "$M:" for "-" placeholders (using variables A, M, Q),
and then after the loop append an extra ":" only when the original input ends
with "::" (detectable from the original parameter or A) so addresses ending with
"::" keep the trailing double colon without producing ":::" in middle positions.

@SimonFair SimonFair marked this pull request as draft January 23, 2026 11:23
@github-actions
Copy link

🔧 PR Test Plugin Available

A test plugin has been generated for this PR that includes the modified files.

Version: 2026.01.23.1539
Build: View Workflow Run

📥 Installation Instructions:

Install via Unraid Web UI:

  1. Go to Plugins → Install Plugin
  2. Copy and paste this URL:
https://preview.dl.unraid.net/pr-plugins/pr-2530/webgui-pr-2530.plg
  1. Click Install

Alternative: Direct Download

⚠️ Important Notes:

  • Testing only: This plugin is for testing PR changes
  • Backup included: Original files are automatically backed up
  • Easy removal: Files are restored when plugin is removed
  • Conflicts: Remove this plugin before installing production updates

📝 Modified Files:

Click to expand file list
etc/rc.d/rc.inet1.conf

🔄 To Remove:

Navigate to Plugins → Installed Plugins and remove webgui-pr-2530, or run:

plugin remove webgui-pr-2530

🤖 This comment is automatically generated and will be updated with each new push to this PR.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@etc/rc.d/rc.inet1.conf`:
- Line 47: unzero6 can return 1 on invalid hex components but none of its
callers check this, causing silent failures; update every call site of unzero6
(the IPv6 address configuration handling, the DNS server entry processing, the
route configuration logic, and the rc.wireless setup calls) to test its return
status, and on non‑zero either log an error and abort the higher‑level operation
(return failure) or skip the malformed entry rather than using the empty output
— ensure you reference the unzero6 invocation and the variable capturing its
output in each caller and early‑exit or continue appropriately to avoid feeding
empty strings into downstream commands.

for Q in ${A//:/ }; do
if [[ $Q != - ]]; then
# Validate hex (1-4 digits)
[[ $Q =~ ^[0-9A-Fa-f]{1,4}$ ]] || return 1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# Search for the function and its usages
rg -n -C3 'unzero6' etc/rc.d/rc.inet1.conf

Repository: unraid/webgui

Length of output: 200


🏁 Script executed:

# Also search for unzero6 calls throughout the codebase
rg -n 'unzero6' --type sh

Repository: unraid/webgui

Length of output: 39


🏁 Script executed:

# Get more context around line 47 in the file
sed -n '30,70p' etc/rc.d/rc.inet1.conf | cat -n

Repository: unraid/webgui

Length of output: 1365


🏁 Script executed:

# Search for calls to unzero6 anywhere in the codebase
rg -n 'unzero6' -A 2 -B 2

Repository: unraid/webgui

Length of output: 2421


🏁 Script executed:

# Also search for patterns that might invoke unzero6
rg -n '\$\(.*unzero6' -A 2 -B 2

Repository: unraid/webgui

Length of output: 1979


🏁 Script executed:

# Get more context around each call site to verify error handling
sed -n '410,415p' etc/rc.d/rc.inet1 | cat -n

Repository: unraid/webgui

Length of output: 459


🏁 Script executed:

sed -n '505,515p' etc/rc.d/rc.inet1 | cat -n

Repository: unraid/webgui

Length of output: 878


🏁 Script executed:

sed -n '645,650p' etc/rc.d/rc.inet1 | cat -n

Repository: unraid/webgui

Length of output: 360


Add error checking at all call sites for the return value of unzero6.

The function returns 1 when encountering invalid hex components. Currently, none of the 7 callers check this return value, resulting in silent failures when processing malformed IPv6 addresses:

  • IPv6 address configuration (line 413)
  • DNS server entries (lines 510-513)
  • Route configuration (lines 647-648)
  • Wireless interface setup (rc.wireless:157)

When unzero6 fails, it outputs nothing, leaving downstream commands with empty or incomplete strings that fail silently.

🤖 Prompt for AI Agents
In `@etc/rc.d/rc.inet1.conf` at line 47, unzero6 can return 1 on invalid hex
components but none of its callers check this, causing silent failures; update
every call site of unzero6 (the IPv6 address configuration handling, the DNS
server entry processing, the route configuration logic, and the rc.wireless
setup calls) to test its return status, and on non‑zero either log an error and
abort the higher‑level operation (return failure) or skip the malformed entry
rather than using the empty output — ensure you reference the unzero6 invocation
and the variable capturing its output in each caller and early‑exit or continue
appropriately to avoid feeding empty strings into downstream commands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant