From 8e2cc56afb321dbf16653158fc76884c972591c4 Mon Sep 17 00:00:00 2001 From: yusing Date: Mon, 23 Sep 2024 16:14:34 +0800 Subject: [PATCH] Fixed nil dereferencing and added missing fields validation --- 0001-fixed-nil-dereferencing.patch | 39 ++++++++++++ src/common/constants.go | 47 -------------- src/common/ports.go | 74 ++++++++++++++++++++++ src/error/builder.go | 2 + src/error/builder_test.go | 6 +- src/error/errors.go | 12 +++- src/models/raw_entry.go | 65 ++++++++++--------- src/proxy/entry.go | 5 +- src/proxy/fields/path_pattern.go | 4 +- src/proxy/fields/path_pattern_test.go | 47 ++++++++++++++ src/proxy/fields/port.go | 19 +++--- src/proxy/fields/stream_port.go | 37 ++++++----- src/proxy/fields/stream_port_test.go | 48 ++++++++++++++ src/proxy/provider/docker_provider.go | 15 +++-- src/proxy/provider/docker_provider_test.go | 2 + src/route/route.go | 2 +- src/utils/testing/testing.go | 9 ++- 17 files changed, 316 insertions(+), 117 deletions(-) create mode 100644 0001-fixed-nil-dereferencing.patch create mode 100644 src/common/ports.go create mode 100644 src/proxy/fields/path_pattern_test.go create mode 100644 src/proxy/fields/stream_port_test.go diff --git a/0001-fixed-nil-dereferencing.patch b/0001-fixed-nil-dereferencing.patch new file mode 100644 index 0000000..12a2e84 --- /dev/null +++ b/0001-fixed-nil-dereferencing.patch @@ -0,0 +1,39 @@ +From 6728bc39d2d7ded19c1ca288691ec787c02ff666 Mon Sep 17 00:00:00 2001 +From: yusing +Date: Mon, 23 Sep 2024 07:19:47 +0800 +Subject: [PATCH] fixed nil dereferencing + +--- + frontend | 2 +- + src/error/builder.go | 6 ++++-- + 2 files changed, 5 insertions(+), 3 deletions(-) + +diff --git a/frontend b/frontend +index d0e5963..441fd70 160000 +--- a/frontend ++++ b/frontend +@@ -1 +1 @@ +-Subproject commit d0e59630d6e0beb1c22d2f242f556464a5056c1f ++Subproject commit 441fd708dbed89c59bf72a742431a78ff2f02bc3 +diff --git a/src/error/builder.go b/src/error/builder.go +index 38a1bf2..16d2816 100644 +--- a/src/error/builder.go ++++ b/src/error/builder.go +@@ -60,10 +60,12 @@ func (b Builder) Build() NestedError { + } + + func (b Builder) To(ptr *NestedError) { +- if *ptr == nil { ++ if ptr == nil { ++ return ++ } else if *ptr == nil { + *ptr = b.Build() + } else { +- **ptr = *b.Build() ++ (*ptr).With(b.Build()) + } + } + +-- +2.46.1 + diff --git a/src/common/constants.go b/src/common/constants.go index 5dc0fbe..51bf1a2 100644 --- a/src/common/constants.go +++ b/src/common/constants.go @@ -37,53 +37,6 @@ const ( const DockerHostFromEnv = "$DOCKER_HOST" -var WellKnownHTTPPorts = map[uint16]bool{ - 80: true, - 8000: true, - 8008: true, - 8080: true, - 3000: true, -} - -var ( - ServiceNamePortMapTCP = map[string]int{ - "postgres": 5432, - "mysql": 3306, - "mariadb": 3306, - "redis": 6379, - "mssql": 1433, - "memcached": 11211, - "rabbitmq": 5672, - "mongo": 27017, - "minecraft-server": 25565, - - "dns": 53, - "ssh": 22, - "ftp": 21, - "smtp": 25, - "pop3": 110, - "imap": 143, - } -) - -var ImageNamePortMapHTTP = map[string]int{ - "nginx": 80, - "httpd": 80, - "adguardhome": 3000, - "gogs": 3000, - "gitea": 3000, - "portainer": 9000, - "portainer-ce": 9000, - "home-assistant": 8123, - "homebridge": 8581, - "uptime-kuma": 3001, - "changedetection.io": 3000, - "prometheus": 9090, - "grafana": 3000, - "dockge": 5001, - "nginx-proxy-manager": 81, -} - const ( IdleTimeoutDefault = "0" WakeTimeoutDefault = "10s" diff --git a/src/common/ports.go b/src/common/ports.go new file mode 100644 index 0000000..0eec93c --- /dev/null +++ b/src/common/ports.go @@ -0,0 +1,74 @@ +package common + +var ( + WellKnownHTTPPorts = map[string]bool{ + "80": true, + "8000": true, + "8008": true, + "8080": true, + "3000": true, + } + + ServiceNamePortMapTCP = map[string]int{ + "mssql": 1433, + "mysql": 3306, + "mariadb": 3306, + "postgres": 5432, + "rabbitmq": 5672, + "redis": 6379, + "memcached": 11211, + "mongo": 27017, + + "ssh": 22, + "ftp": 21, + "smtp": 25, + "dns": 53, + "pop3": 110, + "imap": 143, + } + + ImageNamePortMap = func() (m map[string]int) { + m = make(map[string]int, len(ServiceNamePortMapTCP)+len(imageNamePortMap)) + for k, v := range ServiceNamePortMapTCP { + m[k] = v + } + for k, v := range imageNamePortMap { + m[k] = v + } + return + }() + + imageNamePortMap = map[string]int{ + "adguardhome": 3000, + "bazarr": 6767, + "calibre-web": 8083, + "changedetection.io": 3000, + "dockge": 5001, + "gitea": 3000, + "gogs": 3000, + "grafana": 3000, + "home-assistant": 8123, + "homebridge": 8581, + "httpd": 80, + "immich": 3001, + "jellyfin": 8096, + "lidarr": 8686, + "minecraft-server": 25565, + "nginx": 80, + "nginx-proxy-manager": 81, + "open-webui": 8080, + "plex": 32400, + "portainer": 9000, + "portainer-ce": 9000, + "prometheus": 9090, + "prowlarr": 9696, + "radarr": 7878, + "radarr-sma": 7878, + "rsshub": 1200, + "rss-bridge": 80, + "sonarr": 8989, + "sonarr-sma": 8989, + "uptime-kuma": 3001, + "whisparr": 6969, + } +) diff --git a/src/error/builder.go b/src/error/builder.go index 16d2816..1a44a09 100644 --- a/src/error/builder.go +++ b/src/error/builder.go @@ -55,6 +55,8 @@ func (b Builder) WithSeverity(s Severity) Builder { func (b Builder) Build() NestedError { if len(b.errors) == 0 { return nil + } else if len(b.errors) == 1 { + return b.errors[0] } return Join(b.message, b.errors...).Severity(b.severity) } diff --git a/src/error/builder_test.go b/src/error/builder_test.go index 99f0b99..afd33ae 100644 --- a/src/error/builder_test.go +++ b/src/error/builder_test.go @@ -43,10 +43,10 @@ func TestBuilderNested(t *testing.T) { expected2 := (`error occurred: - Action 1 failed: - - invalid Inner: 2 - - invalid Inner: 1 + - invalid Inner: "1" + - invalid Inner: "2" - Action 2 failed: - - invalid Inner: 3`) + - invalid Inner: "3"`) if got != expected1 && got != expected2 { t.Errorf("expected \n%s, got \n%s", expected1, got) } diff --git a/src/error/errors.go b/src/error/errors.go index a430a1d..c3bccb8 100644 --- a/src/error/errors.go +++ b/src/error/errors.go @@ -10,10 +10,12 @@ var ( ErrUnsupported = stderrors.New("unsupported") ErrUnexpected = stderrors.New("unexpected") ErrNotExists = stderrors.New("does not exist") + ErrMissing = stderrors.New("missing") ErrAlreadyExist = stderrors.New("already exist") + ErrOutOfRange = stderrors.New("out of range") ) -const fmtSubjectWhat = "%w %v: %v" +const fmtSubjectWhat = "%w %v: %q" func Failure(what string) NestedError { return errorf("%s %w", what, ErrFailure) @@ -47,6 +49,14 @@ func NotExist(subject, what any) NestedError { return errorf("%v %w: %v", subject, ErrNotExists, what) } +func Missing(subject any) NestedError { + return errorf("%w %v", ErrMissing, subject) +} + func AlreadyExist(subject, what any) NestedError { return errorf("%v %w: %v", subject, ErrAlreadyExist, what) } + +func OutOfRange(subject string, value any) NestedError { + return errorf("%v %w: %v", subject, ErrOutOfRange, value) +} diff --git a/src/models/raw_entry.go b/src/models/raw_entry.go index 25c8a15..ea91c1f 100644 --- a/src/models/raw_entry.go +++ b/src/models/raw_entry.go @@ -31,50 +31,55 @@ type ( var NewProxyEntries = F.NewMapOf[string, *RawEntry] -func (e *RawEntry) SetDefaults() { - if e.ProxyProperties == nil { +func (e *RawEntry) FillMissingFields() bool { + isDocker := e.ProxyProperties != nil + + if !isDocker { e.ProxyProperties = &D.ProxyProperties{} } - if e.Scheme == "" { - switch { - case strings.ContainsRune(e.Port, ':'): - e.Scheme = "tcp" - case e.ProxyProperties != nil: - if _, ok := ServiceNamePortMapTCP[e.ImageName]; ok { - e.Scheme = "tcp" + if e.Port == "" { + if port, ok := ServiceNamePortMapTCP[e.ImageName]; ok { + e.Port = strconv.Itoa(port) + } else if port, ok := ImageNamePortMap[e.ImageName]; ok { + e.Port = strconv.Itoa(port) + } else { + switch { + case e.Scheme == "https": + e.Port = "443" + case !isDocker: + e.Port = "80" } } } + if e.Port == "" { + if e.FirstPort == "" { + return false + } + e.Port = e.FirstPort + } + if e.Scheme == "" { - switch e.Port { - case "443", "8443": + if _, ok := ServiceNamePortMapTCP[e.ImageName]; ok { + e.Scheme = "tcp" + } else if strings.ContainsRune(e.Port, ':') { + e.Scheme = "tcp" + } else if _, ok := WellKnownHTTPPorts[e.Port]; ok { + e.Scheme = "http" + } else if e.Port == "443" { e.Scheme = "https" - default: + } else if isDocker && e.Port == "" { + return false + } else { e.Scheme = "http" } } + if e.Host == "" { e.Host = "localhost" } - if e.Port == "" { - e.Port = e.FirstPort - } - if e.Port == "" { - if port, ok := ServiceNamePortMapTCP[e.Port]; ok { - e.Port = strconv.Itoa(port) - } else if port, ok := ImageNamePortMapHTTP[e.Port]; ok { - e.Port = strconv.Itoa(port) - } else { - switch e.Scheme { - case "http": - e.Port = "80" - case "https": - e.Port = "443" - } - } - } + if e.IdleTimeout == "" { e.IdleTimeout = IdleTimeoutDefault } @@ -87,4 +92,6 @@ func (e *RawEntry) SetDefaults() { if e.StopMethod == "" { e.StopMethod = StopMethodDefault } + + return true } diff --git a/src/proxy/entry.go b/src/proxy/entry.go index e4e6b23..7217b7e 100644 --- a/src/proxy/entry.go +++ b/src/proxy/entry.go @@ -44,7 +44,10 @@ func (rp *ReverseProxyEntry) UseIdleWatcher() bool { } func ValidateEntry(m *M.RawEntry) (any, E.NestedError) { - m.SetDefaults() + if !m.FillMissingFields() { + return nil, E.Missing("fields") + } + scheme, err := T.NewScheme(m.Scheme) if err.HasError() { return nil, err diff --git a/src/proxy/fields/path_pattern.go b/src/proxy/fields/path_pattern.go index eec599f..a95b0ce 100644 --- a/src/proxy/fields/path_pattern.go +++ b/src/proxy/fields/path_pattern.go @@ -13,7 +13,7 @@ func NewPathPattern(s string) (PathPattern, E.NestedError) { if len(s) == 0 { return "", E.Invalid("path", "must not be empty") } - if !pathPattern.MatchString(string(s)) { + if !pathPattern.MatchString(s) { return "", E.Invalid("path pattern", s) } return PathPattern(s), nil @@ -34,4 +34,4 @@ func ValidatePathPatterns(s []string) (PathPatterns, E.NestedError) { return pp, nil } -var pathPattern = regexp.MustCompile("^((GET|POST|DELETE|PUT|PATCH|HEAD|OPTIONS|CONNECT)\\s)?(/\\w*)+/?$") +var pathPattern = regexp.MustCompile(`^(/[-\w./]*({\$\})?|((GET|POST|DELETE|PUT|HEAD|OPTION) /[-\w./]*({\$\})?))$`) diff --git a/src/proxy/fields/path_pattern_test.go b/src/proxy/fields/path_pattern_test.go new file mode 100644 index 0000000..76d70bc --- /dev/null +++ b/src/proxy/fields/path_pattern_test.go @@ -0,0 +1,47 @@ +package fields + +import ( + "testing" + + E "github.com/yusing/go-proxy/error" + U "github.com/yusing/go-proxy/utils/testing" +) + +var validPatterns = []string{ + "/", + "/index.html", + "/somepage/", + "/drive/abc.mp4", + "/{$}", + "/some-page/{$}", + "GET /", + "GET /static/{$}", + "GET /drive/abc.mp4", + "GET /drive/abc.mp4/{$}", + "POST /auth", + "DELETE /user/", + "PUT /storage/id/", +} + +var invalidPatterns = []string{ + "/$", + "/{$}{$}", + "/{$}/{$}", + "/index.html$", + "get /", + "GET/", + "GET /$", + "GET /drive/{$}/abc.mp4/", + "OPTION /config/{$}/abc.conf/{$}", +} + +func TestPathPatternRegex(t *testing.T) { + for _, pattern := range validPatterns { + _, err := NewPathPattern(pattern) + U.ExpectNoError(t, err.Error()) + } + for _, pattern := range invalidPatterns { + _, err := NewPathPattern(pattern) + U.ExpectError2(t, pattern, E.ErrInvalid, err.Error()) + } +} diff --git a/src/proxy/fields/port.go b/src/proxy/fields/port.go index ce7582d..5708c25 100644 --- a/src/proxy/fields/port.go +++ b/src/proxy/fields/port.go @@ -13,22 +13,19 @@ func ValidatePort(v string) (Port, E.NestedError) { if err != nil { return ErrPort, E.Invalid("port number", v).With(err) } - return NewPortInt(p) + return ValidatePortInt(p) } -func NewPortInt[Int int | uint16](v Int) (Port, E.NestedError) { - pp := Port(v) - if err := pp.boundCheck(); err.HasError() { - return ErrPort, err +func ValidatePortInt[Int int | uint16](v Int) (Port, E.NestedError) { + p := Port(v) + if !p.inBound() { + return ErrPort, E.OutOfRange("port", p) } - return pp, nil + return p, nil } -func (p Port) boundCheck() E.NestedError { - if p < MinPort || p > MaxPort { - return E.Invalid("port", p) - } - return nil +func (p Port) inBound() bool { + return p >= MinPort && p <= MaxPort } const ( diff --git a/src/proxy/fields/stream_port.go b/src/proxy/fields/stream_port.go index 54beccc..9ab3941 100644 --- a/src/proxy/fields/stream_port.go +++ b/src/proxy/fields/stream_port.go @@ -1,7 +1,6 @@ package fields import ( - "fmt" "strings" "github.com/yusing/go-proxy/common" @@ -15,36 +14,42 @@ type StreamPort struct { func ValidateStreamPort(p string) (StreamPort, E.NestedError) { split := strings.Split(p, ":") - if len(split) != 2 { - return StreamPort{}, E.Invalid("stream port", fmt.Sprintf("%q", p)).With("should be in 'x:y' format") + + switch len(split) { + case 1: + split = []string{"0", split[0]} + case 2: + break + default: + return ErrStreamPort, E.Invalid("stream port", p).With("too many colons") } listeningPort, err := ValidatePort(split[0]) - if err.HasError() { - return StreamPort{}, err - } - if err = listeningPort.boundCheck(); err.HasError() { - return StreamPort{}, err + if err != nil { + return ErrStreamPort, err } proxyPort, err := ValidatePort(split[1]) - if err.HasError() { + if err.Is(E.ErrOutOfRange) { + return ErrStreamPort, err + } else if proxyPort == 0 { + return ErrStreamPort, E.Invalid("stream port", p).With("proxy port cannot be 0") + } else if err != nil { proxyPort, err = parseNameToPort(split[1]) - if err.HasError() { - return StreamPort{}, err + if err != nil { + return ErrStreamPort, err } } - if err = proxyPort.boundCheck(); err.HasError() { - return StreamPort{}, err - } - return StreamPort{ListeningPort: listeningPort, ProxyPort: proxyPort}, nil + return StreamPort{listeningPort, proxyPort}, nil } func parseNameToPort(name string) (Port, E.NestedError) { port, ok := common.ServiceNamePortMapTCP[name] if !ok { - return -1, E.Unsupported("service", name) + return ErrPort, E.Invalid("service", name) } return Port(port), nil } + +var ErrStreamPort = StreamPort{ErrPort, ErrPort} diff --git a/src/proxy/fields/stream_port_test.go b/src/proxy/fields/stream_port_test.go new file mode 100644 index 0000000..c16c9b9 --- /dev/null +++ b/src/proxy/fields/stream_port_test.go @@ -0,0 +1,48 @@ +package fields + +import ( + "testing" + + E "github.com/yusing/go-proxy/error" + U "github.com/yusing/go-proxy/utils/testing" +) + +var validPorts = []string{ + "1234:5678", + "0:2345", + "2345", + "1234:postgres", +} + +var invalidPorts = []string{ + "", + "123:", + "0:", + ":1234", + "1234:1234:1234", + "qwerty", + "asdfgh:asdfgh", + "1234:asdfgh", +} + +var outOfRangePorts = []string{ + "-1:1234", + "1234:-1", + "65536", + "0:65536", +} + +func TestStreamPort(t *testing.T) { + for _, port := range validPorts { + _, err := ValidateStreamPort(port) + U.ExpectNoError(t, err.Error()) + } + for _, port := range invalidPorts { + _, err := ValidateStreamPort(port) + U.ExpectError2(t, port, E.ErrInvalid, err.Error()) + } + for _, port := range outOfRangePorts { + _, err := ValidateStreamPort(port) + U.ExpectError2(t, port, E.ErrOutOfRange, err.Error()) + } +} diff --git a/src/proxy/provider/docker_provider.go b/src/proxy/provider/docker_provider.go index 45a04aa..6ea412f 100755 --- a/src/proxy/provider/docker_provider.go +++ b/src/proxy/provider/docker_provider.go @@ -137,6 +137,11 @@ func (p *DockerProvider) entriesFromContainerLabels(container D.Container) (M.Ra errors.Add(p.applyLabel(container, entries, key, val)) } + // remove all entries that failed to fill in missing fields + entries.RemoveAll(func(re *M.RawEntry) bool { + return !re.FillMissingFields() + }) + // selecting correct host port if container.HostConfig.NetworkMode != "host" { for _, a := range container.Aliases { @@ -148,12 +153,12 @@ func (p *DockerProvider) entriesFromContainerLabels(container D.Container) (M.Ra containerPort := strconv.Itoa(int(p.PrivatePort)) publicPort := strconv.Itoa(int(p.PublicPort)) entryPortSplit := strings.Split(entry.Port, ":") - if len(entryPortSplit) == 2 && entryPortSplit[1] == containerPort { - entryPortSplit[1] = publicPort - } else if len(entryPortSplit) == 1 && entryPortSplit[0] == containerPort { - entryPortSplit[0] = publicPort + n := len(entryPortSplit) + if entryPortSplit[n-1] == containerPort { + entryPortSplit[n-1] = publicPort + entry.Port = strings.Join(entryPortSplit, ":") + break } - entry.Port = strings.Join(entryPortSplit, ":") } } } diff --git a/src/proxy/provider/docker_provider_test.go b/src/proxy/provider/docker_provider_test.go index 61d0820..e468a54 100644 --- a/src/proxy/provider/docker_provider_test.go +++ b/src/proxy/provider/docker_provider_test.go @@ -115,6 +115,7 @@ func TestApplyLabel(t *testing.T) { Labels: map[string]string{ D.LableAliases: "a,b,c", "proxy.a.no_tls_verify": "true", + "proxy.a.port": "3333", "proxy.b.port": "1234", "proxy.c.scheme": "https", }}, "") @@ -132,6 +133,7 @@ func TestApplyLabelWithRef(t *testing.T) { Labels: map[string]string{ D.LableAliases: "a,b,c", "proxy.$1.host": "localhost", + "proxy.$1.port": "4444", "proxy.$2.port": "1234", "proxy.$3.scheme": "https", }}, "") diff --git a/src/route/route.go b/src/route/route.go index ae1f92a..4ef4a13 100755 --- a/src/route/route.go +++ b/src/route/route.go @@ -42,7 +42,7 @@ var NewRoutes = F.NewMapOf[string, Route] func NewRoute(en *M.RawEntry) (Route, E.NestedError) { rt, err := P.ValidateEntry(en) - if err.HasError() { + if err != nil { return nil, err } diff --git a/src/utils/testing/testing.go b/src/utils/testing/testing.go index 98d559e..69b9334 100644 --- a/src/utils/testing/testing.go +++ b/src/utils/testing/testing.go @@ -16,7 +16,14 @@ func ExpectNoError(t *testing.T, err error) { func ExpectError(t *testing.T, expected error, err error) { t.Helper() if !errors.Is(err, expected) { - t.Errorf("expected err %s, got nil", expected.Error()) + t.Errorf("expected err %s, got %s", expected.Error(), err.Error()) + } +} + +func ExpectError2(t *testing.T, input any, expected error, err error) { + t.Helper() + if !errors.Is(err, expected) { + t.Errorf("%v: expected err %s, got %s", input, expected.Error(), err.Error()) } }