Skip to content

Conversation

@tudddorrr
Copy link
Contributor

No description provided.

@tudddorrr tudddorrr added the enhancement New feature or request label Dec 22, 2025
@claude
Copy link

claude bot commented Dec 22, 2025

Code Review - PR #170

🟡 Code Quality & Best Practices

Inconsistency: .Trim() applied to all query parameter values

ChannelsAPI.cs:23, 38, 62 - The .Trim() operation is applied to all dictionary values including numeric strings.

While Uri.EscapeDataString() is necessary and correct, the .Trim() creates inconsistency:

  • Trimming page.ToString() and aliasId.ToString() is unnecessary (integers never have whitespace)
  • More importantly, it changes behavior for user-provided values like propKey, propValue, and search

Impact: If users intentionally include leading/trailing spaces in property keys/values, those will now be silently removed. This is a breaking change in behavior.

Suggestion: Apply trimming selectively before adding to dictionary, not in the LINQ select.


🔵 Performance Considerations

Minor: Trim called multiple times in LINQ

The .Trim() is called for every parameter during the LINQ Select(). For better clarity and slight performance, trim when adding to dictionary instead.


✅ Security & Other Categories

  • Security: No issues - Uri.EscapeDataString() properly prevents injection attacks
  • Potential bugs: No issues found
  • Backwards compatibility: No breaking changes to API surface

Overall: The URL encoding addition is a good security improvement. Main concern is the implicit trimming behavior change for user-provided values.

@tudddorrr tudddorrr merged commit 39aef7c into develop Dec 22, 2025
2 checks passed
@tudddorrr tudddorrr deleted the channel-search branch December 22, 2025 20:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants