fix(iter): Fix Unsoundness in Filter/FilterMap::next_chunk

[TKanX]

Summary:

2 unsoundness bugs in next_chunk_dropless / FilterMap::next_chunk: (1) N=0 triggers an out-of-bounds unsafe write before any bounds check; (2) a safe try_for_each can legally ignore ControlFlow::Break, writing past the array end.

Relates #98326

This fix is simple add defensive checks. If any better way please let me know.

Would it be better to panic in the case of a buggy try_for_each?
@KyleDavidE

Fix: add an if N == 0 early return, and panic! when idx >= N inside the closure.

Closes #153803

[rustbot]

r? @scottmcm

rustbot has assigned @scottmcm.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

• Owners of files modified in this PR: @scottmcm, libs
• @scottmcm, libs expanded to 8 candidates
• Random selection from Mark-Simulacrum, joboet, scottmcm

[KyleDavidE]

Would it be better to panic in the case of a buggy try_for_each?

[bend-n]

you could also write a "manual" try_for_each instead, with a try { } block and a for loop

[TKanX]

Would it be better to panic in the case of a buggy try_for_each?

Yes!

[TKanX]

@KyleDavidE Thanks!

[KyleDavidE]

Another option would be to switch to using try_fold with a zst token accumulator. Because the impl can't conjure a new instance of the token, it would prevent calling the closure after it returns Break. This does depend on paramaticity arguments that I'm not 100% sure how much I trust.

[TKanX]

Another option would be to switch to using try_fold with a zst token accumulator. Because the impl can't conjure a new instance of the token, it would prevent calling the closure after it returns Break. This does depend on paramaticity arguments that I'm not 100% sure how much I trust.

Does this approach still hold when the Iterator impl is unsafe?

[KyleDavidE]

Another option would be to switch to using try_fold with a zst token accumulator. Because the impl can't conjure a new instance of the token, it would prevent calling the closure after it returns Break. This does depend on paramaticity arguments that I'm not 100% sure how much I trust.

Does this approach still hold when the Iterator impl is unsafe?

I'm pretty sure. https://doc.rust-lang.org/std/mem/fn.conjure_zst.html

Conjuring a zst in a way that breaks library invarents is not allowed.

[scottmcm]

r? @the8472