1.12.3: [BUGFIX] Fix duplicate STREAM frame detection

This commit is contained in:
Dmitri Tikhonov 2018-08-22 16:46:31 -04:00
parent 483646ebc3
commit 5f5d395b59
6 changed files with 384 additions and 3 deletions

View File

@ -1,3 +1,8 @@
2018-08-22
- 1.12.3
- [BUGFIX] Fix duplicate STREAM frame detection
2018-08-20
- 1.12.2

View File

@ -25,7 +25,7 @@ extern "C" {
#define LSQUIC_MAJOR_VERSION 1
#define LSQUIC_MINOR_VERSION 12
#define LSQUIC_PATCH_VERSION 2
#define LSQUIC_PATCH_VERSION 3
/**
* Engine flags:

View File

@ -196,6 +196,11 @@ frame_list_is_sane (const struct nocopy_data_in *ncdi)
#define CASE(letter) ((int) (letter) << 8)
/* Not all errors are picked up by this function, as it is expensive (and
* potentially error-prone) to check for all possible error conditions.
* It an error be misclassified as an overlap or dup, in the worst case
* we end up with an application error instead of protocol violation.
*/
static int
insert_frame (struct nocopy_data_in *ncdi, struct stream_frame *new_frame,
uint64_t read_offset, unsigned *p_n_frames)
@ -217,6 +222,13 @@ insert_frame (struct nocopy_data_in *ncdi, struct stream_frame *new_frame,
return INS_FRAME_ERR | CASE('C');
if (DF_END(new_frame) > ncdi->ncdi_fin_off)
return INS_FRAME_ERR | CASE('D');
if (read_offset == DF_END(new_frame))
return INS_FRAME_DUP | CASE('M');
}
else
{
if (read_offset == DF_END(new_frame) && !DF_FIN(new_frame))
return INS_FRAME_DUP | CASE('L');
}
/* Find position in the list, going backwards. We go backwards because
@ -401,9 +413,17 @@ nocopy_di_get_frame (struct data_in *data_in, uint64_t read_offset)
struct stream_frame *frame = TAILQ_FIRST(&ncdi->ncdi_frames_in);
if (frame && frame->data_frame.df_offset +
frame->data_frame.df_read_off == read_offset)
{
LSQ_DEBUG("get_frame: frame (off: %"PRIu64", size: %u, fin: %d), at "
"read offset %"PRIu64, DF_OFF(frame), DF_SIZE(frame), DF_FIN(frame),
read_offset);
return &frame->data_frame;
}
else
{
LSQ_DEBUG("get_frame: no frame at read offset %"PRIu64, read_offset);
return NULL;
}
}
@ -424,6 +444,8 @@ nocopy_di_frame_done (struct data_in *data_in, struct data_frame *data_frame)
ncdi->ncdi_flags |= NCDI_FIN_REACHED;
LSQ_DEBUG("FIN has been reached at offset %"PRIu64, DF_END(frame));
}
LSQ_DEBUG("frame (off: %"PRIu64", size: %u, fin: %d) done",
DF_OFF(frame), DF_SIZE(frame), DF_FIN(frame));
lsquic_packet_in_put(ncdi->ncdi_conn_pub->mm, frame->packet_in);
lsquic_malo_put(frame);
}

View File

@ -221,6 +221,10 @@ add_executable(test_cubic test_cubic.c)
target_link_libraries(test_cubic lsquic pthread libssl.a libcrypto.a m ${LIBS})
add_test(cubic test_cubic)
add_executable(test_di_nocopy test_di_nocopy.c)
target_link_libraries(test_di_nocopy lsquic pthread libssl.a libcrypto.a m ${LIBS})
add_test(di_nocopy test_di_nocopy)
add_executable(test_dec test_dec.c)
target_link_libraries(test_dec libssl.a libcrypto.a z m pthread ${LIBS})
@ -437,6 +441,10 @@ add_executable(test_cubic test_cubic.c ../../wincompat/getopt.c ../../wincompat/
target_link_libraries(test_cubic lsquic ${LIBS_LIST})
add_test(cubic test_cubic)
add_executable(test_di_nocopy test_di_nocopy.c)
target_link_libraries(test_di_nocopy lsquic pthread libssl.a libcrypto.a m ${LIBS})
add_test(di_nocopy test_di_nocopy)
add_executable(test_dec test_dec.c ../../wincompat/getopt.c ../../wincompat/getopt1.c)
target_link_libraries(test_dec ${LIBS_LIST})

View File

@ -0,0 +1,333 @@
/* Copyright (c) 2017 - 2018 LiteSpeed Technologies Inc. See LICENSE. */
/*
* Test the "nocopy" data in stream
*/
#include <assert.h>
#include <stdio.h>
#include <string.h>
#include <sys/queue.h>
#include <unistd.h>
#include "lsquic.h"
#include "lsquic_int_types.h"
#include "lsquic_sfcw.h"
#include "lsquic_rtt.h"
#include "lsquic_conn_flow.h"
#include "lsquic_stream.h"
#include "lsquic_conn.h"
#include "lsquic_conn_public.h"
#include "lsquic_malo.h"
#include "lsquic_packet_common.h"
#include "lsquic_packet_in.h"
#include "lsquic_packet_out.h"
#include "lsquic_mm.h"
#include "lsquic_logger.h"
#include "lsquic_data_in_if.h"
struct nocopy_test
{
int lineno;
/* Setup: initial set of frames to insert and read until some offset */
unsigned n_init_frames;
struct data_frame initial_frames[5];
unsigned read_until;
/* Test: data frame to insert and expected insert result */
struct data_frame data_frame;
enum ins_frame ins;
};
#define F(off, size, fin) { .df_offset = (off), .df_fin = (fin), .df_size = (size), }
static const struct nocopy_test tests[] =
{
{ .lineno = __LINE__,
.n_init_frames = 1,
.initial_frames = { F(0, 300, 0), },
.read_until = 300,
.data_frame = F(200, 100, 0),
.ins = INS_FRAME_DUP,
},
{ .lineno = __LINE__,
.n_init_frames = 2,
.initial_frames = { F(0, 300, 0), F(300, 100, 0), },
.read_until = 300,
.data_frame = F(200, 100, 0),
.ins = INS_FRAME_DUP,
},
{ .lineno = __LINE__,
.n_init_frames = 2,
.initial_frames = { F(0, 300, 0), F(300, 0, 1), },
.read_until = 300,
.data_frame = F(200, 100, 1),
.ins = INS_FRAME_DUP,
},
{ .lineno = __LINE__,
.n_init_frames = 1,
.initial_frames = { F(0, 301, 0), },
.read_until = 301,
.data_frame = F(200, 100, 1),
.ins = INS_FRAME_ERR,
},
{ .lineno = __LINE__,
.n_init_frames = 1,
.initial_frames = { F(0, 400, 0), },
.read_until = 301,
.data_frame = F(200, 100, 0),
.ins = INS_FRAME_DUP,
},
{ .lineno = __LINE__,
.n_init_frames = 1,
.initial_frames = { F(200, 100, 1), },
.read_until = 0,
.data_frame = F(200, 50, 1),
.ins = INS_FRAME_ERR,
},
{ .lineno = __LINE__,
.n_init_frames = 1,
.initial_frames = { F(200, 100, 1), },
.read_until = 0,
.data_frame = F(200, 150, 1),
.ins = INS_FRAME_ERR,
},
{ .lineno = __LINE__,
.n_init_frames = 1,
.initial_frames = { F(200, 100, 1), },
.read_until = 0,
.data_frame = F(200, 101, 0),
.ins = INS_FRAME_ERR,
},
{ .lineno = __LINE__,
.n_init_frames = 1,
.initial_frames = { F(200, 100, 1), },
.read_until = 0,
.data_frame = F(500, 1, 0),
.ins = INS_FRAME_ERR,
},
{ .lineno = __LINE__,
.n_init_frames = 1,
.initial_frames = { F(0, 100, 0), },
.read_until = 100,
.data_frame = F(0, 100, 1),
.ins = INS_FRAME_OVERLAP,
},
{ .lineno = __LINE__,
.n_init_frames = 1,
.initial_frames = { F(0, 100, 1), },
.read_until = 100,
.data_frame = F(0, 100, 1),
.ins = INS_FRAME_DUP,
},
/* TODO: Case 'F' and 'L' -- remove "case 'F'" */
{ .lineno = __LINE__,
.n_init_frames = 1,
.initial_frames = { F(0, 100, 0), },
.read_until = 100,
.data_frame = F(0, 100, 0),
.ins = INS_FRAME_DUP,
},
{ .lineno = __LINE__,
.n_init_frames = 1,
.initial_frames = { F(0, 100, 1), },
.read_until = 10,
.data_frame = F(0, 100, 0),
.ins = INS_FRAME_DUP,
},
{ .lineno = __LINE__,
.n_init_frames = 1,
.initial_frames = { F(0, 100, 0), },
.read_until = 10,
.data_frame = F(0, 100, 1),
.ins = INS_FRAME_OVERLAP,
},
{ .lineno = __LINE__,
.n_init_frames = 1,
.initial_frames = { F(0, 100, 0), },
.read_until = 100,
.data_frame = F(100, 0, 0),
.ins = INS_FRAME_DUP,
},
{ .lineno = __LINE__,
.n_init_frames = 1,
.initial_frames = { F(0, 100, 0), },
.read_until = 0,
.data_frame = F(50, 100, 0),
.ins = INS_FRAME_OVERLAP,
},
{ .lineno = __LINE__,
.n_init_frames = 1,
.initial_frames = { F(0, 100, 1), },
.read_until = 0,
.data_frame = F(50, 100, 0),
.ins = INS_FRAME_ERR,
},
{ .lineno = __LINE__,
.n_init_frames = 1,
.initial_frames = { F(100, 100, 0), },
.read_until = 0,
.data_frame = F(50, 100, 0),
.ins = INS_FRAME_OVERLAP,
},
{ .lineno = __LINE__,
.n_init_frames = 1,
.initial_frames = { F(100, 100, 0), },
.read_until = 0,
.data_frame = F(50, 100, 1),
.ins = INS_FRAME_OVERLAP, /* This is really an error,
* but we ignore it.
*/
},
{ .lineno = __LINE__,
.n_init_frames = 1,
.initial_frames = { F(100, 100, 1), },
.read_until = 0,
.data_frame = F(50, 100, 0),
.ins = INS_FRAME_OVERLAP,
},
{ .lineno = __LINE__,
.n_init_frames = 1,
.initial_frames = { F(0, 100, 1), },
.read_until = 60,
.data_frame = F(50, 2, 0),
.ins = INS_FRAME_DUP,
},
{ .lineno = __LINE__,
.n_init_frames = 2,
.initial_frames = { F(0, 100, 0), F(200, 100, 0), },
.read_until = 0,
.data_frame = F(50, 200, 0),
.ins = INS_FRAME_OVERLAP,
},
{ .lineno = __LINE__,
.n_init_frames = 2,
.initial_frames = { F(0, 100, 0), F(200, 100, 0), },
.read_until = 0,
.data_frame = F(100, 100, 0),
.ins = INS_FRAME_OK,
},
{ .lineno = __LINE__,
.n_init_frames = 2,
.initial_frames = { F(0, 100, 0), F(200, 100, 0), },
.read_until = 0,
.data_frame = F(100, 100, 1),
.ins = INS_FRAME_OK, /* Ignore another error */
},
};
static void
run_di_nocopy_test (const struct nocopy_test *test)
{
struct lsquic_mm mm;
struct lsquic_conn_public conn_pub;
struct lsquic_conn conn;
struct stream_frame *frame;
struct data_in *di;
struct data_frame *data_frame;
enum ins_frame ins;
unsigned i;
unsigned nread, n_to_read;
LSQ_NOTICE("running test on line %d", test->lineno);
lsquic_mm_init(&mm);
memset(&conn, 0, sizeof(conn));
conn_pub.lconn = &conn;
conn_pub.mm = &mm;
di = data_in_nocopy_new(&conn_pub, 3);
for (i = 0; i < test->n_init_frames; ++i)
{
frame = lsquic_malo_get(mm.malo.stream_frame);
frame->packet_in = lsquic_mm_get_packet_in(&mm);
frame->packet_in->pi_refcnt = 1;
frame->data_frame = test->initial_frames[i];
ins = di->di_if->di_insert_frame(di, frame, 0);
assert(INS_FRAME_OK == ins); /* Self-test */
}
nread = 0;
while (nread < test->read_until)
{
data_frame = di->di_if->di_get_frame(di, nread);
assert(data_frame); /* Self-check */
n_to_read = test->read_until - nread > data_frame->df_size - data_frame->df_read_off
? data_frame->df_size - data_frame->df_read_off : test->read_until - nread;
data_frame->df_read_off += n_to_read;
nread += n_to_read;
if (data_frame->df_read_off == data_frame->df_size)
di->di_if->di_frame_done(di, data_frame);
else
{
assert(nread == test->read_until);
break;
}
}
frame = lsquic_malo_get(mm.malo.stream_frame);
frame->packet_in = lsquic_mm_get_packet_in(&mm);
frame->packet_in->pi_refcnt = 1;
frame->data_frame = test->data_frame;
ins = di->di_if->di_insert_frame(di, frame, test->read_until);
assert(test->ins == ins);
di->di_if->di_destroy(di);
lsquic_mm_cleanup(&mm);
}
int
main (int argc, char **argv)
{
const struct nocopy_test *test;
int opt;
lsquic_log_to_fstream(stderr, LLTS_NONE);
while (-1 != (opt = getopt(argc, argv, "l:")))
{
switch (opt)
{
case 'l':
lsquic_logger_lopt(optarg);
break;
default:
return 1;
}
}
for (test = tests; test < tests + sizeof(tests) / sizeof(tests[0]); ++test)
run_di_nocopy_test(test);
return 0;
}

View File

@ -20,7 +20,6 @@
#include "lsquic_version.h"
/* The struct is used to test both generation and parsing of version
* negotiation packet.
*/
@ -65,7 +64,6 @@ static const struct gen_ver_nego_test tests[] = {
.gvnt_len = -1, /* Invalid version specified in the bitmask */
},
{ .gvnt_lineno = __LINE__,
.gvnt_cid = 0x0102030405060708UL,
.gvnt_versions = (1 << LSQVER_035) | (1 << LSQVER_039) | (1 << LSQVER_043),
@ -80,6 +78,21 @@ static const struct gen_ver_nego_test tests[] = {
'Q', '0', '4', '3',
},
},
{ .gvnt_lineno = __LINE__,
.gvnt_cid = 0x0102030405060708UL,
.gvnt_versions = (1 << LSQVER_044),
.gvnt_bufsz = 18,
.gvnt_len = 18,
.gvnt_buf = {
0x80,
0x00, 0x00, 0x00, 0x00, /* Version negotiation indicator */
0x05, /* SCIL */
0x08, 0x07, 0x06, 0x05, 0x04, 0x03, 0x02, 0x01, /* Connection ID */
'Q', '0', '4', '4',
},
},
};