I find the original behaviour somewhat unexpected, as that seems to indicate there's no inherent connection between the flow path and its eventual sink, on the one hand, and the flow state, on the other hand. In wonder if this is a restriction of the implementation or an actual bug. It's also not clear to me why this wouldn't have resulted in FPs before this change.

Or is this just a consequence of bottom-up evaluation?

I find the original behaviour somewhat unexpected, as that seems to indicate there's no inherent connection between the flow path and its eventual sink, on the one hand, and the flow state, on the other hand. In wonder if this is a restriction of the implementation or an actual bug. It's also not clear to me why this wouldn't have resulted in FPs before this change.

Or is this just a consequence of bottom-up evaluation?

Yeah, exactly. What currently happens on main is that we first evaluate the giant cartesian product to compute sinkNode#fff and then, at some point during the dataflow recursion, we'll reach a specific ExecState state through the additional taint step (i.e., the concatenation step).

This PR reduces the set of ExecState states initially computed by sinkNode#fff to only the ones that can be reached by the node that receives flow after the concatenation. So no behavior is actually changed (and I think the behavior on main is correct).

Does that make sense?

Does that make sense?

Absolutely.

I think perhaps this query was trying to be a bit too clever with flow states. From what I understand of what its doing I'd probably have written it with just two, "safe" and "dangerouslyConcatenated". This would mean we get less detail in the violation message, so assuming its robust this fix is probably to be preferred.

From what I understand of what its doing I'd probably have written it with just two, "safe" and "dangerouslyConcatenated".

Yeah. I had something like that originally. However, that made it impossible to report anything about the location of the concatenation, which would have been a major regression compared to the two stitched together dataflow configurations that were there previously.

I do think reporting the location of the concatenation is very helpful. I can imagine that would be a natural place to insert a sanitizer (or whatever people would do to fix the vulnerability).