Fix MBS Broadcast distribution session lifecycle and Service Announcement crashes

src/mbsf/DistributionSessionInfo.cc

@@ -162,6 +162,21 @@ CJson DistributionSessionInfo::json(bool as_request = false) const
     return m_mbsDistributionSessionInfo->toJSON(as_request);
 }
 
+namespace {
+// Compares two optional<shared_ptr<T>> fields, treating "absent" and "present but null" as the
+// same "no value" state so a field that's merely re-sent unchanged doesn't spuriously trip up as
+// "changed".
+template <typename T>
+bool optionalPtrFieldsEqual(const std::optional<std::shared_ptr<T>> &a, const std::optional<std::shared_ptr<T>> &b)
+{
+    bool a_has = a.has_value() && a.value();
+    bool b_has = b.has_value() && b.value();
+    if (a_has != b_has) return false;
+    if (!a_has) return true;
+    return *a.value() == *b.value();
+}
+}
+
 std::shared_ptr<MBSDistributionSessionInfo> &DistributionSessionInfo::updateMBSDistributionSessionInfo(
     std::shared_ptr<MBSDistributionSessionInfo> new_mbs_dist_session_infos)
 {
@@ -175,12 +190,34 @@ std::shared_ptr<MBSDistributionSessionInfo> &DistributionSessionInfo::updateMBSD
 
     m_mbsDistributionSessionInfo->setTgtServAreas(std::move(new_mbs_dist_session_infos->getTgtServAreas()));
 
+    // TS 26.502 clause 4.5.6 lists these among the parameters the MBS Application
+    // Provider may update at any time, alongside mbsServInfo/mbsFSAId/tgtServAreas
+    // above -- not gated on INACTIVE like the block below.
+    m_mbsDistributionSessionInfo->setExtTgtServAreas(std::move(new_mbs_dist_session_infos->getExtTgtServAreas()));
+    m_mbsDistributionSessionInfo->setNrRedCapUeInfo(std::move(new_mbs_dist_session_infos->getNrRedCapUeInfo()));
+
     // --------------------------------------------------------------------
     // 2. Conditional updates – only when the session is INACTIVE
     // --------------------------------------------------------------------
     std::optional<std::shared_ptr< DistSessionState > > dist_session_state = m_mbsDistributionSessionInfo->getMbsDistSessState();
+    bool is_inactive = dist_session_state.has_value() && dist_session_state.value()->getValue() == DistSessionState::VAL_INACTIVE;
+
+    // BUG FIX: objDistrInfo/pckDistrInfo (and hence e.g. objAcqIds) were previously only ever
+    // copied across below when the Distribution Session is INACTIVE -- if PATCHed while
+    // ESTABLISHED/ACTIVE the request was accepted (200) but the change was silently dropped, so
+    // e.g. a PATCH narrowing objAcqIds would echo back the OLD array. These fields are only
+    // mutable while INACTIVE, so reject the request outright instead of silently no-op'ing it.
+    if (!is_inactive) {
+        if (!optionalPtrFieldsEqual(m_mbsDistributionSessionInfo->getObjDistrInfo(), new_mbs_dist_session_infos->getObjDistrInfo()) ||
+            !optionalPtrFieldsEqual(m_mbsDistributionSessionInfo->getPckDistrInfo(), new_mbs_dist_session_infos->getPckDistrInfo())) {
+            throw ModelException(
+                "objDistrInfo/pckDistrInfo cannot be modified while the MBS Distribution Session is not INACTIVE",
+                "MBSDistributionSessionInfo", "objDistrInfo",
+                fiveg_mag_reftools::ProblemCause::MODIFICATION_NOT_ALLOWED);
+        }
+    }
 
-    if (dist_session_state.has_value() && dist_session_state.value()->getValue() == DistSessionState::VAL_INACTIVE) {
+    if (is_inactive) {
         // ----- Max Continuous Bit Rate -----
         m_mbsDistributionSessionInfo->setMaxContBitRate(std::move(new_mbs_dist_session_infos->getMaxContBitRate()));
 
@@ -202,18 +239,12 @@ std::shared_ptr<MBSDistributionSessionInfo> &DistributionSessionInfo::updateMBSD
         // ----- Traffic Marking Info -----
         m_mbsDistributionSessionInfo->setTrafficMarkingInfo(std::move(new_mbs_dist_session_infos->getTrafficMarkingInfo()));
 
-        // ----- External Target Service Areas -----
-        m_mbsDistributionSessionInfo->setExtTgtServAreas(std::move(new_mbs_dist_session_infos->getExtTgtServAreas()));
-
         // ----- Multiplexed Service Flag -----
         m_mbsDistributionSessionInfo->setMultiplexedServFlag(std::move(new_mbs_dist_session_infos->getMultiplexedServFlag()));
 
         // ----- Restricted Flag -----
         m_mbsDistributionSessionInfo->setRestrictedFlag(std::move(new_mbs_dist_session_infos->getRestrictedFlag()));
 
-        // ----- NR RedCap UE Info -----
-        m_mbsDistributionSessionInfo->setNrRedCapUeInfo(std::move(new_mbs_dist_session_infos->getNrRedCapUeInfo()));
-
         // ----- Associated Session Id -----
         m_mbsDistributionSessionInfo->setAssociatedSessionId(std::move(new_mbs_dist_session_infos->getAssociatedSessionId()));
     }

src/mbsf/MBSMFMBSSession.cc

@@ -248,6 +248,16 @@ bool MBSMFMBSSession::processEvent(Open5GSEvent &MBSMFEvent)
                         }
                         UserDataIngSession::setMBSSessionFlag(*ids);
                     } else if (mbsf_event->result == OGS_ERROR) {
+                        // BUG FIX: the fallback call below used to run unconditionally, so even
+                        // when a specific cause was already matched and handled just above (e.g.
+                        // the registered 403 MBS_DIST_SESSION_ALREADY_CREATED), it was immediately
+                        // overwritten by a second, generic INBOUND_SERVER_ERROR call with no
+                        // problem_detail -- meaning the client only ever saw the generic 502-class
+                        // error regardless of what MB-SMF actually reported. Track whether a cause
+                        // (specific or the "no cause string" generic-with-detail case) was already
+                        // handled and only fall through to the bare generic call as a genuine
+                        // last resort (no problem_details at all, or an unregistered cause string).
+                        bool cause_handled = false;
                         if (mbsf_event->problem_details) {
                             cJSON *problem = OpenAPI_problem_details_convertToJSON((OpenAPI_problem_details_t*)mbsf_event->problem_details);
                             CJson problem_detail(problem, true);
@@ -256,12 +266,16 @@ bool MBSMFMBSSession::processEvent(Open5GSEvent &MBSMFEvent)
                                             MBSProblemCause::lookup(std::string(mbsf_event->problem_details->cause));
                                 if (cause.has_value()) {
                                     UserDataIngSession::setMBSSessionFailureFlag(*ids, cause.value(), problem_detail);
+                                    cause_handled = true;
                                 }
                             } else {
                                 UserDataIngSession::setMBSSessionFailureFlag(*ids, ProblemCause::INBOUND_SERVER_ERROR, problem_detail);
+                                cause_handled = true;
                             }
                         }
-                        UserDataIngSession::setMBSSessionFailureFlag(*ids, ProblemCause::INBOUND_SERVER_ERROR);
+                        if (!cause_handled) {
+                            UserDataIngSession::setMBSSessionFailureFlag(*ids, ProblemCause::INBOUND_SERVER_ERROR);
+                        }
                     } else {
                         UserDataIngSession::setMBSSessionFailureFlag(*ids, ProblemCause::INBOUND_SERVER_ERROR);
                     }

src/mbsf/MBSNcgi.cc

@@ -81,7 +81,9 @@ mb_smf_sc_ncgi_t *MBSNcgi::populateNcgi() {
 
     mb_smf_sc_ncgi_t *ncgi = mb_smf_sc_ncgi_new();
 
-    mb_smf_sc_ncgi_set_plmn_id(ncgi, mcc, mnc);
+    // Use the length-aware setter: mcc()/mnc() alone lose the MNC's actual digit
+    // count (2 vs 3), which a plain numeric value under 100 cannot distinguish.
+    mb_smf_sc_ncgi_set_plmn_id_len(ncgi, mcc, mnc, mbs_plmn_id->mncLen());
     uint64_t cell_id = nrCellId();
     ncgi->nr_cell_id = static_cast<uint64_t>(cell_id) & ((1ULL << 36) - 1);
     ncgi->nid = nid();

src/mbsf/MBSPlmnId.hh

@@ -56,6 +56,10 @@ public:
 
     uint16_t mcc();
     uint16_t mnc();
+    // The MNC's actual digit count (2 or 3), taken directly from the source
+    // string rather than guessed from mnc()'s numeric value -- a 3-digit MNC
+    // under 100 (e.g. "001") is otherwise indistinguishable from a 2-digit one.
+    uint8_t mncLen() const {return static_cast<uint8_t>(getMnc().length());};
 
 private:
     std::shared_ptr<reftools::mbsf::PlmnId> m_plmnId;

src/mbsf/Nmb2Build.cc

@@ -242,15 +242,29 @@ ogs_sbi_request_t *Nmb2Build::buildNmb2DistSessionPatch(void *context, void *dat
     std::shared_ptr<UserDataIngSession::ContextData> context_data_ptr(ing_session->getDistributionSessionInfoData(session_ids->second->second));
     DistSessionState req_state;
     if (context_data_ptr->needsUpdate) {
-        status_item.path = (char *)"/distSession";
+        // RFC 6901: the whole document is addressed by the empty JSON Pointer "".
+        status_item.path = (char *)"";
         std::shared_ptr<DistSession> dist_session = build_nmb2_create_dist_session(ing_session, context_data_ptr);
 
         std::string sess_id(context_data_ptr->mbstfDistSessionId);
 
         dist_session->setDistSessionId(sess_id);
         UserDataIngSession::addToRegistry(sess_id, session_ids->second);
 
-        patch_val = dist_session->toJSON(true);
+        // BUG FIX (found live, 2026-08-11): MBSTF stores the PATCH target for
+        // /dist-sessions/{id} as a CreateReqData (see DistributionSession.cc, which patches
+        // distributionSessionReqData() -- a CreateReqData, not a bare DistSession), and
+        // CreateReqData::fromJSON() (invoked by its applyJSONPatch() for an empty-path
+        // add/replace) requires its value to be a full CreateReqData document -- i.e. an
+        // object with a "distSession" property wrapping the DistSession fields, not the
+        // DistSession's own JSON directly. Sending dist_session->toJSON() unwrapped, as this
+        // used to, made every needsUpdate PATCH (a content/session change, not just a state
+        // change -- see the stateUpdate branch below for that) fail with "Mandatory
+        // Information Element Missing: distSession: Field \"distSession\" is required",
+        // silently breaking updates to any already-created distribution session.
+        CJson wrapped_patch_val = CJson::newObject();
+        wrapped_patch_val.set("distSession", dist_session->toJSON(true));
+        patch_val = wrapped_patch_val;
         const auto &state = dist_session->getDistSessionState();
         if (state) req_state = *state;
     } else if (context_data_ptr->stateUpdate) {
@@ -264,6 +278,19 @@ ogs_sbi_request_t *Nmb2Build::buildNmb2DistSessionPatch(void *context, void *dat
                 req_state = want_state;
             }
             patch_val = req_state.toJSON();
+            // BUG FIX (found live, 2026-08-10): MBSTF's actual PATCH target for
+            // /dist-sessions/{id} is CreateReqData (see DistributionSession::_apiSessionPatch(),
+            // which patches distributionSessionReqData(), a CreateReqData), and CreateReqData's
+            // generated applyPatch() (CreateReqData.cc) only recognises paths under its own
+            // "/distSession" property -- it does NOT expose DistSession's fields at the top
+            // level. The previous "/distSessionState" (no wrapper) path was rejected by MBSTF
+            // with "Runtime Error: Unknown path in JSON Patch", which meant every state-only
+            // PATCH (activate/deactivate) silently failed and triggered a rollback -- including
+            // the one that activates the built-in "USER SERVICE ANNOUNCEMENT CHANNEL" session,
+            // i.e. the real MBS-4-MC broadcast Service Announcement carousel never got updated
+            // with newly-provisioned services. Confirmed against CreateReqData.cc's path_prefix
+            // dispatch: it matches "/distSession" then delegates the remainder ("/distSessionState")
+            // to the nested DistSession object, which does recognise it (see DistSession.cc).
             status_item.path = (char *)"/distSession/distSessionState";
         }
     }

src/mbsf/TrackingAreaIdentity.cc

@@ -86,7 +86,9 @@ mb_smf_sc_tai_t *TrackingAreaIdentity::populateTai() {
     tracking_area = tac();
     n_id = nid();
 
-    return mb_smf_sc_tai_new(mcc, mnc, tracking_area, n_id);
+    // Use the length-aware constructor: mcc()/mnc() alone lose the MNC's actual
+    // digit count (2 vs 3), which a plain numeric value under 100 cannot distinguish.
+    return mb_smf_sc_tai_new_len(mcc, mnc, mbs_plmn_id->mncLen(), tracking_area, n_id);
 }
 
 uint32_t TrackingAreaIdentity::tac() {
@@ -124,12 +126,9 @@ uint32_t TrackingAreaIdentity::tac() {
 uint64_t* TrackingAreaIdentity::nid() {
     const std::optional<std::string > &nid = getNid();
     if (!nid.has_value()) return nullptr;
-    uint64_t value = 0;
-    for (char ch : nid.value()) {
-        if (std::isdigit(static_cast<unsigned char>(ch))) {
-            value = value * 10 + (ch - '0');
-        }
-    }
+    // TS 29.571 Nid is an 11-character hex string (44-bit SNPN Network Id) --
+    // parse as base 16, matching the correct sibling implementation MBSNcgi::nid().
+    uint64_t value = std::stoull(nid.value(), nullptr, 16);
 
     uint64_t *result = static_cast<uint64_t*>(std::malloc(sizeof(uint64_t)));
     if (result != nullptr) {