fix: URL-decode WiFi passwords before truncating to buffer size

[muness]

Summary

• Fixes silent corruption of WiFi passwords containing special characters during device setup
• A 63-char password from a password manager with special chars (e.g. !@#$) URL-encodes to potentially 189 bytes, but get_form_field() was truncating to 64 bytes before URL-decoding, producing a garbled shorter password → "Wrong Password"
• Uses a 256-byte intermediate buffer for the URL-encoded form, decodes there, then copies decoded result to the caller's 65-byte output buffer

Fixes #179

Files Changed

• idf_app/main/captive_portal.c — captive portal WiFi setup (GET handler)
• idf_app/main/config_server.c — config server WiFi add (POST handler)
• frame_app/main/captive_portal.c — frame app captive portal (GET handler)

Test plan

• Flash firmware and configure WiFi with a short, simple password — should still work
• Configure WiFi with a 63-character password containing special characters (!@#$%^&*) — should now connect successfully
• Verify config server /wifi-add POST endpoint also works with long passwords

Summary by CodeRabbit

• Bug Fixes
  • Fixed form-field decoding and truncation issues so submitted form data is decoded reliably and preserved.
  • Improved handling of stored configuration fields (Wi‑Fi credentials and bridge URL) to prevent accidental truncation and ensure proper termination, reducing corrupted or incomplete settings.

[coderabbitai]

Warning

Rate limit exceeded

@muness has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 1 minutes and 56 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 3d62fa51-4de2-4e78-853c-beb92ca4007a

📥 Commits

Reviewing files that changed from the base of the PR and between 2f0c0f8 and ebef10a.

📒 Files selected for processing (10)

• common/bridge_client.c
• common/manifest_parse.c
• common/manifest_ui.c
• common/rk_cfg.h
• common/ui.c
• frame_app/main/ble_remote.c
• frame_app/main/captive_portal.c
• idf_app/main/captive_portal.c
• idf_app/main/config_server.c
• idf_app/main/ota_update.c

📝 Walkthrough

Walkthrough

Reworked form-field parsing and several string-copy sites: get_form_field() now decodes URL-encoded substrings via a fixed intermediate 256-byte buffer before copying into caller buffers; multiple places switched from strncpy(..., size-1) to snprintf(..., "%s", ...) for safer truncation and NUL-termination.

Changes

Cohort / File(s)	Summary
Frame captive portal frame_app/main/captive_portal.c	get_form_field() now copies the matched URL-encoded substring into a fixed 256-byte stack buffer, URL-decodes there, then copies the decoded result into the caller out buffer with explicit clamping and NUL-termination (prevents truncation of URL-encoded inputs).
IDF captive portal & config server idf_app/main/captive_portal.c, idf_app/main/config_server.c	Same get_form_field() change as above. Also replaced strncpy(..., size-1) with snprintf(..., "%s", ...) when writing Wi‑Fi/bridge strings to ensure consistent truncation and termination semantics.
Config helpers common/rk_cfg.h	Added #include <stdio.h> and replaced strncpy(..., size-1) + explicit termination with snprintf(..., sizeof(...), "%s", ...) when adding/updating cfg->wifi[].ssid and cfg->wifi[].pass.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I nibble bytes in careful stride,
Encoded crumbs no longer hide,
Into a buffer neat and true,
I decode secrets back to you.
Wi‑Fi whispers safe at last, hop hop—no more past!

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main fix: reordering URL-decoding before truncation to prevent password corruption when decoding multi-byte URL-encoded special characters.
Linked Issues check	✅ Passed	Changes implement all key requirements from #179: use 256-byte intermediate buffer for URL-encoded data [captive_portal.c, config_server.c, rk_cfg.h], decode before copying to output, preserve final buffer sizes (pass[65], ssid[33]), and apply fix across all affected handlers.
Out of Scope Changes check	✅ Passed	All changes directly address #179's requirements: intermediate buffer addition, decoding order fix, and strncpy→snprintf conversions to eliminate compiler warnings from the buffer size changes.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/wifi-password-url-decode

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

🧹 Nitpick comments (1)

idf_app/main/captive_portal.c (1)

124-127: Consider aligning field search logic with frame_app/main/captive_portal.c.

The frame_app version has substring match protection that checks the preceding character is & or start-of-string, preventing false matches like xssid= when searching for ssid=. This file (and config_server.c) use simple strstr(). While not introduced by this PR, you may want to align them for consistency.

♻️ Optional: Add substring match protection

-  const char *start = strstr(data, search);
-  if (!start) {
-    return false;
-  }
+  // Search for the field, skipping substring matches like "xssid=" for "ssid="
+  const char *start = data;
+  while ((start = strstr(start, search)) != NULL) {
+    if (start == data || *(start - 1) == '&') break;
+    start++;  // Skip this substring match, keep looking
+  }
+  if (!start) {
+    return false;
+  }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@idf_app/main/captive_portal.c` around lines 124 - 127, The current substring
search uses const char *start = strstr(data, search); which can produce false
positives (e.g. matching "xssid="); change the logic to find the next occurrence
and verify the character before the match is either '&' or the start of the
string: loop using strstr(start + 1, search) on failure of that check until a
valid match or NULL, or implement a small helper that checks the preceding
character; update references in this file (start, strstr, data, search) to use
that guarded-match approach consistent with frame_app/main/captive_portal.c.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@idf_app/main/captive_portal.c`:
- Around line 124-127: The current substring search uses const char *start =
strstr(data, search); which can produce false positives (e.g. matching
"xssid="); change the logic to find the next occurrence and verify the character
before the match is either '&' or the start of the string: loop using
strstr(start + 1, search) on failure of that check until a valid match or NULL,
or implement a small helper that checks the preceding character; update
references in this file (start, strstr, data, search) to use that guarded-match
approach consistent with frame_app/main/captive_portal.c.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: e2085b49-40ed-4d31-85e6-48a1bf82a819

📥 Commits

Reviewing files that changed from the base of the PR and between b639f5b and 74367ef.

📒 Files selected for processing (3)

• frame_app/main/captive_portal.c
• idf_app/main/captive_portal.c
• idf_app/main/config_server.c

[github-actions]

Firmware Preview

Flash this PR's firmware directly in your browser:
https://roon-knob.muness.com/pr/180/flash.html

Updated: 2026-03-20T18:44:38.999Z

[muness]

Requested a test from the user who reported #179

[muness]

Superseded by direct fix on master. The strncpy refactor is no longer needed.