Fix crash #1450
Fix crash #1450
Conversation
tempesta_fw/cache.c
Outdated
| @@ -996,7 +996,7 @@ __tfw_cache_strcpy(char **p, TdbVRec **trec, TfwStr *src, size_t tot_len, | |||
| { | |||
| long copied = 0; | |||
|
|
|||
| while (copied < src->len) { | |||
| while (copied < src->len && tot_len - copied) { | |||
There was a problem hiding this comment.
The condition is strange and masks issue root. Why a new string to be copied to cache could be bigger than response length in total? This is the question that needs to be fixed.
This workaround messes up the cache entry and the response is never cached and always forwarded from origin.
tempesta_fw/http_msg.c
Outdated
| @@ -1418,7 +1418,7 @@ tfw_h2_msg_rewrite_data(TfwHttpTransIter *mit, const TfwStr *str, | |||
| return -E2BIG; | |||
| } | |||
|
|
|||
| memcpy_fast(mit->curr_ptr, c->data + c_off, n_copy); | |||
| tfw_cstrtolower(mit->curr_ptr, c->data + c_off, n_copy); | |||
There was a problem hiding this comment.
With this change not only header names are converted to lowercase, but also header values. Such behaviour in unacceptable and harmful.
header, also, if there are duplicate headers, then we must write the duplicate header. earlier in the presence of duplicates, there was a double shift at the beginning of the copying procedure CSTR_MOVE_HDR CSTR_WRITE_HDR *CSTR_MOVE_HDR* *CSTR_MOVE_HDR* CSTR_WRITE_HDR
at each iteration in the loop above, there is already accumulation of duplicate lengths in dc_iter.acc_len
There was a problem hiding this comment.
Unfortunately all the tests from test_hpack.c has passed on this branch without any change. I say unfortunately because we has the issues with hpack encoding and the PR fixes them while all the tests are green in both cases. Thus the tests must be extended to catch the issue.
| * deleting a node with less than two children. | ||
| */ | ||
| if (was_del) { | ||
| res = tfw_hpack_rbtree_find(tbl, hdr, &node, place); |
There was a problem hiding this comment.
I see, there is a case, when place can become invalid, but the proposed change is expensive: it possibly involves a lot of string compare operations. It seems that tfw_hpack_rbtree_erase() should be updated to keep place valid.
otherwise, at the next iteration, a mismatch occurs
otherwise, at the next iteration, a mismatch occurs
can change in relation to the parent
because the old place may be invalid due to the reducing of the procedure for deleting a node with two children to deleting a node with less than two children
fix #1408
Earlier in the presence of duplicates, there was a double shift at the beginning of the copying procedure:
CSTR_MOVE_HDR
CSTR_WRITE_HDR
CSTR_MOVE_HDR
CSTR_MOVE_HDR
CSTR_WRITE_HDR
eliminated the progressive accumulation of duplicate lengths
some fixes for rebalancing procedure
if hpack rb-tree node was deleted, then we look for a new place to add a node, because the old place may be invalid due to the reducing of the procedure for deleting a node with two children to deleting a node with less than two children