Skip to content

PMP requests continue even if library throws error #10486

@redfast00

Description

@redfast00

What happened?

If err != nil, not in all cases is nil returned:

syncthing/lib/pmp/pmp.go

Lines 53 to 61 in bc7e56f

if err != nil {
if errors.Is(err, context.Canceled) {
return nil
}
if strings.Contains(err.Error(), "Timed out") {
slog.Debug("Timeout trying to get external address, assume no NAT-PMP available")
return nil
}
}

and https://github.com/jackpal/go-nat-pmp/blob/master/natpmp.go#L121

My router responds with version = 2 packets (PCP instead of PMP), so go-nat-pmp throws an error (https://github.com/jackpal/go-nat-pmp/blob/master/natpmp.go#L121). This should normally result in an error in syncthing, but the checking code in pmp.go does not return nil in all error cases.

Suggestion: do return nil in all error cases, and log error; that way, it would have been a lot easier to see that my router only supports v2 (PCP) instead of having to look at a network dump.

I suspect that because of this, upnp is also not tried when the PMP code looks fine.

Syncthing version

v2.0.12

Platform & operating system

Linux amd64

Browser version

No response

Relevant log output

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugA problem with current functionality, as opposed to missing functionality (enhancement)needs-triageNew issues needed to be validated

    Type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions