zeek / spicy

C++ parser generator for dissecting protocols & files.
https://docs.zeek.org/projects/spicy
Other
246 stars 35 forks source link

Fail for vectors with bytes but no stop condition #1854

Closed evantypanski closed 1 week ago

evantypanski commented 2 weeks ago

Fixes #1832

I went back and forth on this for a while, but ended up going with the naive solution here.

The issue is just that attributes are connected to a field, not a type. So, when the vector type gets validated, it just passes along its own attributes. The inner bytes just copy the vector's &size:

https://github.com/zeek/spicy/blob/64c0000c2a9385020e7f357711c0da3de4b03517/spicy/toolchain/src/compiler/validator.cc#L147-L152

That just seems like it shouldn't be allowed because it's just not useful - now everything will just get shoved in the first vector element since the first element of the vector can fit all of the bytes.

You can fix that by using parentheses around the bytes, which makes an anonymous field with its own attributes. Now the validator correctly grabs that field's attributes. You can also just redirect with a type definition. Either seems preferred to this

For the fix, since a condition before checks for the item case, we'd only get the new error with a bytes type that doesn't have its own declaration nor its own field. This error will only get triggered if there is an inner bytes type that cannot have attributes.

Other solutions would mainly be done at parse time, so I don't like them and won't go deep into that. This seems like the most consistent fix to me

bbannier commented 1 week ago

Merged with f9bbf82b7cadbf31f0e606708c380f42022847cc.