| Message ID | 20181115140501.7872-6-david@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | qapi: rewrite string-input-visitor | expand |
On 11/15/18 8:04 AM, David Hildenbrand wrote: > Test that very big/small values are not accepted and that ranges with > only one element work. > > Rename expect4 to expect5, as we will be moving that to a separate ulist > test after the rework. > > Signed-off-by: David Hildenbrand <david@redhat.com> > --- > tests/test-string-input-visitor.c | 22 ++++++++++++++++++++-- > 1 file changed, 20 insertions(+), 2 deletions(-) > I don't see a test for a range that wraps around (such as UINT_MAX-0); that's worth testing (whether it happens to work or is rejected as invalid). Do we require ranges to be ascending, or does 6-5 result in the sequence 5, 6? I also recall that our range code imposes a limit on the maximum elements present in a single range, in order to prevent denial-of-service attacks where a caller could request 0-INT_MAX to exhaust resources enumerating everything in the range; does our testsuite cover those limits?
On 15.11.18 18:13, Eric Blake wrote: > On 11/15/18 8:04 AM, David Hildenbrand wrote: >> Test that very big/small values are not accepted and that ranges with >> only one element work. >> >> Rename expect4 to expect5, as we will be moving that to a separate ulist >> test after the rework. >> >> Signed-off-by: David Hildenbrand <david@redhat.com> >> --- >> tests/test-string-input-visitor.c | 22 ++++++++++++++++++++-- >> 1 file changed, 20 insertions(+), 2 deletions(-) >> > > I don't see a test for a range that wraps around (such as UINT_MAX-0); > that's worth testing (whether it happens to work or is rejected as > invalid). Do we require ranges to be ascending, or does 6-5 result in > the sequence 5, 6? I also recall that our range code imposes a limit on > the maximum elements present in a single range, in order to prevent > denial-of-service attacks where a caller could request 0-INT_MAX to > exhaust resources enumerating everything in the range; does our > testsuite cover those limits? > Ranges have to be ascending and old code enforced that. New code still enforces it. Wrapping ranges are AFAIC also not supported - not ascending. I can add a test. The limit is a good point. It is neither in the tests nor in the new code. But now we finally have an explanation on the 65000-somewhat thingy. I assume that we need such a limit?
David Hildenbrand <david@redhat.com> writes: > On 15.11.18 18:13, Eric Blake wrote: >> On 11/15/18 8:04 AM, David Hildenbrand wrote: >>> Test that very big/small values are not accepted and that ranges with >>> only one element work. >>> >>> Rename expect4 to expect5, as we will be moving that to a separate ulist >>> test after the rework. >>> >>> Signed-off-by: David Hildenbrand <david@redhat.com> >>> --- >>> tests/test-string-input-visitor.c | 22 ++++++++++++++++++++-- >>> 1 file changed, 20 insertions(+), 2 deletions(-) >>> >> >> I don't see a test for a range that wraps around (such as UINT_MAX-0); >> that's worth testing (whether it happens to work or is rejected as >> invalid). Do we require ranges to be ascending, or does 6-5 result in >> the sequence 5, 6? I also recall that our range code imposes a limit on >> the maximum elements present in a single range, in order to prevent >> denial-of-service attacks where a caller could request 0-INT_MAX to >> exhaust resources enumerating everything in the range; does our >> testsuite cover those limits? >> > Ranges have to be ascending and old code enforced that. New code still > enforces it. Wrapping ranges are AFAIC also not supported - not > ascending. I can add a test. Good. > The limit is a good point. It is neither in the tests nor in the new > code. But now we finally have an explanation on the 65000-somewhat > thingy. I assume that we need such a limit? Yes, we do. We don't expect untrusted input here, but typo in the monitor (say 0--1 parsed as uint64_t 0, UINT_MAX) killing the VM by eating all memory is not a happy user experience.
diff --git a/tests/test-string-input-visitor.c b/tests/test-string-input-visitor.c index 88e0e1aa9a..f55e0696c0 100644 --- a/tests/test-string-input-visitor.c +++ b/tests/test-string-input-visitor.c @@ -121,7 +121,8 @@ static void test_visitor_in_intList(TestInputVisitorData *data, int64_t expect1[] = { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 20 }; int64_t expect2[] = { 32767, -32768, -32767 }; int64_t expect3[] = { INT64_MAX, INT64_MIN }; - uint64_t expect4[] = { UINT64_MAX }; + int64_t expect4[] = { 1 }; + uint64_t expect5[] = { UINT64_MAX }; Error *err = NULL; int64List *res = NULL; int64List *tail; @@ -140,8 +141,25 @@ static void test_visitor_in_intList(TestInputVisitorData *data, "-9223372036854775808,9223372036854775807"); check_ilist(v, expect3, ARRAY_SIZE(expect3)); + v = visitor_input_test_init(data, "1-1"); + check_ilist(v, expect4, ARRAY_SIZE(expect4)); + v = visitor_input_test_init(data, "18446744073709551615"); - check_ulist(v, expect4, ARRAY_SIZE(expect4)); + check_ulist(v, expect5, ARRAY_SIZE(expect5)); + + /* Value too large */ + + v = visitor_input_test_init(data, "9223372036854775808"); + visit_type_int64List(v, NULL, &res, &err); + error_free_or_abort(&err); + g_assert(!res); + + /* Value too small */ + + v = visitor_input_test_init(data, "-9223372036854775809"); + visit_type_int64List(v, NULL, &res, &err); + error_free_or_abort(&err); + g_assert(!res); /* Empty list */
Test that very big/small values are not accepted and that ranges with only one element work. Rename expect4 to expect5, as we will be moving that to a separate ulist test after the rework. Signed-off-by: David Hildenbrand <david@redhat.com> --- tests/test-string-input-visitor.c | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-)