Comment by scottlamb

Comment by scottlamb a day ago

13 replies

View on Hacker News

> The issue is that the rust library apparently conflates datagram and UDP, when they're not the same thing.

It comes down to these two lines (using full items paths for clarity):

    let socket = socket2::Socket::new(Domain::IPV4, Type::DGRAM, Some(Protocol::ICMPV4))?;
    let socket: std::net::UdpSocket = socket.into();
The latter is using this impl: https://docs.rs/socket2/0.6.1/socket2/struct.Socket.html#imp...

Basically the `socket2` crate lets you convert the fd it produces into a `UdpSocket`. It doesn't verify it really is a UDP socket first; that's up to you. If you do it blindly, you can get something with the wrong name, but it's probably harmless. (At the very least, it doesn't violate memory safety guarantees, which is what Rust code tends to be very strict about.)

`UdpSocket` itself has a `From<OwnedFd>` impl that similarly doesn't check it really is a UDP socket; you could convert the `socket2::Socket` to an `OwnedFd` then that to a `UdpSocket`. https://doc.rust-lang.org/stable/std/net/struct.UdpSocket.ht... https://docs.rs/socket2/0.6.1/socket2/struct.Socket.html#imp...

antonvs a day ago

It may be memory safe but it's not using the type system to represent the domain very well.

One could imagine a more type-friendly design in which we could write that first line as follows:

    let socket: Socket<IPv4, Datagram, IcmpV4> = Socket::new()?;
Now, the specifics of socket types will be statically checked.

Edit: I realized that the issue here is actually the conversion, and that UdpSocket on its own is actually a type-safe representation of a UDP socket, not a general datagram socket. But the fact that this dubiously-safe conversion is possible and even useful suggests that an improved design is possible. For example, a method like UdpSocket's `set_broadcast` can't work with a socket like the above, and from a type safety perspective, it shouldn't be possible to call it on such a socket.

Reply View 12 replies
  • scottlamb a day ago

    One could, but one probably doesn't want to have separate types for TCP-over-IPv4 vs TCP-over-IPv6 for example, even if they accept/produce different forms of addresses. That'd force a lot of code bloat with monomorphization.

    So now one is making one's own enumeration which is different than the OS one and mapping between them, which can get into a mess of all the various protocols Linux and other OSs support, and I'm not sure it's solving a major problem. Opinions vary, but I prefer to use complex types sparingly.

    I think there are likely a bunch of other cases where it's useful to choose these values more dynamically too. Networking gets weird!

    Reply View 1 reply
    • antonvs a day ago

      It's precisely because networking gets weird that a good representation at the type level could be useful. But I agree that it'd need to be done carefully to avoid creating usability issues.

      Reply View 0 replies
  • codys 12 hours ago

    Rust has an RFC about some of the conventions for the Fd conversions. https://rust-lang.github.io/rfcs/3128-io-safety.html

    It's unfortunate they did not extend marking the OwnedFd conversions as unsafe due to the focus in the RFC on a single class of unsafety in Fds instead of having a recognition that there are other issues with arbitrary Fd conversions.

    Reply View 0 replies
  • knorker a day ago

    > dubiously-safe

    No, it's perfectly safe. Except if you expand the scope of "safe" by a lot.

    OP turned the socket into an (almost) raw file descriptor, and created an UDP socket from it. Weird, yes, but since it's perfectly memory safe and invalid operations would correctly error, it's not "dubiously-safe". It's safe.

    I mean, either your language has the ability to do raw (technically Owned in this case) file descriptors, or it doesn't.

    Maybe you'd prefer Rust had a third mode? Safe, `unsafe {}`, and `are_you_sure_you_understand_this {}`, the last one also being 'safe', but just… odd.

    Reply View 8 replies
    • antonvs a day ago

      It's dubiously safe because it allows invalid combinations, i.e. calling UDP-related methods on non-UDP sockets. I'm using "safe" in the general English sense here, "protected from or not exposed to danger or risk."

      > invalid operations would correctly error

      At runtime, yes. I'm pointing out that Rust makes it possible to do better, and catch such issues at compile time.

      Reply View 7 replies
      • knorker 21 hours ago

        Of course it allows invalid combinations.

        This also compiles:

            let f = std::fs::File::open("/dev/null").unwrap();
    let f: std::os::fd::OwnedFd = f.into();
    let socket: std::net::UdpSocket = f.into();
        If you convert a high level object into a low level one, and then back up as another type, then what exactly do you expect the language to do about that?

        > "protected from or not exposed to danger or risk."

        A computer will do what you tell it to do, not what you intend it to do. Opening a file is way more dangerous than risking errors because "this syscall doesn't work on that fd".

        There's also always risk that a syscall will fail at runtime, whether the type of fd is correct or not.

        It sounds like you would prefer if UdpSocket From<OwnedFD> should run getsockname() or something to confirm it's of the expected type, but I would prefer not. Indeed, in the general case some perfectly coded `unsafe` code could `dup2()` over the fd, so any checking at UdpSocket creation time is moot; you still don't get the safety you are asking for.

        Reply View 6 replies