fix: Ignore everything after closing boundary when parsing multi form data

[seakayone]

Resolves: #2411

Enables: softwaremill/tapir#3690

Based on an idea by @fliiiix. Co-authored with @fliiiix and @grigala during a Hackergarten session.

/claim #2411
/split @fliiiix
/split @grigala

[ipetkovic]

thx @seakayone for pushing this one.
I was just about to make PR myself and then saw yours.

[seakayone]

thx @seakayone for pushing this one. I was just about to make PR myself and then saw yours.

This fix is not yet complete and more of a rough first shot. It currently has a couple of "problems":

• For one we had to replace the ChunkBuilder with a Chunk. I plan to revert that change. Even though zio's ChunkBuilder now has a knownSize method it would not work on 2.12.
• Although the actual issue is fixed other tests are broken.

I will not be able to do much the next couple of days and will progress on this in June hopefully.

[ipetkovic]

thx @seakayone for pushing this one. I was just about to make PR myself and then saw yours.

This fix is not yet complete and more of a rough first shot. It currently has a couple of "problems":

• For one we had to replace the ChunkBuilder with a Chunk. I plan to revert that change. Even though zio's ChunkBuilder now has a knownSize method it would not work on 2.12.
• Although the actual issue is fixed other tests are broken.

I will not be able to do much the next couple of days and will progress on this in June hopefully.

can we overcome first issue by having size counter in FormStateBuffer instead?

[987Nabil]

@seakayone if you want to have the bounty, you have to add /claim with the issue number to the PR description. You can also use /split if you want to split with the co-authors.

[algora-pbc]

💵 To receive payouts, sign up on Algora, link your Github account and connect with Stripe/Alipay.

[kyri-petrou]

Just some minor comments from my side, otherwise looks good to me :)

[seakayone]

Thank you @kyri-petrou I have addressed all you remarks, tests are passing. Please have another look.

[kyri-petrou]

Sorry I didn't pick this up in the last review round. I might be missing something but I think we can simplify the logic massively

[kyri-petrou]

There is something here that I don't understand / failing to see. What is the reason for using an array? Can't this be just a boolean and then L62-68 become this?

boundaryMatches &&= posInLine < closingBoundaryBytesSize && boundary.closingBoundaryBytes(posInLine) == byte

val boundaryDetected = boundaryMatches && posInLine == closingBoundaryBytesSize - 1

[seakayone]

Wonderful, you are right we do not need to remember all checked bytes this way.

That works => f1f33ba

Thanks.

[fliiiix]

tbh no idea why i thought we need an array 🙈 so much better without - nice find @kyri-petrou 🎉

[kyri-petrou]

Happy to approve this PR! However, I think you still need an approval from a code owner to get it merged