TTL should be regarded as octal number (8-bit unsigned integer)

[safwanrahman]

I tried this code:

use std::net::UdpSocket;
fn main(){
    let mut socket = UdpSocket::bind("0.0.0.0:1234").unwrap();
    socket.set_ttl(300);
}

I expected to see this happen: I expected that it would raise compilation error because the ttl is octal and maximum value is 255. But I could set 300 as ttl.

Instead, this happened: It does not raise any error and I could send packet with ttl more than 255. It was causing trouble as my software was setting ttl greater than 255 and was providing wrong data.
Upon looking more into the source code and documentation, it seems like set_ttl method is taking 32-bit unsigned integer type u32 as ttl, where is should take 8-bit unsigned integer type u8 as ttl.

Meta

rustc --version --verbose:

rustc 1.43.1 (8d69840ab 2020-05-04)
binary: rustc
commit-hash: 8d69840ab92ea7f4d323420088dd8c9775f180cd
commit-date: 2020-05-04
host: x86_64-apple-darwin
release: 1.43.1
LLVM version: 9.0

Backtrace

<backtrace>

[tesuji]

Actually in this case, set_ttl returns an error by converting the C API error to std::io::Result:

warning: unused `std::result::Result` that must be used
 --> src/main.rs:4:5
  |
4 |     socket.set_ttl(300);
  |     ^^^^^^^^^^^^^^^^^^^^
  |
  = note: `#[warn(unused_must_use)]` on by default
  = note: this `Result` may be an `Err` variant, which should be handled

Code: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=bfa7dfb16d218160b8758458cd68da21
Output:

[src/main.rs:4] socket.set_ttl(300) = Err(
    Os {
        code: 22,
        kind: InvalidInput,
        message: "Invalid argument",
    },
)

[safwanrahman]

@lzutao interesting! I did not call unwrap so it did not panic. But I think the parameter should be u8 instead of u32. What do you think?

[tesuji]

I read the Unix code a bit.

rust/src/libstd/sys_common/net.rs

Lines 414 to 416 in feb3536

	pub fn set_ttl(&self, ttl: u32) -> io::Result<()> {
	setsockopt(&self.inner, c::IPPROTO_IP, c::IP_TTL, ttl as c_int)
	}

rust/src/libstd/sys_common/net.rs

Lines 58 to 70 in feb3536

	pub fn setsockopt<T>(sock: &Socket, opt: c_int, val: c_int, payload: T) -> io::Result<()> {
	unsafe {
	let payload = &payload as *const T as *const c_void;
	cvt(c::setsockopt(
	*sock.as_inner(),
	opt,
	val,
	payload,
	mem::size_of::<T>() as c::socklen_t,
	))?;
	Ok(())
	}
	}

Maybe we could implement the check for ttl range in Rust side? But what are the advantages
by doing that?

[tesuji]

Also we might not be able to change the set_ttl parameter type to u8. That would be a breaking change.

[wesleywiser]

We can't change the parameter type because this is a stable API. Also, because you don't use the Result, you will get a warning:

warning: unused `std::result::Result` that must be used
 --> src/main.rs:4:5
  |
4 |     socket.set_ttl(300);
  |     ^^^^^^^^^^^^^^^^^^^^
  |
  = note: `#[warn(unused_must_use)]` on by default
  = note: this `Result` may be an `Err` variant, which should be handled

[safwanrahman]

@wesleywiser I understand it is a stable API. but taking u32 as ttl does not make much sense as we can avoide the issue in compilation time rathar than panicing in runtime.

[nagisa]

One way to change the input type is to deprecate this function and introduce a new one with a correct type.