diff --git a/.gitmodules b/.gitmodules index 98deb1f..ac5dbd1 100644 --- a/.gitmodules +++ b/.gitmodules @@ -1,3 +1,4 @@ [submodule "vendor/libplum"] path = vendor/libplum - url = file:///home/arnaud/Work/2-towns/libplum + url = https://github.com/2-towns/libplum.git + branch = feat/configurable-timeouts diff --git a/README.md b/README.md index 1a67c16..e93821a 100644 --- a/README.md +++ b/README.md @@ -6,20 +6,6 @@ libplum tries each protocol in order (PCP → NAT-PMP → UPnP-IGD) and falls ba ## Installation -Clone with submodules: - -```bash -git clone --recurse-submodules -``` - -Or after cloning: - -```bash -git submodule update --init --recursive -``` - -Install via Nimble: - ```bash nimble install ``` @@ -31,11 +17,15 @@ import chronos import libplum/plum proc main() {.async.} = - check init().isOk() + let initRes = init() + if initRes.isErr(): + echo "init failed: ", initRes.error + return let r = await createMapping(TCP, 8080) if r.isErr(): echo "failed: ", r.error + discard cleanup() return let res = r.value @@ -47,7 +37,25 @@ proc main() {.async.} = waitFor main() ``` -See the [examples](examples) directory for a complete example. +See [examples/port_mapping.nim](examples/port_mapping.nim) for a complete example that pauses between add and remove so you can verify the mapping on your router: + +```bash +nim c -r examples/port_mapping.nim +``` + +### Timeouts + +Pass timeout options to `init` to control how long discovery and mapping wait: + +```nim +discard init(discoverTimeout = 5000, mappingTimeout = 10000) +``` + +Pass a `timeout` to `createMapping` to control the overall wait: + +```nim +let r = await createMapping(TCP, 8080, timeout = seconds(15)) +``` ### Ongoing state changes @@ -60,16 +68,29 @@ proc onStateChange(state: PlumState, mapping: PlumMapping) {.cdecl, raises: [], let r = await createMapping(TCP, 8080, onStateChange = onStateChange) ``` -### Configurable timeouts - -The discovery and mapping timeouts can be configured via `init`: +### Checking mapping state ```nim -discard init( - discoverTimeout = 5000, # ms, default 10000 - mappingTimeout = 5000, # ms, default 10000 - recheckPeriod = 60000, # ms, default 300000 -) +if hasMapping(id): + echo "mapping is still active" +``` + +## API + +See [api.md](api.md) for the full API reference. + +## Testing + +Basic tests run without a router: + +```bash +nimble test +``` + +To run tests against a real NAT device: + +```bash +NAT_TEST_PLUM=1 nimble test ``` ## License diff --git a/api.md b/api.md new file mode 100644 index 0000000..0af8707 --- /dev/null +++ b/api.md @@ -0,0 +1,113 @@ +# API Reference + +## Types + +```nim +type PlumProtocol* = enum + TCP + UDP +``` + +```nim +type PlumState* = enum + Destroyed + Pending + Success + Failure + Destroying +``` + +```nim +type PlumMapping* = object + protocol*: PlumProtocol + internalPort*: uint16 + externalPort*: uint16 + externalHost*: string +``` + +```nim +type MappingResult* = object + id*: cint + mapping*: PlumMapping +``` + +```nim +type PlumStateCallback* = + proc(state: PlumState, mapping: PlumMapping) {.cdecl, raises: [], gcsafe.} +``` + +## Procedures + +### init + +```nim +proc init*( + logLevel: plum_log_level_t = PLUM_LOG_LEVEL_NONE, + discoverTimeout: int = 0, + mappingTimeout: int = 0, + recheckPeriod: int = 0 +): Result[void, string] +``` + +Initializes the library and starts the internal thread. Must be called before any other proc. + +- `logLevel`: verbosity of internal logs (default: none) +- `discoverTimeout`: how long to probe for a NAT device, in ms (default: 10000) +- `mappingTimeout`: how long to wait for a mapping response, in ms (default: 10000) +- `recheckPeriod`: interval between periodic mapping rechecks, in ms (default: 300000) + +### cleanup + +```nim +proc cleanup*(): Result[void, string] +``` + +Stops the internal thread and releases all resources. Returns an error if called more than once or before `init`. + +### createMapping + +```nim +proc createMapping*( + protocol: PlumProtocol, + internalPort: uint16, + externalPort: uint16 = 0, + timeout: Duration = seconds(30), + onStateChange: PlumStateCallback = nil +): Future[Result[MappingResult, string]] +``` + +Requests a port mapping from the NAT device. Tries PCP, then NAT-PMP, then UPnP-IGD in order. + +- `protocol`: `TCP` or `UDP` +- `internalPort`: the local port to map +- `externalPort`: preferred external port, or 0 to let the router choose +- `timeout`: how long to wait for the mapping to be established (default: 30s) +- `onStateChange`: optional callback invoked when the mapping state changes after the initial result + +Returns a `MappingResult` containing the mapping `id` (needed for `destroyMapping`) and the `PlumMapping` with the external address and port. + +Returns an error if no NAT device is found, the mapping fails, or the timeout expires. + +### destroyMapping + +```nim +proc destroyMapping*(id: cint) +``` + +Removes a mapping. Must be called exactly once after a successful `createMapping`. + +### hasMapping + +```nim +proc hasMapping*(id: cint): bool +``` + +Returns true if the mapping exists and has not been destroyed yet. + +### getLocalAddress + +```nim +proc getLocalAddress*(): Result[string, string] +``` + +Returns the local IP address as seen from the network interface. Useful to check whether the machine already has a public IP (in which case no NAT mapping is needed). diff --git a/config.nims b/config.nims index 05af83c..c8663b9 100644 --- a/config.nims +++ b/config.nims @@ -1,7 +1,6 @@ switch("threads", "on") # begin Nimble config (version 2) ---noNimblePath when withDir(thisDir(), system.fileExists("nimble.paths")): include "nimble.paths" # end Nimble config diff --git a/examples/port_mapping.nim b/examples/port_mapping.nim index 22c8252..15a93b6 100644 --- a/examples/port_mapping.nim +++ b/examples/port_mapping.nim @@ -7,7 +7,8 @@ # those terms. # This example creates a TCP port mapping via PCP, NAT-PMP, or UPnP-IGD, -# prints the external address, and destroys the mapping before exiting. +# prints the external address so you can verify it on the router, +# then waits for Enter before removing the mapping. import chronos import libplum/plum @@ -20,6 +21,7 @@ proc main() {.async.} = echo "Local address: ", getLocalAddress().valueOr("") + # --- Add mapping --- let r = await createMapping(TCP, 8080) if r.isErr(): echo "createMapping failed: ", r.error @@ -28,11 +30,16 @@ proc main() {.async.} = let res = r.value echo "Mapping created:" - echo " external host: ", res.mapping.externalHost - echo " external port: ", res.mapping.externalPort + echo " external: ", res.mapping.externalHost, ":", res.mapping.externalPort echo " internal port: ", res.mapping.internalPort + echo "" + echo "Verify the mapping on your router, then press Enter to remove it." + try: discard readLine(stdin) + except IOError: discard + # --- Remove mapping --- destroyMapping(res.id) + echo "Mapping removed." let cleanupRes = cleanup() if cleanupRes.isErr(): diff --git a/libplum.nimble b/libplum.nimble index 1742f01..eb4ec5f 100644 --- a/libplum.nimble +++ b/libplum.nimble @@ -10,7 +10,8 @@ installDirs = @["libplum", "vendor"] ### Dependencies requires "nim >= 1.6.0", "results >= 0.4.0", - "chronos >= 4.2.0 & < 5.0.0" + "chronos >= 4.2.0 & < 5.0.0", + "unittest2" proc compileStaticLibraries() = withDir "vendor/libplum": @@ -22,6 +23,7 @@ task buildBundledLibs, "build bundled libraries": task test, "run tests": compileStaticLibraries() + exec("nimble setup") exec("nim c -r tests/test_plum.nim") before install: diff --git a/libplum/libplum.nim b/libplum/libplum.nim index fd4b133..931866a 100644 --- a/libplum/libplum.nim +++ b/libplum/libplum.nim @@ -66,9 +66,12 @@ type # Define the config struct, passed by copy (usual for struct). plum_config_t* {.importc: "plum_config_t", header: "plum.h", bycopy.} = object - log_level* {.importc: "log_level".}: plum_log_level_t - log_callback* {.importc: "log_callback".}: plum_log_callback_t - dummytls_domain* {.importc: "dummytls_domain".}: cstring + log_level* {.importc: "log_level".}: plum_log_level_t + log_callback* {.importc: "log_callback".}: plum_log_callback_t + dummytls_domain* {.importc: "dummytls_domain".}: cstring + discover_timeout* {.importc: "discover_timeout".}: cint # msecs, 0 means use default (10000) + mapping_timeout* {.importc: "mapping_timeout".}: cint # msecs, 0 means use default (10000) + recheck_period* {.importc: "recheck_period".}: cint # msecs, 0 means use default (300000) # Define the mapping struct, passed by copy (usual for struct). # The user_ptr is a pointer to the MappingHandle in order to receive the result diff --git a/libplum/plum.nim b/libplum/plum.nim index 7dd013b..62783b8 100644 --- a/libplum/plum.nim +++ b/libplum/plum.nim @@ -103,7 +103,7 @@ proc mappingCallback(id: cint, state: plum_state_t, protocol: PlumProtocol(raw[].protocol.int), internalPort: raw[].internal_port, externalPort: raw[].external_port, - externalHost: $cast[cstring](unsafeAddr raw[].external_host[0]) + externalHost: $cast[cstring](addr raw[].external_host) ) if not handle.resolved.exchange(true): @@ -118,13 +118,21 @@ proc mappingCallback(id: cint, state: plum_state_t, if not handle.onStateChange.isNil: handle.onStateChange(plumState, mapping) -proc init*(logLevel = PLUM_LOG_LEVEL_NONE): Result[void, string] {.raises: [].} = +proc init*( + logLevel: plum_log_level_t = PLUM_LOG_LEVEL_NONE, + discoverTimeout: int = 0, + mappingTimeout: int = 0, + recheckPeriod: int = 0 +): Result[void, string] {.raises: [].} = ## init MUST be called to setup internal plum thread (plum_init). var config = plum_config_t( log_level: logLevel, log_callback: nil, - dummytls_domain: nil + dummytls_domain: nil, + discover_timeout: discoverTimeout.cint, + mapping_timeout: mappingTimeout.cint, + recheck_period: recheckPeriod.cint ) let res = plum_init(addr config) @@ -146,6 +154,7 @@ proc createMapping*( protocol: PlumProtocol, internalPort: uint16, externalPort: uint16 = 0, + timeout: Duration = seconds(30), onStateChange: PlumStateCallback = nil ): Future[Result[MappingResult, string]] {.async: (raises: [CancelledError]).} = let signal = ThreadSignalPtr.new().valueOr: @@ -174,24 +183,23 @@ proc createMapping*( var completed = false try: # Wait for the callback to fireSync - completed = await withTimeout(signal.wait(), seconds(30)) + completed = await withTimeout(signal.wait(), timeout) except CancelledError: + # CancelledError skips the lines below and propagates after finally. raise finally: if not completed: - # The signal reached timeout or was cancelled, - # we cannot close it now otherwise the pending operation - # might trigger a memory issue. - # When entering into DESTROYING state, libplum will ignore - # the pending operation so closing the signal in the DESTROYED - # callback is safe. + # Timeout or cancellation: we cannot close the signal here because + # the C callback may fire later and call fireSync on it. + # Mark the handle as abandoned so the DESTROYED callback closes it. handle.abandoned.store(true) discard plum_destroy_mapping(id) else: - # The signal is completed, we can close it + # Signal fired normally, safe to close. discard signal.close() - - if not completed: + + # Reached only when completed = true (CancelledError skips this). + if not completed: return err("plum: mapping " & $id & " timed out") if handle.resolvedState == Success: @@ -204,11 +212,16 @@ proc destroyMapping*(id: cint) {.raises: [].} = ## Must be called exactly once after a successful createMapping. discard plum_destroy_mapping(id) +proc hasMapping*(id: cint): bool {.raises: [].} = + ## Returns true if the mapping exists and has not been destroyed yet. + withSafeLock: + result = id in activeMappings + proc getLocalAddress*(): Result[string, string] {.raises: [].} = var buf = newString(PLUM_MAX_ADDRESS_LEN) let res = plum_get_local_address(buf.cstring, buf.len.csize_t) if res >= 0: - buf.setLen(min(res, PLUM_MAX_ADDRESS_LEN)) + buf.setLen(min(res.int, PLUM_MAX_ADDRESS_LEN)) ok(buf) else: err("plum_get_local_address failed: " & $res) diff --git a/tests/nim.cfg b/tests/nim.cfg new file mode 100644 index 0000000..4e57d3d --- /dev/null +++ b/tests/nim.cfg @@ -0,0 +1 @@ +-p:".." diff --git a/tests/test_plum.nim b/tests/test_plum.nim index a972a4d..ba119e0 100644 --- a/tests/test_plum.nim +++ b/tests/test_plum.nim @@ -31,10 +31,17 @@ suite "plum": check r.value.len > 0 discard cleanup() + test "hasMapping returns false for unknown id": + check not hasMapping(999) + test "createMapping fails without router": # In CI with no NAT device, expect Failure or timeout — both return err. + if getEnv("NAT_TEST_PLUM") == "1": + skip() + return + discard init() - let r = waitFor createMapping(UDP, 12345) + let r = waitFor createMapping(UDP, 12345, timeout = seconds(5)) check r.isErr() discard cleanup() @@ -44,30 +51,31 @@ suite "plum - NAT port mapping (requires NAT_TEST_PLUM=1)": skip() return - check init().isOk() + check init(discoverTimeout = 15000).isOk() - let r = waitFor createMapping(TCP, 8101) + let r = waitFor createMapping(TCP, 8101, timeout = seconds(40)) check r.isOk() + if r.isOk(): + let res = r.value + check res.mapping.externalPort > 0 + check res.mapping.externalHost.len > 0 + check hasMapping(res.id) + destroyMapping(res.id) - let res = r.value - check res.mapping.externalPort > 0 - check res.mapping.externalHost.len > 0 - - destroyMapping(res.id) - check cleanup().isOk() + discard cleanup() test "createMapping UDP": if getEnv("NAT_TEST_PLUM") != "1": skip() return - check init().isOk() + check init(discoverTimeout = 2000).isOk() - let r = waitFor createMapping(UDP, 8090) + let r = waitFor createMapping(UDP, 8090, timeout = seconds(40)) check r.isOk() + if r.isOk(): + let res = r.value + check res.mapping.externalPort > 0 + destroyMapping(res.id) - let res = r.value - check res.mapping.externalPort > 0 - - destroyMapping(res.id) - check cleanup().isOk() + discard cleanup() diff --git a/vendor/libplum b/vendor/libplum index ff91874..7b3639b 160000 --- a/vendor/libplum +++ b/vendor/libplum @@ -1 +1 @@ -Subproject commit ff91874909882e0ba708a84cd9f1cf5761631869 +Subproject commit 7b3639bcf973ff9e3de554545f703f5a595a0536