rawhat / glisten

A pure gleam TCP library
Apache License 2.0
55 stars 8 forks source link

ClientIp is not always the correct type #20

Closed robertwayne closed 1 month ago

robertwayne commented 1 month ago

From out brief Discord convo, I figured I'd open an issue.


The client_ip on connection is (sometimes? always?) returning an unexpected type. ClientIp is defined as pub type ClientIp = Result(#(#(Int, Int, Int, Int), Int), Nil). So, we expect a tuple of 4 Int's but I am receiving a tuple of 8 Int's.

Reproduction

Given a basic TCP server:

import gleam/bit_array
import gleam/bytes_builder
import gleam/erlang/process
import gleam/io
import gleam/option.{None}
import gleam/otp/actor
import glisten.{Packet}

pub fn main() {
  let assert Ok(_) =
    glisten.handler(handle_connection, handle_message)
    |> glisten.serve(3000)

  io.print("Listening on 127.0.0.1:3000\n")

  process.sleep_forever()
}

fn handle_connection(conn: glisten.Connection(_)) {
  let assert Ok(ip) = conn.client_ip
  io.debug(ip)

  #(Nil, None)
}

fn handle_message(msg, state, conn) {
  let assert Packet(msg) = msg
  let assert Ok(str) = bit_array.to_string(msg)

  let assert Ok(_) = glisten.send(conn, bytes_builder.from_bit_array(msg))

  actor.continue(state)
}

Running ncat -4 127.0.0.1 3000 gives a debug output of #(#(0, 0, 0, 0, 0, 65535, 32512, 1), 58988), which is incorrect. Specifically, this is 8 u16's representing an IpV6, when we want 4 u8's for an IpV4.

From digging, this looks like there just needs to be some check after calling the erlang inet.peername function to ensure the correctly sized values are being shoved into the connection state.

rawhat commented 1 month ago

Hey, sorry this kinda flew under my radar for a little bit.

This should be fixed on the master branch, I will be doing a (major) release soon.

It's a slightly different access, where you'll need to call glisten.get_client_info(connection) to get it, but it should now be "correct". I'm putting this in quotes because I did my best to make sure it's the correct version, but I might have missed something.

If you're able to test this, I would love to know if it resolves the issue :)

rawhat commented 1 month ago

I think fixed in v4.0.0. Let me know if it doesn't work!