Repository: pion/turn (go)

Issue:
Allocation refresh with zero lifetime not deleted as expected
### Your environment.
  - Version: v1.3.3 -38ac12
  - Browser: n/a

### What did you do?

Refresh allocation with 0 second lifetime

### What did you expect?
The `allocation` doesn't remove from the manager as expected.
### What happened?
Not removed, this because when we use `0` fresh lifetime, the old lifetimer only stop rather firing at once, related code is [allocation.go#L165](https://github.com/pion/turn/blob/master/internal/allocation/allocation.go#L165).

Pull request:
Allocation fresh bug fixed
Fixed #84 


Patch:
diff --git a/.travis.yml b/.travis.yml
index 8c5db64..11451c0 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -16,7 +16,7 @@ before_script:
 
 script:
   - golangci-lint run ./...
-  - rm -rf cmd # Remove examples, no test coverage for them
+  - rm -rf examples # Remove examples, no test coverage for them
   - go test -coverpkg=$(go list ./... | tr '\n' ',') -coverprofile=cover.out -v -race -covermode=atomic ./...
   - bash <(curl -s https://codecov.io/bash)
   - bash .github/assert-contributors.sh
diff --git a/internal/allocation/allocation.go b/internal/allocation/allocation.go
index acd464d..19b3091 100644
--- a/internal/allocation/allocation.go
+++ b/internal/allocation/allocation.go
@@ -161,12 +161,6 @@ func (a *Allocation) GetChannelByAddr(addr net.Addr) *ChannelBind {
 
 // Refresh updates the allocations lifetime
 func (a *Allocation) Refresh(lifetime time.Duration) {
-	if lifetime == 0 {
-		if !a.lifetimeTimer.Stop() {
-			a.log.Errorf("Failed to stop allocation timer for %v", a.fiveTuple)
-		}
-		return
-	}
 	if !a.lifetimeTimer.Reset(lifetime) {
 		a.log.Errorf("Failed to reset allocation timer for %v", a.fiveTuple)
 	}
diff --git a/internal/proto/uri.go b/internal/proto/uri.go
deleted file mode 100644
index 255fb6b..0000000
--- a/internal/proto/uri.go
+++ /dev/null
@@ -1,72 +0,0 @@
-package proto
-
-import (
-	"errors"
-	"fmt"
-	"net/url"
-	"strconv"
-)
-
-// Scheme definitions from RFC 7065 Section 3.2.
-const (
-	Scheme       = "turn"
-	SchemeSecure = "turns"
-)
-
-// Transport definitions as in RFC 7065.
-const (
-	TransportTCP = "tcp"
-	TransportUDP = "udp"
-)
-
-// URI as defined in RFC 7065.
-type URI struct {
-	Scheme    string
-	Host      string
-	Port      int
-	Transport string
-}
-
-func (u URI) String() string {
-	transportSuffix := ""
-	if len(u.Transport) > 0 {
-		transportSuffix = "?transport=" + u.Transport
-	}
-	if u.Port != 0 {
-		return fmt.Sprintf("%s:%s:%d%s",
-			u.Scheme, u.Host, u.Port, transportSuffix,
-		)
-	}
-	return u.Scheme + ":" + u.Host + transportSuffix
-}
-
-// ParseURI parses URI from string.
-func ParseURI(rawURI string) (URI, error) {
-	// Carefully reusing URI parser from net/url.
-	u, urlParseErr := url.Parse(rawURI)
-	if urlParseErr != nil {
-		return URI{}, urlParseErr
-	}
-	if u.Scheme != Scheme && u.Scheme != SchemeSecure {
-		return URI{}, fmt.Errorf("unknown uri scheme %q", u.Scheme)
-	}
-	if u.Opaque == "" {
-		return URI{}, errors.New("invalid uri format: expected opaque")
-	}
-	// Using URL methods to split host.
-	u.Host = u.Opaque
-	host, rawPort := u.Hostname(), u.Port()
-	uri := URI{
-		Scheme:    u.Scheme,
-		Host:      host,
-		Transport: u.Query().Get("transport"),
-	}
-	if len(rawPort) > 0 {
-		port, portErr := strconv.Atoi(rawPort)
-		if portErr != nil {
-			return uri, fmt.Errorf("failed to parse %q as port: %v", rawPort, portErr)
-		}
-		uri.Port = port
-	}
-	return uri, nil
-}


Test patch:
diff --git a/internal/allocation/allocation_test.go b/internal/allocation/allocation_test.go
index acd633d..3903486 100644
--- a/internal/allocation/allocation_test.go
+++ b/internal/allocation/allocation_test.go
@@ -3,6 +3,7 @@ package allocation
 import (
 	"fmt"
 	"net"
+	"sync"
 	"testing"
 	"time"
 
@@ -24,6 +25,7 @@ func TestAllocation(t *testing.T) {
 		{"GetChannelByNumber", subTestGetChannelByNumber},
 		{"GetChannelByAddr", subTestGetChannelByAddr},
 		{"RemoveChannelBind", subTestRemoveChannelBind},
+		{"Refresh", subTestAllocationRefresh},
 		{"Close", subTestAllocationClose},
 		{"packetHandler", subTestPacketHandler},
 	}
@@ -170,6 +172,21 @@ func subTestRemoveChannelBind(t *testing.T) {
 	assert.Nil(t, channelByAddr)
 }
 
+func subTestAllocationRefresh(t *testing.T) {
+	a := NewAllocation(nil, nil, nil)
+
+	var wg sync.WaitGroup
+	wg.Add(1)
+	a.lifetimeTimer = time.AfterFunc(proto.DefaultLifetime, func() {
+		wg.Done()
+	})
+	a.Refresh(0)
+	wg.Wait()
+
+	// lifetimeTimer has expired
+	assert.False(t, a.lifetimeTimer.Stop())
+}
+
 func subTestAllocationClose(t *testing.T) {
 	network := "udp"
 
diff --git a/internal/proto/uri_test.go b/internal/proto/uri_test.go
deleted file mode 100644
index bd46605..0000000
--- a/internal/proto/uri_test.go
+++ /dev/null
@@ -1,160 +0,0 @@
-package proto
-
-import "testing"
-
-func TestParseURI(t *testing.T) {
-	for _, tc := range []struct {
-		name string
-		in   string
-		out  URI
-	}{
-		{
-			name: "default",
-			in:   "turn:example.org",
-			out: URI{
-				Host:   "example.org",
-				Scheme: Scheme,
-			},
-		},
-		{
-			name: "secure",
-			in:   "turns:example.org",
-			out: URI{
-				Host:   "example.org",
-				Scheme: SchemeSecure,
-			},
-		},
-		{
-			name: "with port",
-			in:   "turn:example.org:8000",
-			out: URI{
-				Host:   "example.org",
-				Scheme: Scheme,
-				Port:   8000,
-			},
-		},
-		{
-			name: "with port and transport",
-			in:   "turn:example.org:8000?transport=tcp",
-			out: URI{
-				Host:      "example.org",
-				Scheme:    Scheme,
-				Port:      8000,
-				Transport: TransportTCP,
-			},
-		},
-		{
-			name: "with transport",
-			in:   "turn:example.org?transport=udp",
-			out: URI{
-				Host:      "example.org",
-				Scheme:    Scheme,
-				Transport: TransportUDP,
-			},
-		},
-		{
-			name: "with port and custom transport",
-			in:   "turns:example.org:8000?transport=quic",
-			out: URI{
-				Host:      "example.org",
-				Scheme:    SchemeSecure,
-				Port:      8000,
-				Transport: "quic",
-			},
-		},
-	} {
-		t.Run(tc.name, func(t *testing.T) {
-			out, parseErr := ParseURI(tc.in)
-			if parseErr != nil {
-				t.Fatal(parseErr)
-			}
-			if out != tc.out {
-				t.Errorf("%s != %s", out, tc.out)
-			}
-		})
-	}
-	t.Run("MustFail", func(t *testing.T) {
-		for _, tc := range []struct {
-			name string
-			in   string
-		}{
-			{
-				name: "hierarchical",
-				in:   "turn://example.org",
-			},
-			{
-				name: "bad port",
-				in:   "turn:example.org:port",
-			},
-			{
-				name: "bad scheme",
-				in:   "tcp:example.org",
-			},
-			{
-				name: "invalid uri scheme",
-				in:   "turn_s:test",
-			},
-		} {
-			t.Run(tc.name, func(t *testing.T) {
-				_, parseErr := ParseURI(tc.in)
-				if parseErr == nil {
-					t.Fatal("should fail, but did not")
-				}
-			})
-		}
-	})
-}
-
-func TestURI_String(t *testing.T) {
-	for _, tc := range []struct {
-		name string
-		uri  URI
-		out  string
-	}{
-		{
-			name: "blank",
-			out:  ":",
-		},
-		{
-			name: "simple",
-			uri: URI{
-				Host:   "example.org",
-				Scheme: Scheme,
-			},
-			out: "turn:example.org",
-		},
-		{
-			name: "secure",
-			uri: URI{
-				Host:   "example.org",
-				Scheme: SchemeSecure,
-			},
-			out: "turns:example.org",
-		},
-		{
-			name: "secure with port",
-			uri: URI{
-				Host:   "example.org",
-				Scheme: SchemeSecure,
-				Port:   443,
-			},
-			out: "turns:example.org:443",
-		},
-		{
-			name: "secure with transport",
-			uri: URI{
-				Host:      "example.org",
-				Scheme:    SchemeSecure,
-				Port:      443,
-				Transport: "tcp",
-			},
-			out: "turns:example.org:443?transport=tcp",
-		},
-	} {
-		t.Run(tc.name, func(t *testing.T) {
-			if v := tc.uri.String(); v != tc.out {
-				t.Errorf("%q != %q", v, tc.out)
-			}
-		})
-	}
-}