diff --git a/CHANGELOG.md b/CHANGELOG.md
index 21ae2e01..20703d0b 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -93,6 +93,7 @@ Developers may need to change their code
### Corrected Bugs
+* Fixed: [Netconf commit confirm session-id mismatch #407](https://github.com/clicon/clixon/issues/407)
* Fixed: Initialized session-id to 1 instead of 0 following ietf-netconf.yang
* Fixed: [snmpwalk doesn't show properly SNMP boolean values which equal false](https://github.com/clicon/clixon/issues/400)
* Fixed: yang-library: Remove revision if empty instead of sending empty revision
diff --git a/apps/backend/backend_client.c b/apps/backend/backend_client.c
index 0c3bdf50..03f52b63 100644
--- a/apps/backend/backend_client.c
+++ b/apps/backend/backend_client.c
@@ -567,7 +567,7 @@ from_client_edit_config(clicon_handle h,
break;
}
}
- if ((ret = candidate_commit(h, NULL, "candidate", cbret)) < 0){ /* Assume validation fail, nofatal */
+ if ((ret = candidate_commit(h, NULL, "candidate", myid, cbret)) < 0){ /* Assume validation fail, nofatal */
if (netconf_operation_failed(cbret, "application", clicon_err_reason)< 0)
goto done;
xmldb_copy(h, "running", "candidate");
diff --git a/apps/backend/backend_commit.c b/apps/backend/backend_commit.c
index ffa49031..ea4082e1 100644
--- a/apps/backend/backend_commit.c
+++ b/apps/backend/backend_commit.c
@@ -648,6 +648,7 @@ candidate_validate(clicon_handle h,
* @param[in] h Clicon handle
* @param[in] xe Request: (or NULL)
* @param[in] db A candidate database, not necessarily "candidate"
+ * @param[in] myid Client id of triggering incoming message (or 0)
* @param[out] cbret Return xml tree, eg ..., value that matches the value accompanying the prior confirmed-commit
*
* @param[in] h Clicon handle
- * @param[in] xe Request:
+ * @param[in] xe Request:
* @param[in] myid current client session-id
* @retval 1 The confirming-commit is valid
* @retval 0 The confirming-commit is not valid
@@ -419,6 +419,8 @@ check_valid_confirming_commit(clicon_handle h,
clicon_err(OE_CFG, EINVAL, "xe is NULL");
goto done;
}
+ if (myid == 0)
+ goto invalid;
switch (confirmed_commit_state_get(h)) {
case PERSISTENT:
if (xe_persist_id(xe, &persist_id)) {
@@ -470,15 +472,18 @@ check_valid_confirming_commit(clicon_handle h,
*
* @param[in] h Clicon handle
* @param[in] xe Commit rpc xml or NULL
+ * @param[in] myid Current session-id, only valid > 0 if call is made as a result of an incoming message
* @retval 0 OK
* @retval -1 Error
+ * @note There are some calls to this function where myid is 0 (which is invalid). It is unclear if such calls
+ * actually occur, and if so, if they are correctly handled. The calls are from do_rollback() and load_failsafe()
*/
int
handle_confirmed_commit(clicon_handle h,
- cxobj *xe)
+ cxobj *xe,
+ uint32_t myid)
{
int retval = -1;
- uint32_t session_id;
char *persist;
unsigned long confirm_timeout = 0L;
int cc_valid;
@@ -488,17 +493,14 @@ handle_confirmed_commit(clicon_handle h,
clicon_err(OE_CFG, EINVAL, "xe is NULL");
goto done;
}
- if (clicon_session_id_get(h, &session_id) < 0) {
- clicon_err(OE_DAEMON, 0,
- "an ephemeral confirmed-commit was issued, but the session-id could not be determined");
- goto done;
- };
+ if (myid == 0)
+ goto ok;
/* The case of a valid confirming-commit is also handled in the first phase, but only if there is no subsequent
* confirmed-commit. It is tested again here as the case of a valid confirming-commit *with* a subsequent
* confirmed-commit must be handled once the transaction has begun and after all the plugins' validate callbacks
* have been called.
*/
- cc_valid = check_valid_confirming_commit(h, xe, session_id);
+ cc_valid = check_valid_confirming_commit(h, xe, myid);
if (cc_valid) {
if (cancel_rollback_event(h) < 0) {
clicon_err(OE_DAEMON, 0, "A valid confirming-commit was received, but the corresponding rollback event was not found");
@@ -538,7 +540,7 @@ handle_confirmed_commit(clicon_handle h,
/* The client did not pass a value for and therefore any subsequent confirming-commit must be
* issued within the same session.
*/
- confirmed_commit_session_id_set(h, session_id);
+ confirmed_commit_session_id_set(h, myid);
confirmed_commit_state_set(h, EPHEMERAL);
clicon_log(LOG_INFO,
@@ -603,6 +605,7 @@ handle_confirmed_commit(clicon_handle h,
goto done;
}
}
+ ok:
retval = 0;
done:
return retval;
@@ -650,7 +653,7 @@ do_rollback(clicon_handle h,
confirmed_commit_persist_id_set(h, NULL);
}
confirmed_commit_state_set(h, ROLLBACK);
- if (candidate_commit(h, NULL, "rollback", cbret) < 0) { /* Assume validation fail, nofatal */
+ if (candidate_commit(h, NULL, "rollback", 0, cbret) < 0) { /* Assume validation fail, nofatal */
/* theoretically, this should never error, since the rollback database was previously active and therefore
* had itself been previously and successfully committed.
*/
@@ -713,12 +716,14 @@ from_client_cancel_commit(clicon_handle h,
void *arg,
void *regarg)
{
- cxobj *persist_id_xml;
- char *persist_id = NULL;
- uint32_t cur_session_id;
- int retval = -1;
- int rollback = 0;
+ cxobj *persist_id_xml;
+ char *persist_id = NULL;
+ uint32_t myid;
+ int retval = -1;
+ int rollback = 0;
+ struct client_entry *ce = (struct client_entry *)arg;
+ myid = ce->ce_id;
if ((persist_id_xml = xml_find_type(xe, NULL, "persist-id", CX_ELMNT)) != NULL) {
/* persist == persist_id == NULL is legal */
persist_id = xml_body(persist_id_xml);
@@ -729,11 +734,7 @@ from_client_cancel_commit(clicon_handle h,
if (netconf_invalid_value(cbret, "protocol", "current confirmed-commit is not persistent") < 0)
goto done;
}
- else if (clicon_session_id_get(h, &cur_session_id) < 0) {
- if (netconf_invalid_value(cbret, "application", "session-id was not set") < 0)
- goto done;
- }
- else if (cur_session_id != confirmed_commit_session_id_get(h)) {
+ else if (myid != confirmed_commit_session_id_get(h)) {
if (netconf_invalid_value(cbret, "protocol", "confirming-commit must be given within session that gave the confirmed-commit") < 0)
goto done;
}
diff --git a/apps/backend/clixon_backend_commit.h b/apps/backend/clixon_backend_commit.h
index 666f7562..808298d5 100644
--- a/apps/backend/clixon_backend_commit.h
+++ b/apps/backend/clixon_backend_commit.h
@@ -62,7 +62,7 @@ enum confirmed_commit_state confirmed_commit_state_get(clicon_handle h);
uint32_t confirmed_commit_session_id_get(clicon_handle h);
int cancel_rollback_event(clicon_handle h);
int cancel_confirmed_commit(clicon_handle h);
-int handle_confirmed_commit(clicon_handle h, cxobj *xe);
+int handle_confirmed_commit(clicon_handle h, cxobj *xe, uint32_t myid);
int do_rollback(clicon_handle h, uint8_t *errs);
int from_client_cancel_commit(clicon_handle h, cxobj *xe, cbuf *cbret, void *arg, void *regarg);
int from_client_confirmed_commit(clicon_handle h, cxobj *xe, uint32_t myid, cbuf *cbret);
@@ -71,7 +71,7 @@ int from_client_confirmed_commit(clicon_handle h, cxobj *xe, uint32_t myid, cbuf
int startup_validate(clicon_handle h, char *db, cxobj **xtr, cbuf *cbret);
int startup_commit(clicon_handle h, char *db, cbuf *cbret);
int candidate_validate(clicon_handle h, char *db, cbuf *cbret);
-int candidate_commit(clicon_handle h, cxobj *xe, char *db, cbuf *cbret);
+int candidate_commit(clicon_handle h, cxobj *xe, char *db, uint32_t myid, cbuf *cbret);
int from_client_commit(clicon_handle h, cxobj *xe, cbuf *cbret, void *arg, void *regarg);
int from_client_discard_changes(clicon_handle h, cxobj *xe, cbuf *cbret, void *arg, void *regarg);
diff --git a/lib/src/clixon_data.c b/lib/src/clixon_data.c
index daaca13c..8e3c0916 100644
--- a/lib/src/clixon_data.c
+++ b/lib/src/clixon_data.c
@@ -808,12 +808,16 @@ clicon_db_elmnt_set(clicon_handle h,
}
/*! Get session id
+ *
* @param[in] h Clicon handle
* @param[out] sid Session identifier
* @retval 0 OK
* @retval -1 Session id not set
* Session-ids survive TCP sessions that are created for each message sent to the backend.
* The backend assigns session-id for clients: backend assigns, clients get it from backend.
+ * @note a client will get the currrent session-id of that client, BUT
+ * a backend will get the next session-id to be assigned
+ * A backend getting a session-id of an ongoing session should use ce->ce_id
*/
int
clicon_session_id_get(clicon_handle h,
diff --git a/util/clixon_util_validate.c b/util/clixon_util_validate.c
index 906d7a2b..a9a3550a 100644
--- a/util/clixon_util_validate.c
+++ b/util/clixon_util_validate.c
@@ -227,7 +227,7 @@ main(int argc,
goto done;
}
if (commit){
- if ((ret = candidate_commit(h, NULL, database, cb)) < 0)
+ if ((ret = candidate_commit(h, NULL, database, 0, cb)) < 0)
goto done;
}
else{