Skip to content

fix(websub): handle empty channels map in mapWebSubAPIModelToAPI#2009

Open
SAY-5 wants to merge 1 commit into
wso2:mainfrom
SAY-5:fix/websub-empty-channels-panic
Open

fix(websub): handle empty channels map in mapWebSubAPIModelToAPI#2009
SAY-5 wants to merge 1 commit into
wso2:mainfrom
SAY-5:fix/websub-empty-channels-panic

Conversation

@SAY-5
Copy link
Copy Markdown

@SAY-5 SAY-5 commented May 21, 2026

Purpose

Resolves #1995.

mapWebSubChannelsModelToAPI returned a nil *map for empty or nil input, and the caller in mapWebSubAPIModelToAPI then dereferenced that nil pointer when assigning to api.WebSubAPI.Channels (a value-type map). Any Get, List, or Update return path panicked for a WebSub API stored with an empty channels map. The bug was originally noted in #1992 (review comment).

Goals

Make the conversion safe for empty/nil channel inputs and have it always produce a usable (non-nil) map suitable for direct assignment to the value-type Channels field.

Approach

mapWebSubChannelsModelToAPI now returns map[string]api.WebSubChannel directly instead of *map[string]api.WebSubChannel. For empty/nil input it returns an empty (non-nil) map. The call site in mapWebSubAPIModelToAPI drops the * indirection.

User stories

N/A (defect fix).

Documentation

N/A — internal helper behavior, no user-facing API change.

Automation tests

  • Unit tests
    • TestWebSubAPI_MapModelToAPI_EmptyChannelsDoesNotPanic exercises mapWebSubAPIModelToAPI with a nil channel map and an empty channel map, asserting it does not panic and that the resulting Channels map is non-nil and empty. On main both subtests fail with "runtime error: invalid memory address or nil pointer dereference"; with this fix they pass.
    • TestWebSubAPI_MapChannelsModelToAPI_NeverReturnsNil pins the helper contract that nil and empty inputs both yield a non-nil map.
  • Existing TestWebSubAPI_* tests in platform-api/src/internal/service/websub_api_test.go continue to pass.
$ go test ./internal/service/ -count=1 -timeout 180s
ok  	platform-api/src/internal/service	0.418s

Security checks

Samples

N/A.

Related PRs

Test environment

  • Go 1.26.2 (matches platform-api-pr-check.yml), macOS arm64 locally.

Fixes wso2#1995

mapWebSubChannelsModelToAPI returned a nil *map for empty/nil input and
the caller in mapWebSubAPIModelToAPI then dereferenced that nil pointer
when assigning to api.WebSubAPI.Channels (a value-type map). Any Get,
List, or Update return path panicked for a WebSub API stored with an
empty channels map.

Return the value-type map directly so the result is always usable, and
drop the now-unneeded pointer indirection at the call site.
@CLAassistant
Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 21, 2026

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 41929be2-88d3-43d5-aa54-aa4f152f98ce

📥 Commits

Reviewing files that changed from the base of the PR and between f7a2827 and d2d5911.

📒 Files selected for processing (2)
  • platform-api/src/internal/service/websub_api.go
  • platform-api/src/internal/service/websub_api_test.go

📝 Walkthrough

Summary

This PR fixes a runtime panic that occurred during Get, List, and Update operations on WebSub APIs that were stored with empty or nil channel maps.

Root Cause

The mapping helper mapWebSubChannelsModelToAPI previously returned a pointer to a map (*map[string]api.WebSubChannel) and would return nil when the input map was empty or nil. The caller would then dereference this potentially nil pointer when assigning to the value-typed api.WebSubAPI.Channels field, causing a panic.

Changes Made

platform-api/src/internal/service/websub_api.go:

  • Changed mapWebSubChannelsModelToAPI return type from *map[string]api.WebSubChannel to map[string]api.WebSubChannel
  • Removed the early nil check that returned nil for empty maps; the function now always returns a (possibly empty) non-nil map
  • Updated the call site in mapWebSubAPIModelToAPI to remove the pointer dereference: *mapWebSubChannelsModelToAPI(...)mapWebSubChannelsModelToAPI(...)
  • Added documentation comments explaining that the helper always returns a non-nil map to safely support value-type map fields

platform-api/src/internal/service/websub_api_test.go:

  • Added TestWebSubAPI_MapModelToAPI_EmptyChannelsDoesNotPanic to verify that mapping with nil or empty channel maps does not cause a panic and produces a non-nil, empty Channels map
  • Added TestWebSubAPI_MapChannelsModelToAPI_NeverReturnsNil to pin the helper contract and ensure it always returns a non-nil map

Impact

  • Fixes panics in Get, List, and Update operations for WebSub APIs with empty channel maps (fixes issue #1995)
  • All changes are to unexported helper functions; no public API modifications
  • Existing tests continue to pass

Walkthrough

This pull request fixes a nil-pointer dereference in the WebSub API channel mapping layer. The mapWebSubChannelsModelToAPI function was refactored to return a non-nil map[string]api.WebSubChannel instead of a nullable pointer. The corresponding caller mapWebSubAPIModelToAPI was updated to use the direct return value. Two new unit tests validate the fix: one ensures the top-level mapper handles empty channels without panicking, and the other verifies the channel mapper always returns a non-nil map.

Suggested reviewers

  • VirajSalaka
  • renuka-fernando
  • malinthaprasan
  • AnuGayan
  • senthuran16
  • tharindu1st
  • RakhithaRR
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main fix: handling an empty channels map in mapWebSubAPIModelToAPI to prevent nil-pointer dereference.
Description check ✅ Passed The description is comprehensive and follows the template structure, covering purpose, goals, approach, documentation, automation tests, security checks, and test environment details.
Linked Issues check ✅ Passed The PR addresses issue #1995 by changing mapWebSubChannelsModelToAPI to return a non-nil map and updating the call site in mapWebSubAPIModelToAPI, eliminating the nil-pointer dereference when mapping empty channels.
Out of Scope Changes check ✅ Passed All changes are directly scoped to fixing the nil-pointer dereference issue: the helper function signature change, the map return logic, the call site update, and supporting unit tests.
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

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.12.2)

level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain modules listed in go.work or their selected dependencies"


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.

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.

Fix nil-pointer dereference when mapping WebSub API channels in mapWebSubAPIModelToAPI

2 participants