udp: Fix UdpFramed with regards to Decode

[JohnDoneth]

Duplicate of #1444 but for master instead of the v0.1.x branch.

Should (fix) #1443 on master.

UdpFramed now attempts to continue decoding past the first discrete item if there is more data in the read buffer. I added a test for this that sends a packet with 3 lines and tests for the successful decoding of each item in turn.

[LucioFranco]

@JohnDoneth thanks for doing this, I'm gonna review #1444 then when that commit is approved we can port the fix into master.

[ipetkov]

@LucioFranco I think this is ready for review now that #1444 has been merged!

[LucioFranco]

Yeah, we need to update this to the new method done in #1444 that uses decode_eof instead of just plain decode.

[Ralith]

Looks like there's still pending work on this.

[JohnDoneth]

I am working on this again. Currently stuck on this API change where BytesMut returns a slice of possibly uninitialized bytes.

error[E0308]: mismatched types
  --> tokio-util\src\udp\frame.rs:72:79
   |
72 |                 let res = ready!(Pin::new(&mut pin.socket).poll_recv_from(cx, pin.rd.bytes_mut()));
   |                                                                               ^^^^^^^^^^^^^^^^^^ expected u8, found union `std::mem::MaybeUninit`
   |
   = note: expected type `&mut [u8]`
              found type `&mut [std::mem::MaybeUninit<u8>]`

Edit: Ah, I think I got it. Just index it.

Edit 2: That didn't work either. In the end I used this bit of code I found elsewhere in the project.

let b = &mut *(pin.rd.bytes_mut() as *mut [MaybeUninit<u8>] as *mut [u8]);

[Darksonn]

I was wondering what the status on this is?

[JohnDoneth]

@Darksonn It was ready to be reviewed. It now needs to be caught up to master again

[Darksonn]

Sorry about that. If you merge master again I'll make sure someone actually reviews it :)

[LucioFranco]

@JohnDoneth sorry about the delay, but anyway you could get this rebased and we can get it merged?

[JohnDoneth]

I will put it on my todo list!

[LucioFranco]

Thanks!

[LucioFranco]

@JohnDoneth looks like some clippy and rustfmt failures?

[JohnDoneth]

I think it might be CI? cargo clippy doesn't produce any output locally and cargo fmt leaves the project unchanged.

[Darksonn]

The clippy error is the following. You probably forgot an --all-features on the clippy invocation.

error: transmute from a reference to a reference
  --> tokio-util/src/udp/frame.rs:71:36
   |
71 |                 let b: &mut [u8] = std::mem::transmute(pin.rd.bytes_mut());
   |                                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `&mut *(pin.rd.bytes_mut() as *mut [std::mem::MaybeUninit<u8>] as *mut [u8])`
   |
   = note: `-D clippy::transmute-ptr-to-ptr` implied by `-D warnings`
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#transmute_ptr_to_ptr

As for rustfmt, it's pretty confusing but you have to use the following command in Tokio's repositories. This has to do with our extensive use of features together with the use of macros to enable/disable them, which break rustfmt in a way that means it cannot find all the files.

rustfmt --edition 2018 $(find . -name '*.rs' -print)

Check out our contributing guide for more info.

[JohnDoneth]

@Darksonn Thanks! That was the problem.

I believe this PR is ready now.

ping @LucioFranco

stale

[LucioFranco]

@JohnDoneth thanks a ton for following up!

[kurnevsky]

Looks like this change makes decoding errors useless. If we return error from decode function then we will try to parse the same bytes from the same udp packet and will get the same error, and so on ad infinitum. @JohnDoneth was it expected? Should we put decoding errors to the Item type now?

[Darksonn]

Ah, that sounds like a mistake. Consider opening a PR to fix that.

[kurnevsky]

Looked through the implementation once again and realized that we have to clear the buffer when packet was parsed, and then have to handle empty input as None. So we could do the same when error happens actually.

[kurnevsky]

But then we won't be able to distinguish zero sized packets from completely consumed packets. Probably we should check for buffer size here https://github.com/tokio-rs/tokio/blob/master/tokio-util/src/udp/frame.rs#L63 ? And then do something similar for errors?