Merge pull request #1088 from tempesta-tech/ri-sticky-cookie-len-calc…

…ulation

fixups for sticky cookie calculation in case of segmented data

tempesta_fw/http_sess.c

@@ -517,14 +517,20 @@ tfw_http_redir_mark_get(TfwHttpReq *req, TfwStr *out_val)
 #define HEX_STR_TO_BIN_GET(obj, f)					\
 ({									\
 	int count = 0;							\
-	if (!tr && c < end)						\
+	if (c >= end)							\
+		goto end_##f;						\
+	if (!tr)							\
 		tr = c->ptr;						\
-	for ( ; c < end; ++c) {						\
+	for (;;) {							\
 		for ( ; tr < (unsigned char *)c->ptr + c->len; ++tr) {	\
 			if (count++ == sizeof((obj)->f) * 2)		\
 				goto end_##f;				\
 			(obj)->f = ((obj)->f << 4) + hex_to_bin(*tr);	\
 		}							\
+		++c;							\
+		if (c >= end)						\
+			break;						\
+		tr = c->ptr;						\
 	}								\
 end_##f:								\
 	;								\
@@ -533,8 +539,11 @@ end_##f:								\
 #define HEX_STR_TO_BIN_HMAC(hmac, ts, addr)				\
 ({									\
 	unsigned char b;						\
-	int i, hi, r = TFW_HTTP_SESS_SUCCESS;				\
-	for (i = 0, hi = 1; c < end; ++c) {				\
+	int i = 0, hi = 1, r = TFW_HTTP_SESS_SUCCESS;			\
+									\
+	if (c >= end)							\
+		goto end;						\
+	for (;;) {							\
 		for ( ; tr < (unsigned char *)c->ptr + c->len; ++tr) {	\
 			b = hi ? hex_asc_hi((hmac)[i])			\
 			       : hex_asc_lo((hmac)[i]);			\
@@ -552,6 +561,10 @@ end_##f:								\
 			hi = !hi;					\
 			i += hi;					\
 		}							\
+		++c;							\
+		if (c >= end)						\
+			break;						\
+		tr = c->ptr;						\
 	}								\
 	BUG_ON(i != STICKY_KEY_MAXLEN);					\
 end:									\
@@ -696,6 +709,7 @@ tfw_http_sticky_verify(TfwHttpReq *req, TfwStr *value, StickyVal *sv)
 	if (value->len != sizeof(StickyVal) * 2) {
 		sess_warn("bad sticky cookie length", addr, ": %lu(%lu)\n",
 			  value->len, sizeof(StickyVal) * 2);
+		tfw_http_sticky_calc(req, sv);
 		return TFW_HTTP_SESS_VIOLATE;
 	}
 

tempesta_fw/str.c

@@ -140,6 +140,11 @@ void tfw_str_collect_cmp(TfwStr *chunk, TfwStr *end, TfwStr *out,
 
 	BUG_ON(!TFW_STR_PLAIN(chunk));
 
+	if (unlikely(chunk == end)) {
+		bzero_fast(out, sizeof(*out));
+		return;
+	}
+
 	/* If this is last chunk, just return it in this case. */
 	next = chunk + 1;
 	if (likely(next == end || (stop && *(char *)next->ptr == *stop))) {
@@ -149,8 +154,11 @@ void tfw_str_collect_cmp(TfwStr *chunk, TfwStr *end, TfwStr *out,
 
 	/* Add chunks to out-string. */
 	out->ptr = chunk;
-	TFW_STR_CHUNKN_ADD(out, 1);
-	out->len = chunk->len;
+	out->flags = 0;
+	out->len = 0;
+	out->eolen = 0;
+	/* __TFW_STR_CHUNKN_SET(out, 0); is done by out->flags = 0 */
+
 	for (; chunk != end; ++chunk) {
 		if (stop && *(char *)chunk->ptr == *stop)
 			break;

tempesta_fw/t/unit/test_tfw_str.c

@@ -1100,6 +1100,84 @@ TEST(tfw_str_crc32, plain_compound)
 	EXPECT_EQ(crc_pln, crc_cmpnd);
 }
 
+TEST(tfw_str_collect_cmp, collect_chunks)
+{
+	TfwStr in = {
+		.ptr = (TfwStr []){
+			TFW_STR_FROM("abcd"),
+			TFW_STR_FROM("efghi"),
+			TFW_STR_FROM("jklmnopq"),
+			TFW_STR_FROM("rst"),
+			TFW_STR_FROM("uvwxyz")
+		},
+		.len = sizeof("abcdefghijklmnopqrstuvwxyz") - 1,
+		.flags = 5 << TFW_STR_CN_SHIFT
+	};
+	TfwStr *chunks = in.ptr;
+	TfwStr out = { .ptr = (void *)123, .skb = (void *)456, .len = 789,
+	               .eolen = 10, .flags = 1112 };
+
+	tfw_str_collect_cmp(chunks, chunks + 5, &out, NULL);
+	EXPECT_TRUE(tfw_str_eq_cstr(&out, "abcdefghijklmnopqrstuvwxyz", 26, 0));
+	EXPECT_EQ(TFW_STR_CHUNKN(&out), 5);
+	EXPECT_EQ(out.len, 26);
+	/*
+	 * tfw_str_collect_cmp() is expected to clear previous values from all
+	 * other fields of the output TfwStr.
+	 */
+	EXPECT_EQ(out.eolen, 0);
+	EXPECT_EQ(out.flags & TFW_STR_FMASK, 0);
+
+	/*
+	 * Try to start at other chunks too.
+	 * Deliberately not reinitializing 'out' here to check that its previous
+	 * contents is discarded.
+	 */
+	tfw_str_collect_cmp(chunks + 1, chunks + 5, &out, NULL);
+	EXPECT_TRUE(tfw_str_eq_cstr(&out, "efghijklmnopqrstuvwxyz", 22, 0));
+	EXPECT_EQ(TFW_STR_CHUNKN(&out), 4);
+
+	tfw_str_collect_cmp(chunks + 2, chunks + 5, &out, NULL);
+	EXPECT_TRUE(tfw_str_eq_cstr(&out, "jklmnopqrstuvwxyz", 17, 0));
+	EXPECT_EQ(TFW_STR_CHUNKN(&out), 3);
+
+	tfw_str_collect_cmp(chunks + 3, chunks + 5, &out, NULL);
+	EXPECT_TRUE(tfw_str_eq_cstr(&out, "rstuvwxyz", 9, 0));
+	EXPECT_EQ(TFW_STR_CHUNKN(&out), 2);
+
+	tfw_str_collect_cmp(chunks + 4, chunks + 5, &out, NULL);
+	EXPECT_TRUE(tfw_str_eq_cstr(&out, "uvwxyz", 6, 0));
+	/*
+	 * Cutting out one segment should create a plain string, rather than
+	 * a chunked one with a single segment.
+	 */
+	EXPECT_TRUE(TFW_STR_PLAIN(&out));
+
+	/* Empty slice. */
+	tfw_str_collect_cmp(chunks + 4, chunks + 4, &out, NULL);
+	EXPECT_TRUE(tfw_str_eq_cstr(&out, "", 0, 0));
+	EXPECT_TRUE(TFW_STR_PLAIN(&out));
+
+	/* Collecting until a stop character. Two chunks. */
+	tfw_str_collect_cmp(chunks, chunks + 5, &out, "j");
+	EXPECT_TRUE(tfw_str_eq_cstr(&out, "abcdefghi", 9, 0));
+	EXPECT_EQ(TFW_STR_CHUNKN(&out), 2);
+
+	/* Collecing until a stop character. Single chunk. */
+	tfw_str_collect_cmp(chunks + 1, chunks + 5, &out, "j");
+	EXPECT_TRUE(tfw_str_eq_cstr(&out, "efghi", 5, 0));
+	EXPECT_TRUE(TFW_STR_PLAIN(&out));
+
+	/*
+	 * tfw_str_collect_cmp() is expected to check for the stop character
+	 * only at the beginning of each segment. Even if the character appears
+	 * somewhere inside, all segments are expected to be collected.
+	 */
+	tfw_str_collect_cmp(chunks, chunks + 5, &out, "k");
+	EXPECT_TRUE(tfw_str_eq_cstr(&out, "abcdefghijklmnopqrstuvwxyz", 26, 0));
+	EXPECT_EQ(TFW_STR_CHUNKN(&out), 5);
+}
+
 TEST_SUITE(tfw_str)
 {
 	TEST_SETUP(create_str_pool);
@@ -1155,4 +1233,5 @@ TEST_SUITE(tfw_str)
 	TEST_RUN(tfw_str_eq_cstr_off, compound);
 
 	TEST_RUN(tfw_str_crc32, plain_compound);
+	TEST_RUN(tfw_str_collect_cmp, collect_chunks);
 }