Cap socket send length to c_int::MAX on Apple targets#159530
Conversation
|
r? @Darksonn rustbot has assigned @Darksonn. Use Why was this reviewer chosen?The reviewer was selected based on:
|
This comment has been minimized.
This comment has been minimized.
| // macOS with `EINVAL`; `write_all` should now transfer it via short sends. | ||
| #[test] | ||
| #[cfg(target_pointer_width = "64")] | ||
| #[ignore = "allocates ~2 GiB and transfers it over loopback"] |
There was a problem hiding this comment.
Is the ignore because this is very slow? Is that the TCP side or the allocation?
There was a problem hiding this comment.
A bit of both but mostly the memory allocation, the client allocates a buffer of c_int::MAX + 1 (~2 GiB); the reader only uses a 1 MiB scratch buffer.
| Err(e) => panic!("read error: {e}"), | ||
| } | ||
| } | ||
| t!(tx.send(received)); |
There was a problem hiding this comment.
Why do we need a channel instead of just returning received from the thread and asserting this on join?
On Apple, `send`/`sendto` reject a length larger than `c_int::MAX` with `EINVAL` instead of doing a short send. The send length was only clamped to `wrlen_t::MAX` (a no-op on 64-bit unix), so writing more than `c_int::MAX` bytes to a socket failed on macOS. Add a `MAX_SEND_LEN` cap (`c_int::MAX` on Apple, `wrlen_t::MAX` elsewhere), used in `write`, `send`, `send_to`, and `send_with_flags`.
|
|
||
| pub fn send(&self, buf: &[u8]) -> io::Result<usize> { | ||
| let len = cmp::min(buf.len(), <wrlen_t>::MAX as usize) as wrlen_t; | ||
| let len = cmp::min(buf.len(), MAX_SEND_LEN) as wrlen_t; |
There was a problem hiding this comment.
For datagram sockets like UDP isn't inability to send it as atomic message supposed to result in EMSGSIZE? Truncating the buffer would be incorrect though truncating to i32::MAX will probably still happen to work as UDP doesn't support packets larger than 64k and thus still result in EMSGSIZE for an i32::MAX sized buffer. I think it would be better to return an explicit EMSGSIZE when the buffer is too large though.
There was a problem hiding this comment.
It can't truncate a datagram: anything over i32::MAX already exceeds the max message size, so both the full and clamped length fail with EMSGSIZE — on macOS the clamp just turns EINVAL into EMSGSIZE. Happy to add an explicit EMSGSIZE if you'd prefer, but it'd cost a SO_TYPE syscall per send.
There was a problem hiding this comment.
but it'd cost a SO_TYPE syscall per send.
Why?
There was a problem hiding this comment.
nevermind :) I misread the code layout..
On Apple,
send/sendtoreject a length larger thanc_int::MAXwithEINVALinstead of doing a short send. The send length was only clamped towrlen_t::MAX(a no-op on 64-bit unix), so writing more thanc_int::MAXbytes to a socket failed on macOS.Add a
MAX_SEND_LENcap (c_int::MAXon Apple,wrlen_t::MAXelsewhere), used inwrite,send,send_to, andsend_with_flags.Fixes #115325