Completed initial test for ICE

git-svn-id: http://svn.pjsip.org/repos/pjproject/trunk@1094 74dad513-b988-da41-8d7b-12977e46ad98

5 files changed, 240 insertions, 39 deletions

diff --git a/pjnath/src/pjnath-test/ice.c b/pjnath/src/pjnath-test/ice.c
index a6c19414..d9600fa0 100644
--- a/pjnath/src/pjnath-test/ice.c
+++ b/pjnath/src/pjnath-test/ice.c
@@ -23,10 +23,14 @@
 
 struct ice_data
 {
+    const char	   *obj_name;
     pj_bool_t	    complete;
     pj_status_t	    err_code;
     unsigned	    rx_rtp_cnt;
     unsigned	    rx_rtcp_cnt;
+
+    char	    rx_rtp_data[32];
+    char	    rx_rtcp_data[32];
 };
 
 static pj_stun_config stun_cfg;
@@ -37,6 +41,8 @@ static void on_ice_complete(pj_icemt *icemt,
     struct ice_data *id = (struct ice_data*) icemt->user_data;
     id->complete = PJ_TRUE;
     id->err_code = status;
+    PJ_LOG(3,(THIS_FILE, "    ICE %s complete %s", id->obj_name,
+	      (status==PJ_SUCCESS ? "successfully" : "with failure")));
 }
 
 
@@ -46,7 +52,12 @@ static void on_rx_rtp(pj_icemt *icemt,
 		      unsigned src_addr_len)
 {
     struct ice_data *id = (struct ice_data*) icemt->user_data;
+
     id->rx_rtp_cnt++;
+    pj_memcpy(id->rx_rtp_data, pkt, size);
+
+    PJ_UNUSED_ARG(src_addr);
+    PJ_UNUSED_ARG(src_addr_len);
 }
 
 
@@ -56,7 +67,12 @@ static void on_rx_rtcp(pj_icemt *icemt,
 		       unsigned src_addr_len)
 {
     struct ice_data *id = (struct ice_data*) icemt->user_data;
+
     id->rx_rtcp_cnt++;
+    pj_memcpy(id->rx_rtcp_data, pkt, size);
+
+    PJ_UNUSED_ARG(src_addr);
+    PJ_UNUSED_ARG(src_addr_len);
 }
 
 
@@ -128,15 +144,33 @@ static pj_status_t set_remote_list(pj_icemt *src, pj_icemt *dst)
 }
 
 
-/* Direct agent to agent communication */
-static int ice_direct_test()
+/* Perform ICE test with the following parameters:
+ *
+ * - title:	The title of the test
+ * - ocand_cnt,
+ *   ocand	Additional candidates to be added to offerer
+ * - acand_cnt,
+ *   acand	Additional candidates to be added to answerer
+ *
+ * The additional candidates are invalid candidates, meaning they
+ * won't be reachable by the agents. They are used to "confuse"
+ * ICE processing.
+ */
+static int perform_ice_test(const char *title,
+			    unsigned ocand_cnt,
+			    const pj_ice_cand ocand[],
+			    unsigned acand_cnt,
+			    const pj_ice_cand acand[])
 {
     pj_icemt *im1, *im2;
     pj_icemt_cb icemt_cb;
     struct ice_data *id1, *id2;
+    pj_timestamp t_start, t_end;
+    pj_ice_cand *rcand;
+    unsigned i;
     pj_status_t status;
 
-    PJ_LOG(3,(THIS_FILE, "...direct communication"));
+    PJ_LOG(3,(THIS_FILE, "...%s", title));
 
     pj_bzero(&icemt_cb, sizeof(icemt_cb));
     icemt_cb.on_ice_complete = &on_ice_complete;
@@ -144,27 +178,50 @@ static int ice_direct_test()
     icemt_cb.on_rx_rtcp = &on_rx_rtcp;
 
     /* Create first ICE */
-    status = pj_icemt_create(&stun_cfg, NULL, PJ_ICE_ROLE_CONTROLLING,
+    status = pj_icemt_create(&stun_cfg, "offerer", PJ_ICE_ROLE_CONTROLLING,
 			     &icemt_cb, 0, PJ_FALSE, PJ_FALSE, NULL, &im1);
     if (status != PJ_SUCCESS)
 	return -20;
 
     id1 = PJ_POOL_ZALLOC_T(im1->pool, struct ice_data);
+    id1->obj_name = "offerer";
     im1->user_data = id1;
 
+    /* Add additional candidates */
+    for (i=0; i<ocand_cnt; ++i) {
+	status = pj_ice_add_cand(im1->ice, 1, ocand[i].type, 65535,
+				 &ocand[i].foundation, &ocand[i].addr,
+				 &ocand[i].base_addr, &ocand[i].srv_addr,
+				 sizeof(pj_sockaddr_in), NULL);
+	if (status != PJ_SUCCESS)
+	    return -22;
+    }
+
     /* Create second ICE */
-    status = pj_icemt_create(&stun_cfg, NULL, PJ_ICE_ROLE_CONTROLLED,
+    status = pj_icemt_create(&stun_cfg, "answerer", PJ_ICE_ROLE_CONTROLLED,
 			     &icemt_cb, 0, PJ_FALSE, PJ_FALSE, NULL, &im2);
     if (status != PJ_SUCCESS)
 	return -25;
 
     id2 = PJ_POOL_ZALLOC_T(im2->pool, struct ice_data);
+    id2->obj_name = "answerer";
     im2->user_data = id2;
 
+    /* Add additional candidates */
+    for (i=0; i<acand_cnt; ++i) {
+	status = pj_ice_add_cand(im1->ice, 1, acand[i].type, 65535,
+				 &acand[i].foundation, &acand[i].addr,
+				 &acand[i].base_addr, &acand[i].srv_addr,
+				 sizeof(pj_sockaddr_in), NULL);
+	if (status != PJ_SUCCESS)
+	    return -22;
+    }
+
+    /* Set credentials */
     {
-	pj_str_t u1 = pj_str("uname1");
+	pj_str_t u1 = pj_str("offerer");
 	pj_str_t p1 = pj_str("pass1");
-	pj_str_t u2 = pj_str("uname2");
+	pj_str_t u2 = pj_str("answerer");
 	pj_str_t p2 = pj_str("pass2");
 
 	pj_ice_set_credentials(im1->ice, &u1, &p1, &u2, &p2);
@@ -181,23 +238,76 @@ static int ice_direct_test()
     if (status != PJ_SUCCESS)
 	return -35;
 
+    /* Mark start time */
+    pj_get_timestamp(&t_start);
+
     /* Both can start now */
     status = pj_ice_start_check(im1->ice);
     if (status != PJ_SUCCESS)
 	return -40;
 
-#if 0
+#if 1
     status = pj_ice_start_check(im2->ice);
     if (status != PJ_SUCCESS)
-	return -40;
+	return -45;
 #endif
 
     /* Just wait until both completes, or timed out */
-    while (!id1->complete || !id2->complete)
+    while (!id1->complete || !id2->complete) {
+	pj_timestamp t_now;
+
 	handle_events(1);
 
-    return 0;
+	pj_get_timestamp(&t_now);
+	if (pj_elapsed_msec(&t_start, &t_now) >= 10000) {
+	    PJ_LOG(3,(THIS_FILE, "....error: timed-out"));
+	    return -50;
+	}
+    }
+
+    /* Mark end-time */
+    pj_get_timestamp(&t_end);
 
+    /* Check status */
+    if (id1->err_code != PJ_SUCCESS)
+	return -53;
+    if (id2->err_code != PJ_SUCCESS)
+	return -56;
+
+    /* Verify that offerer gets answerer's transport address */
+    rcand = im1->ice->clist.checks[im1->ice->comp[0].nominated_check_id].rcand;
+    if (pj_memcmp(&rcand->addr, &im2->ice->lcand[0].addr, sizeof(pj_sockaddr_in))!=0) {
+	PJ_LOG(3,(THIS_FILE, "....error: address mismatch"));
+	return -60;
+    }
+
+    /* And the other way around */
+    rcand = im2->ice->clist.checks[im2->ice->comp[0].nominated_check_id].rcand;
+    if (pj_memcmp(&rcand->addr, &im1->ice->lcand[0].addr, sizeof(pj_sockaddr_in))!=0) {
+	PJ_LOG(3,(THIS_FILE, "....error: address mismatch"));
+	return -70;
+    }
+
+    /* Done */
+    PJ_LOG(3,(THIS_FILE, "....success: ICE completed in %d msec", 
+	      pj_elapsed_msec(&t_start, &t_end)));
+
+    /* Wait for some more time */
+    PJ_LOG(3,(THIS_FILE, ".....waiting.."));
+    for (;;) {
+	pj_timestamp t_now;
+
+	pj_get_timestamp(&t_now);
+	if (pj_elapsed_msec(&t_end, &t_now) > 10000)
+	    break;
+
+	handle_events(1);
+    }
+
+
+    pj_icemt_destroy(im1);
+    pj_icemt_destroy(im2);
+    return 0;
 }
 
 
@@ -207,7 +317,10 @@ int ice_test(void)
     pj_pool_t *pool;
     pj_ioqueue_t *ioqueue;
     pj_timer_heap_t *timer_heap;
-    
+    pj_ice_cand ocand[PJ_ICE_MAX_CAND];
+    pj_ice_cand acand[PJ_ICE_MAX_CAND];
+    pj_str_t s;
+
     pool = pj_pool_create(mem, NULL, 4000, 4000, NULL);
     pj_ioqueue_create(pool, 12, &ioqueue);
     pj_timer_heap_create(pool, 100, &timer_heap);
@@ -216,14 +329,29 @@ int ice_test(void)
 
     pj_log_set_level(5);
 
+    /* Basic create/destroy */
     rc = ice_basic_create_destroy_test();
     if (rc != 0)
 	goto on_return;
 
-    rc = ice_direct_test();
+    /* Direct communication */
+    rc = perform_ice_test("Direct connection", 0, NULL, 0, NULL);
     if (rc != 0)
 	goto on_return;
 
+    /* Direct communication with invalid address */
+    pj_bzero(ocand, sizeof(ocand));
+    pj_sockaddr_in_init(&ocand[0].addr.ipv4, pj_cstr(&s, "127.0.0.127"), 1234);
+    pj_sockaddr_in_init(&ocand[0].base_addr.ipv4, pj_cstr(&s, "127.0.0.128"), 1234);
+    ocand[0].comp_id = 1;
+    ocand[0].foundation = pj_str("H2");
+    ocand[0].type = PJ_ICE_CAND_TYPE_HOST;
+
+    rc = perform_ice_test("Direct connection with 1 invalid address", 1, ocand, 0, NULL);
+    if (rc != 0)
+	goto on_return;
+
+
 on_return:
     pj_log_set_level(3);
     pj_ioqueue_destroy(stun_cfg.ioqueue);
diff --git a/pjnath/src/pjnath-test/main.c b/pjnath/src/pjnath-test/main.c
index 33583e30..452c5c30 100644
--- a/pjnath/src/pjnath-test/main.c
+++ b/pjnath/src/pjnath-test/main.c
@@ -48,6 +48,11 @@ int main(int argc, char *argv[])
 
     rc = test_main();
 
+    if (argc == 2 && pj_ansi_strcmp(argv[1], "-i")==0) {
+	char buf[10];
+	fgets(buf, sizeof(buf), stdin);
+    }
+
     return rc;
 }
 
diff --git a/pjnath/src/pjnath-test/test.c b/pjnath/src/pjnath-test/test.c
index 6cc829d5..3dc2085d 100644
--- a/pjnath/src/pjnath-test/test.c
+++ b/pjnath/src/pjnath-test/test.c
@@ -49,9 +49,11 @@ static int test_inner(void)
 
     mem = &caching_pool.factory;
 
+#if 0
     pj_log_set_level(3);
     pj_log_set_decor(PJ_LOG_HAS_NEWLINE | PJ_LOG_HAS_TIME | 
                      PJ_LOG_HAS_MICRO_SEC);
+#endif
 
     rc = pj_init();
     if (rc != 0) {
diff --git a/pjnath/src/pjnath/ice.c b/pjnath/src/pjnath/ice.c
index b0ba0f3d..de781706 100644
--- a/pjnath/src/pjnath/ice.c
+++ b/pjnath/src/pjnath/ice.c
@@ -660,6 +660,7 @@ static pj_uint64_t CALC_CHECK_PRIO(const pj_ice *ice,
 }
 
 static const char *dump_check(char *buffer, unsigned bufsize,
+			      const pj_ice *ice,
 			      const pj_ice_check *check)
 {
     const pj_ice_cand *lcand = check->lcand;
@@ -671,7 +672,8 @@ static const char *dump_check(char *buffer, unsigned bufsize,
 
     if (lcand->addr.addr.sa_family == PJ_AF_INET) {
 	len = pj_ansi_snprintf(buffer, bufsize,
-			       "%s:%d-->%s:%d",
+			       "%d: %s:%d-->%s:%d",
+			       GET_CHECK_ID(check),
 			       laddr, (int)pj_ntohs(lcand->addr.ipv4.sin_port),
 			       pj_inet_ntoa(rcand->addr.ipv4.sin_addr),
 			       (int)pj_ntohs(rcand->addr.ipv4.sin_port));
@@ -699,11 +701,28 @@ static void dump_checklist(const char *title, const pj_ice *ice,
     LOG((ice->obj_name, "%s", title));
     for (i=0; i<clist->count; ++i) {
 	const pj_ice_check *c = &clist->checks[i];
-	LOG((ice->obj_name, " %d: %s (prio=0x%"PJ_INT64_FMT"x, state=%s)",
-	     i, dump_check(buffer, sizeof(buffer), c),
-	     c->prio, check_state_name[c->state]));
+	LOG((ice->obj_name, " %s (%s, state=%s)",
+	     dump_check(buffer, sizeof(buffer), ice, c),
+	     (c->nominated ? "nominated" : "not nominated"), 
+	     check_state_name[c->state]));
     }
 }
+
+static void dump_valid_list(const char *title, const pj_ice *ice)
+{
+    unsigned i;
+    char buffer[CHECK_NAME_LEN];
+
+    LOG((ice->obj_name, "%s", title));
+    for (i=0; i<ice->valid_cnt; ++i) {
+	const pj_ice_check *c = &ice->clist.checks[ice->valid_list[i]];
+	LOG((ice->obj_name, " %s (%s, state=%s)",
+	     dump_check(buffer, sizeof(buffer), ice, c),
+	     (c->nominated ? "nominated" : "not nominated"), 
+	     check_state_name[c->state]));
+    }
+}
+
 #else
 #define dump_checklist(ice, clist)
 #endif
@@ -713,8 +732,11 @@ static void check_set_state(pj_ice *ice, pj_ice_check *check,
 			    pj_status_t err_code)
 {
     char buf[CHECK_NAME_LEN];
+
+    pj_assert(check->state < PJ_ICE_CHECK_STATE_SUCCEEDED);
+
     LOG((ice->obj_name, "Check %s: state changed from %s to %s",
-	 dump_check(buf, sizeof(buf), check),
+	 dump_check(buf, sizeof(buf), ice, check),
 	 check_state_name[check->state],
 	 check_state_name[st]));
     check->state = st;
@@ -724,10 +746,12 @@ static void check_set_state(pj_ice *ice, pj_ice_check *check,
 static void clist_set_state(pj_ice *ice, pj_ice_checklist *clist,
 			    pj_ice_checklist_state st)
 {
-    LOG((ice->obj_name, "Checklist: state changed from %s to %s",
-	 clist_state_name[clist->state],
-	 clist_state_name[st]));
-    clist->state = st;
+    if (clist->state != st) {
+	LOG((ice->obj_name, "Checklist: state changed from %s to %s",
+	     clist_state_name[clist->state],
+	     clist_state_name[st]));
+	clist->state = st;
+    }
 }
 
 /* Sort checklist based on priority */
@@ -840,9 +864,9 @@ static void prune_checklist(pj_ice *ice, pj_ice_checklist *clist)
 	    const pj_sockaddr *ljaddr;
 
 	    if (ljcand->type == PJ_ICE_CAND_TYPE_SRFLX)
-		ljaddr = &licand->base_addr;
+		ljaddr = &ljcand->base_addr;
 	    else
-		ljaddr = &licand->addr;
+		ljaddr = &ljcand->addr;
 
 	    if (sockaddr_cmp(liaddr, ljaddr) == SOCKADDR_EQUAL &&
 		sockaddr_cmp(&ricand->addr, &rjcand->addr) == SOCKADDR_EQUAL)
@@ -851,7 +875,7 @@ static void prune_checklist(pj_ice *ice, pj_ice_checklist *clist)
 		char buf[CHECK_NAME_LEN];
 
 		LOG((ice->obj_name, "Check %s pruned",
-		    dump_check(buf, sizeof(buf), &clist->checks[j])));
+		    dump_check(buf, sizeof(buf), ice, &clist->checks[j])));
 
 		pj_array_erase(clist->checks, sizeof(clist->checks[0]),
 			       clist->count, j);
@@ -871,6 +895,11 @@ static void on_ice_complete(pj_ice *ice, pj_status_t status)
 
 	ice->is_complete = PJ_TRUE;
 	ice->ice_status = status;
+    
+	/* Log message */
+	LOG((ice->obj_name, "ICE process complete"));
+	dump_checklist("Dumping checklist", ice, &ice->clist);
+	dump_valid_list("Dumping valid list", ice);
 
 	/* Call callback */
 	(*ice->cb.on_ice_complete)(ice, status);
@@ -898,13 +927,19 @@ static pj_bool_t on_check_complete(pj_ice *ice,
     if (check->err_code==PJ_SUCCESS && check->nominated) {
 	pj_ice_comp *comp;
 
+	LOG((ice->obj_name, "Check %d is successful and nominated",
+	     GET_CHECK_ID(check)));
+
 	for (i=0; i<ice->clist.count; ++i) {
 	    pj_ice_check *c = &ice->clist.checks[i];
 	    if (c->lcand->comp_id == check->lcand->comp_id &&
 		(c->state==PJ_ICE_CHECK_STATE_FROZEN ||
 		 c->state==PJ_ICE_CHECK_STATE_WAITING))
 	    {
-		check_set_state(ice, check, PJ_ICE_CHECK_STATE_FAILED,
+		LOG((ice->obj_name, 
+		     "Check %d to be failed because state is %s",
+		     i, check_state_name[c->state]));
+		check_set_state(ice, c, PJ_ICE_CHECK_STATE_FAILED,
 				PJ_ECANCELLED);
 	    }
 	}
@@ -952,6 +987,7 @@ static pj_bool_t on_check_complete(pj_ice *ice,
     /* TODO */
 
 
+#if 0
     /* For now, just see if we have a valid pair in component 1 and
      * just terminate ICE.
      */
@@ -966,9 +1002,23 @@ static pj_bool_t on_check_complete(pj_ice *ice,
 	on_ice_complete(ice, PJ_SUCCESS);
 	return PJ_TRUE;
     }
+#else
+    /* See if all components have nominated pair. If they do, then mark
+     * ICE processing as success, otherwise wait.
+     */
+    for (i=0; i<ice->comp_cnt; ++i) {
+	if (ice->comp[i].nominated_check_id == -1)
+	    break;
+    }
+    if (i == ice->comp_cnt) {
+	/* All components have nominated pair */
+	on_ice_complete(ice, PJ_SUCCESS);
+	return PJ_TRUE;
+    }
+#endif
 
-    /* We don't have valid pair for component 1.
-     * See if we have performed all checks in the checklist. If we do,
+    /* 
+     * See if all checks in the checklist have completed. If we do,
      * then mark ICE processing as failed.
      */
     for (i=0; i<ice->clist.count; ++i) {
@@ -1100,8 +1150,8 @@ static pj_status_t perform_check(pj_ice *ice, pj_ice_checklist *clist,
     comp = find_comp(ice, lcand->comp_id);
 
     LOG((ice->obj_name, 
-	 "Sending connectivity check for check %d: %s", 
-	 check_id, dump_check(buffer, sizeof(buffer), check)));
+	 "Sending connectivity check for check %s", 
+	 dump_check(buffer, sizeof(buffer), ice, check)));
 
     /* Create request */
     status = pj_stun_session_create_req(comp->stun_sess, 
@@ -1315,12 +1365,14 @@ static void on_stun_request_complete(pj_stun_session *stun_sess,
     rcand = check->rcand;
 
     LOG((ice->obj_name, 
-	 "Connectivity check %s for check %s",
-	 (status==PJ_SUCCESS ? "SUCCESS" : "FAILED"), 
-	 dump_check(buffer, sizeof(buffer), check)));
+	 "Check %s%s: connectivity check %s",
+	 dump_check(buffer, sizeof(buffer), ice, check),
+	 (check->nominated ? " (nominated)" : " (not nominated)"),
+	 (status==PJ_SUCCESS ? "SUCCESS" : "FAILED")));
 
     if (status != PJ_SUCCESS) {
 	check_set_state(ice, check, PJ_ICE_CHECK_STATE_FAILED, status);
+	on_check_complete(ice, check);
 	pj_mutex_unlock(ice->mutex);
 	return;
     }
@@ -1339,6 +1391,7 @@ static void on_stun_request_complete(pj_stun_session *stun_sess,
     if (!xaddr) {
 	check_set_state(ice, check, PJ_ICE_CHECK_STATE_FAILED, 
 			PJNATH_ESTUNNOXORMAP);
+	on_check_complete(ice, check);
 	pj_mutex_unlock(ice->mutex);
 	return;
     }
@@ -1365,6 +1418,7 @@ static void on_stun_request_complete(pj_stun_session *stun_sess,
 				 sizeof(pj_sockaddr_in), &cand_id);
 	if (status != PJ_SUCCESS) {
 	    check_set_state(ice, check, PJ_ICE_CHECK_STATE_FAILED, status);
+	    on_check_complete(ice, check);
 	    pj_mutex_unlock(ice->mutex);
 	    return;
 	}
@@ -1547,6 +1601,12 @@ static pj_status_t on_stun_rx_request(pj_stun_session *sess,
 	rcand->foundation.slen = pj_ansi_snprintf(rcand->foundation.ptr, 32,
 						  "f%p", 
 						  rcand->foundation.ptr);
+
+	LOG((ice->obj_name, 
+	     "Added new remote candidate from the request: %s:%d",
+	     pj_inet_ntoa(rcand->addr.ipv4.sin_addr),
+	     (int)pj_ntohs(rcand->addr.ipv4.sin_port)));
+
     } else {
 	/* Remote candidate found */
 	rcand = &ice->rcand[i];
@@ -1606,8 +1666,10 @@ static pj_status_t on_stun_rx_request(pj_stun_session *sess,
     if (i != ice->clist.count) {
 	pj_ice_check *c = &ice->clist.checks[i];
 
-	/* If USE-CANDIDATE is present, set nominated flag */
-	c->nominated = (uc != NULL);
+	/* If USE-CANDIDATE is present, set nominated flag 
+	 * Note: DO NOT overwrite nominated flag if one is already set.
+	 */
+	c->nominated = ((uc != NULL) || c->nominated);
 
 	if (c->state == PJ_ICE_CHECK_STATE_FROZEN ||
 	    c->state == PJ_ICE_CHECK_STATE_WAITING)
@@ -1619,12 +1681,17 @@ static pj_status_t on_stun_rx_request(pj_stun_session *sess,
 	    /* Should retransmit here, but how??
 	     * TODO
 	     */
+	    LOG((ice->obj_name, "Triggered check for check %d not performed "
+				"because it's in progress", i));
 	} else if (c->state == PJ_ICE_CHECK_STATE_SUCCEEDED) {
 	    /* Check complete for this component.
 	     * Note this may end ICE process.
 	     */
 	    pj_bool_t complete;
 
+	    LOG((ice->obj_name, "Triggered check for check %d not performed "
+				"because it's completed", i));
+
 	    complete = on_check_complete(ice, c);
 	    if (complete) {
 		pj_mutex_unlock(ice->mutex);
diff --git a/pjnath/src/pjnath/stun_msg.c b/pjnath/src/pjnath/stun_msg.c
index 9b548c52..2fdd9027 100644
--- a/pjnath/src/pjnath/stun_msg.c
+++ b/pjnath/src/pjnath/stun_msg.c
@@ -865,7 +865,7 @@ pj_stun_empty_attr_create(pj_pool_t *pool,
     PJ_ASSERT_RETURN(pool && p_attr, PJ_EINVAL);
 
     attr = PJ_POOL_ZALLOC_T(pool, pj_stun_empty_attr);
-    INIT_ATTR(attr, attr_type, sizeof(pj_stun_empty_attr));
+    INIT_ATTR(attr, attr_type, 0);
 
     *p_attr = attr;
 
@@ -908,7 +908,7 @@ static pj_status_t decode_empty_attr(pj_pool_t *pool,
     attr->hdr.length = pj_ntohs(attr->hdr.length);
 
     /* Check that the attribute length is valid */
-    if (attr->hdr.length != ATTR_HDR_LEN)
+    if (attr->hdr.length != 0)
 	return PJNATH_ESTUNINATTRLEN;
 
     /* Done */
@@ -930,8 +930,7 @@ static pj_status_t encode_empty_attr(const void *a, pj_uint8_t *buf,
     pj_memcpy(buf, a, ATTR_HDR_LEN);
     attr = (pj_stun_empty_attr*) buf;
     attr->hdr.type = pj_htons(attr->hdr.type);
-    pj_assert(attr->hdr.length == ATTR_HDR_LEN);
-    attr->hdr.length = pj_htons(ATTR_HDR_LEN);
+    attr->hdr.length = 0;
 
     /* Done */
     *printed = ATTR_HDR_LEN;