From 154b491ed4993d4a220ef4d752c201c556e7eb60 Mon Sep 17 00:00:00 2001 From: Matthias Ladkau Date: Sun, 22 Sep 2019 10:14:59 +0100 Subject: [PATCH] feat: Adding instance specific debug logging fix: Allowing auth to be set by an environment variable --- CHANGELOG.md | 7 ++++- README.md | 8 ++++-- auth.go | 6 ++-- examples/demo/run_demo.sh | 2 +- requesthandler.go | 59 +++++++++++++++++---------------------- requesthandler_test.go | 56 ++++++++++++++++++++++--------------- server.go | 59 +++++++++++++++++++++++++++++++-------- server/dudeldu.go | 13 ++++++++- server/dudeldu_test.go | 38 +++++++++++++++++++------ server_test.go | 29 +++++++++++++------ 10 files changed, 186 insertions(+), 91 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5f4b79d..956e841 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,12 @@ All notable changes to this project will be documented in this file. See [standard-version](https://github.com/conventional-changelog/standard-version) for commit guidelines. -### [1.2.1](https://devt.de///compare/v1.2.0...v1.2.1) (2019-09-21) +## [1.3.0](https://devt.de///compare/v1.2.0...v1.3.0) (2019-09-22) + + +### Features + +* Adding instance specific debug logging ([77d225c](https://devt.de///commit/77d225c)) diff --git a/README.md b/README.md index 752819f..c363d24 100644 --- a/README.md +++ b/README.md @@ -55,7 +55,7 @@ Note: By default you can only reach the streams via localhost. Use the -host par ### Command line options The main DudelDu executable has the following command line options: ``` -DudelDu 0.0.0 +DudelDu x.x.x Usage of ./dudeldu [options] -? Show this help message -auth string @@ -65,15 +65,19 @@ Usage of ./dudeldu [options] -fqs int Frame queue size (default 10000) -host string - Server hostname to listen on (default "localhost") + Server hostname to listen on (default "127.0.0.1") -loop Loop playlists -port string Server port to listen on (default "9091") + -pp string + Prefix all paths with a string -shuffle Shuffle playlists -tps int Thread pool size (default 10) + +Authentication can also be defined via the environment variable: DUDELDU_AUTH=":" ``` Building DudelDu diff --git a/auth.go b/auth.go index 982d6f6..604ae0c 100644 --- a/auth.go +++ b/auth.go @@ -68,7 +68,7 @@ func (drh *DefaultRequestHandler) checkAuth(bufStr string, clientString string) b, err := base64.StdEncoding.DecodeString(res[1]) if err != nil { - Print("Invalid request (cannot decode authentication): ", bufStr) + drh.logger.PrintDebug("Invalid request (cannot decode authentication): ", bufStr) return auth, bufStr, false } @@ -77,7 +77,7 @@ func (drh *DefaultRequestHandler) checkAuth(bufStr string, clientString string) // Authorize request if auth != drh.auth && drh.auth != "" { - printDebug("Wrong authentication:", auth) + drh.logger.PrintDebug("Wrong authentication:", auth) return auth, bufStr, false } @@ -89,7 +89,7 @@ func (drh *DefaultRequestHandler) checkAuth(bufStr string, clientString string) // No authorization - printDebug("No authentication found") + drh.logger.PrintDebug("No authentication found") return auth, bufStr, false } else if bufStr == "" && hasAuth { diff --git a/examples/demo/run_demo.sh b/examples/demo/run_demo.sh index 8bce376..531189a 100755 --- a/examples/demo/run_demo.sh +++ b/examples/demo/run_demo.sh @@ -1,2 +1,2 @@ #!/bin/bash -../../dudeldu -loop -shuffle -debug -auth web:web demo_playlist.dpl +DUDELDU_AUTH="web:web" ../../dudeldu -loop -shuffle -debug demo_playlist.dpl diff --git a/requesthandler.go b/requesthandler.go index 1650372..6a9a5de 100644 --- a/requesthandler.go +++ b/requesthandler.go @@ -14,7 +14,6 @@ import ( "bytes" "fmt" "io" - "log" "math" "net" "regexp" @@ -59,16 +58,6 @@ requestOffsetPattern is the pattern which is used to extract the requested offse */ var requestOffsetPattern = regexp.MustCompile("(?im)^Range: bytes=([0-9]+)-.*$") -/* -Print logger method. Using a custom type so it can be customized. -*/ -var Print = log.Print - -/* -DebugOutput is a flag to enable additional debugging output -*/ -var DebugOutput = false - /* DefaultRequestHandler data structure */ @@ -81,12 +70,15 @@ type DefaultRequestHandler struct { shuffle bool // Flag if the playlist should be shuffled auth string // Required (basic) authentication string - may be empty authPeers *datautil.MapCache // Peers which have been authenticated + logger DebugLogger // Logger for debug output } /* NewDefaultRequestHandler creates a new default request handler object. */ -func NewDefaultRequestHandler(pf PlaylistFactory, loop bool, shuffle bool, auth string) *DefaultRequestHandler { +func NewDefaultRequestHandler(pf PlaylistFactory, loop bool, + shuffle bool, auth string) *DefaultRequestHandler { + drh := &DefaultRequestHandler{ PlaylistFactory: pf, loop: loop, @@ -94,11 +86,19 @@ func NewDefaultRequestHandler(pf PlaylistFactory, loop bool, shuffle bool, auth shuffle: shuffle, auth: auth, authPeers: datautil.NewMapCache(0, peerNoAuthTimeout), + logger: nil, } drh.ServeRequest = drh.defaultServeRequest return drh } +/* +SetDebugLogger sets the debug logger for this request handler. +*/ +func (drh *DefaultRequestHandler) SetDebugLogger(logger DebugLogger) { + drh.logger = logger +} + /* HandleRequest handles requests from streaming clients. It tries to extract the path and if meta data is supported. Once a request has been successfully @@ -107,7 +107,7 @@ finishes. */ func (drh *DefaultRequestHandler) HandleRequest(c net.Conn, nerr net.Error) { - printDebug("Handling request from: ", c.RemoteAddr()) + drh.logger.PrintDebug("Handling request from: ", c.RemoteAddr()) defer func() { c.Close() @@ -116,13 +116,13 @@ func (drh *DefaultRequestHandler) HandleRequest(c net.Conn, nerr net.Error) { // Check if there was an error if nerr != nil { - Print(nerr) + drh.logger.PrintDebug(nerr) return } buf, err := drh.decodeRequestHeader(c) if err != nil { - Print(err) + drh.logger.PrintDebug(err) return } @@ -137,7 +137,7 @@ func (drh *DefaultRequestHandler) HandleRequest(c net.Conn, nerr net.Error) { clientString, _, _ = net.SplitHostPort(c.RemoteAddr().String()) } - printDebug("Client:", c.RemoteAddr(), " Request:", bufStr) + drh.logger.PrintDebug("Client:", c.RemoteAddr(), " Request:", bufStr) if i := strings.Index(bufStr, "\r\n\r\n"); i >= 0 { var auth string @@ -186,7 +186,7 @@ func (drh *DefaultRequestHandler) HandleRequest(c net.Conn, nerr net.Error) { } } - Print("Invalid request: ", bufStr) + drh.logger.PrintDebug("Invalid request: ", bufStr) } /* @@ -231,7 +231,7 @@ func (drh *DefaultRequestHandler) defaultServeRequest(c net.Conn, path string, m var currentPlaying string var err error - printDebug("Serve request path:", path, " Metadata support:", metaDataSupport, " Offset:", offset) + drh.logger.PrintDebug("Serve request path:", path, " Metadata support:", metaDataSupport, " Offset:", offset) pl := drh.PlaylistFactory.Playlist(path, drh.shuffle) if pl == nil { @@ -249,20 +249,20 @@ func (drh *DefaultRequestHandler) defaultServeRequest(c net.Conn, path string, m for { for !pl.Finished() { - if DebugOutput { + if drh.logger.IsDebugOutputEnabled() { playingString := fmt.Sprintf("%v - %v", pl.Title(), pl.Artist()) if playingString != currentPlaying { currentPlaying = playingString - printDebug("Written bytes: ", writtenBytes) - printDebug("Sending: ", currentPlaying) + drh.logger.PrintDebug("Written bytes: ", writtenBytes) + drh.logger.PrintDebug("Sending: ", currentPlaying) } } // Check if there were any errors if err != nil { - Print(err) + drh.logger.PrintDebug(err) return } @@ -282,7 +282,7 @@ func (drh *DefaultRequestHandler) defaultServeRequest(c net.Conn, path string, m } } - printDebug("Serve request path:", path, " complete") + drh.logger.PrintDebug("Serve request path:", path, " complete") } /* @@ -315,13 +315,13 @@ func (drh *DefaultRequestHandler) prepareFrame(c net.Conn, pl Playlist, frameOff if frame == nil { if !pl.Finished() { - Print(fmt.Sprintf("Empty frame for: %v - %v (Error: %v)", pl.Title(), pl.Artist(), err)) + drh.logger.PrintDebug(fmt.Sprintf("Empty frame for: %v - %v (Error: %v)", pl.Title(), pl.Artist(), err)) } } else if err != nil { if err != ErrPlaylistEnd { - Print(fmt.Sprintf("Error while retrieving playlist data: %v", err)) + drh.logger.PrintDebug(fmt.Sprintf("Error while retrieving playlist data: %v", err)) } err = nil @@ -458,12 +458,3 @@ func (drh *DefaultRequestHandler) writeUnauthorized(c net.Conn) error { return err } - -/* -printDebug will print additional debug output if `DebugOutput` is enabled. -*/ -func printDebug(v ...interface{}) { - if DebugOutput { - Print(v...) - } -} diff --git a/requesthandler_test.go b/requesthandler_test.go index baa563a..f7249cf 100644 --- a/requesthandler_test.go +++ b/requesthandler_test.go @@ -14,7 +14,6 @@ import ( "bytes" "errors" "fmt" - "log" "net" "strings" "sync" @@ -151,20 +150,17 @@ func (tp *testPlaylist) Close() error { func TestRequestServing(t *testing.T) { - DebugOutput = true + // Collect the print output var out bytes.Buffer - // Collect the print output - Print = func(v ...interface{}) { + debugLogger := &TestDebugLogger{true, func(v ...interface{}) { out.WriteString(fmt.Sprint(v...)) out.WriteString("\n") - } - defer func() { - Print = log.Print - }() + }} drh := NewDefaultRequestHandler(&testPlaylistFactory{}, false, false, "") + drh.SetDebugLogger(debugLogger) testConn := &testutil.ErrorTestingConnection{} // Test a path not found @@ -182,6 +178,8 @@ func TestRequestServing(t *testing.T) { [][]byte{[]byte("12"), nil, []byte("3")}, []error{nil, nil, errors.New("TestError")}, 0}}, false, false, "") + drh.SetDebugLogger(debugLogger) + testConn = &testutil.ErrorTestingConnection{} out.Reset() @@ -218,6 +216,8 @@ func TestRequestServing(t *testing.T) { tpl := &testPlaylist{[][]byte{[]byte("123"), []byte("4567"), []byte("0123"), []byte("456789")}, nil, 0} drh = NewDefaultRequestHandler(&testPlaylistFactory{tpl}, false, false, "") + drh.SetDebugLogger(debugLogger) + testConn = &testutil.ErrorTestingConnection{} drh.defaultServeRequest(testConn, "/testpath", true, 0, "") @@ -241,6 +241,8 @@ func TestRequestServing(t *testing.T) { tpl.fp = 0 drh = NewDefaultRequestHandler(&testPlaylistFactory{tpl}, false, false, "") + drh.SetDebugLogger(debugLogger) + testConn = &testutil.ErrorTestingConnection{} testConn.OutErr = 5 out.Reset() @@ -269,6 +271,8 @@ func TestRequestServing(t *testing.T) { tpl.fp = 0 drh = NewDefaultRequestHandler(&testPlaylistFactory{tpl}, false, false, "") + drh.SetDebugLogger(debugLogger) + testConn = &testutil.ErrorTestingConnection{} drh.defaultServeRequest(testConn, "/testpath", true, 0, "") @@ -294,6 +298,8 @@ func TestRequestServing(t *testing.T) { tpl.fp = 0 drh = NewDefaultRequestHandler(&testPlaylistFactory{tpl}, false, false, "") + drh.SetDebugLogger(debugLogger) + testConn = &testutil.ErrorTestingConnection{} drh.defaultServeRequest(testConn, "/testpath", true, 7, "") @@ -315,6 +321,8 @@ func TestRequestServing(t *testing.T) { tpl.fp = 0 drh = NewDefaultRequestHandler(&testPlaylistFactory{tpl}, false, false, "") + drh.SetDebugLogger(debugLogger) + testConn = &testutil.ErrorTestingConnection{} drh.defaultServeRequest(testConn, "/testpath", true, 2, "") @@ -339,6 +347,8 @@ func TestRequestServing(t *testing.T) { tpl.fp = 0 drh = NewDefaultRequestHandler(&testPlaylistFactory{tpl}, true, false, "") + drh.SetDebugLogger(debugLogger) + testConn = &testutil.ErrorTestingConnection{} drh.LoopTimes = 3 @@ -371,6 +381,8 @@ func TestRequestServing(t *testing.T) { tpl.fp = 0 drh = NewDefaultRequestHandler(&testPlaylistFactory{tpl}, false, false, "") + drh.SetDebugLogger(debugLogger) + testConn = &testutil.ErrorTestingConnection{} testConn.OutClose = true out.Reset() @@ -389,20 +401,18 @@ func TestRequestServing(t *testing.T) { func TestRequestHandling(t *testing.T) { - DebugOutput = true + // Collect the print output var out bytes.Buffer - // Collect the print output - Print = func(v ...interface{}) { + debugLogger := &TestDebugLogger{true, func(v ...interface{}) { out.WriteString(fmt.Sprint(v...)) out.WriteString("\n") - } - defer func() { - Print = log.Print - }() + }} drh := NewDefaultRequestHandler(nil, false, false, "") + drh.SetDebugLogger(debugLogger) + testConn := &testutil.ErrorTestingConnection{} // Check normal error return @@ -465,6 +475,8 @@ func TestRequestHandling(t *testing.T) { // Test auth drh = NewDefaultRequestHandler(nil, false, false, "web:web") + drh.SetDebugLogger(debugLogger) + testConn = &testutil.ErrorTestingConnection{} testConn.In.Reset() @@ -496,6 +508,8 @@ func TestRequestHandling(t *testing.T) { out.Reset() drh = NewDefaultRequestHandler(nil, false, false, "web:web2") + drh.SetDebugLogger(debugLogger) + testConn = &testutil.ErrorTestingConnection{} testConn.In.Reset() @@ -515,20 +529,18 @@ func TestRequestHandling(t *testing.T) { func TestRequestHandler(t *testing.T) { - DebugOutput = true + // Collect the print output var out bytes.Buffer - // Collect the print output - Print = func(v ...interface{}) { + debugLogger := &TestDebugLogger{true, func(v ...interface{}) { out.WriteString(fmt.Sprint(v...)) out.WriteString("\n") - } - defer func() { - Print = log.Print - }() + }} drh := NewDefaultRequestHandler(nil, false, false, "") + drh.SetDebugLogger(debugLogger) + dds := NewServer(drh.HandleRequest) var wg sync.WaitGroup diff --git a/server.go b/server.go index 9adafeb..4f78d2f 100644 --- a/server.go +++ b/server.go @@ -11,6 +11,7 @@ package dudeldu import ( + "log" "net" "os" "os/signal" @@ -22,23 +23,41 @@ import ( /* ProductVersion is the current version of DudelDu */ -const ProductVersion = "1.2.1" +const ProductVersion = "1.3.0" /* ConnectionHandler is a function to handle new connections */ type ConnectionHandler func(net.Conn, net.Error) +/* +DebugLogger is the debug logging interface of the Server +*/ +type DebugLogger interface { + + /* + IsDebugOutputEnabled returns true if debug output is enabled. + */ + IsDebugOutputEnabled() bool + + /* + PrintDebug will print debug output if `DebugOutput` is enabled. + */ + PrintDebug(v ...interface{}) +} + /* Server data structure */ type Server struct { - Running bool // Flag indicating if the server is running - Handler ConnectionHandler // Handler function for new connections - signalling chan os.Signal // Channel for receiving signals - tcpListener *net.TCPListener // TCP listener which accepts connections - serving bool // Internal flag indicating if the socket should be served - wgStatus *sync.WaitGroup // Optional wait group which should be notified once the server has started + Running bool // Flag indicating if the server is running + Handler ConnectionHandler // Handler function for new connections + DebugOutput bool // Enable additional debugging output + LogPrint func(v ...interface{}) // Print logger method. + signalling chan os.Signal // Channel for receiving signals + tcpListener *net.TCPListener // TCP listener which accepts connections + serving bool // Internal flag indicating if the socket should be served + wgStatus *sync.WaitGroup // Optional wait group which should be notified once the server has started } /* @@ -46,8 +65,26 @@ NewServer creates a new DudelDu server. */ func NewServer(handler ConnectionHandler) *Server { return &Server{ - Running: false, - Handler: handler, + Running: false, + Handler: handler, + DebugOutput: false, + LogPrint: log.Print, + } +} + +/* +IsDebugOutputEnabled returns true if debug output is enabled. +*/ +func (ds *Server) IsDebugOutputEnabled() bool { + return ds.DebugOutput +} + +/* +PrintDebug will print debug output if `DebugOutput` is enabled. +*/ +func (ds *Server) PrintDebug(v ...interface{}) { + if ds.DebugOutput { + ds.LogPrint(v...) } } @@ -102,8 +139,8 @@ func (ds *Server) Run(laddr string, wgStatus *sync.WaitGroup) error { // Listen for shutdown signal - if DebugOutput { - Print("Listen for shutdown signal") + if ds.IsDebugOutputEnabled() { + ds.PrintDebug("Listen for shutdown signal") } signal := <-ds.signalling diff --git a/server/dudeldu.go b/server/dudeldu.go index 3380379..488e337 100644 --- a/server/dudeldu.go +++ b/server/dudeldu.go @@ -75,6 +75,8 @@ var print = consolelogger(func(a ...interface{}) { fmt.Fprint(os.Stderr, "\n") }) +var lookupEnv func(string) (string, bool) = os.LookupEnv + /* DudelDu server instance (used by unit tests) */ @@ -103,6 +105,8 @@ func main() { flag.Usage = func() { print(fmt.Sprintf("Usage of %s [options] ", os.Args[0])) flag.PrintDefaults() + print() + print(fmt.Sprint("Authentication can also be defined via the environment variable: DUDELDU_AUTH=\":\"")) } flag.Parse() @@ -112,7 +116,11 @@ func main() { return } - dudeldu.DebugOutput = *enableDebug + // Check for auth environment variable + + if envAuth, ok := lookupEnv("DUDELDU_AUTH"); ok && *auth == "" { + *auth = envAuth + } laddr := fmt.Sprintf("%v:%v", *serverHost, *serverPort) @@ -134,6 +142,9 @@ func main() { rh := dudeldu.NewDefaultRequestHandler(plf, *loopPlaylist, *shufflePlaylist, *auth) dds = dudeldu.NewServer(rh.HandleRequest) + dds.DebugOutput = *enableDebug + + rh.SetDebugLogger(dds) defer print("Shutting down") diff --git a/server/dudeldu_test.go b/server/dudeldu_test.go index e4e5be3..329dc2f 100644 --- a/server/dudeldu_test.go +++ b/server/dudeldu_test.go @@ -15,7 +15,6 @@ import ( "flag" "fmt" "io/ioutil" - "log" "os" "testing" @@ -51,18 +50,31 @@ Test comment const pdir = "playlisttest" +type TestDebugLogger struct { + DebugOutput bool + LogPrint func(v ...interface{}) +} + +func (ds *TestDebugLogger) IsDebugOutputEnabled() bool { + return ds.DebugOutput +} + +func (ds *TestDebugLogger) PrintDebug(v ...interface{}) { + if ds.DebugOutput { + ds.LogPrint(v...) + } +} + func TestRequestHandlerFilePlaylist(t *testing.T) { + // Collect the print output + var out bytes.Buffer - // Collect the print output - dudeldu.Print = func(v ...interface{}) { + debugLogger := &TestDebugLogger{true, func(v ...interface{}) { out.WriteString(fmt.Sprint(v...)) out.WriteString("\n") - } - defer func() { - dudeldu.Print = log.Print - }() + }} os.Mkdir(pdir, 0770) defer func() { @@ -81,6 +93,7 @@ func TestRequestHandlerFilePlaylist(t *testing.T) { } drh := dudeldu.NewDefaultRequestHandler(fac, false, false, "") + drh.SetDebugLogger(debugLogger) testConn := &testutil.ErrorTestingConnection{} dudeldu.MetaDataInterval = 5 playlist.FrameSize = 5 @@ -153,6 +166,8 @@ Usage of dudeldu [options] Shuffle playlists -tps int Thread pool size (default 10) + +Authentication can also be defined via the environment variable: DUDELDU_AUTH=":" ` { t.Error("Unexpected output:", "#"+ret+"#", err) return @@ -160,7 +175,14 @@ Usage of dudeldu [options] ioutil.WriteFile("test.dpl", []byte("{}"), 0644) - os.Args = []string{"dudeldu", "-auth", "web:web", "-port", "-1", "test.dpl"} + lookupEnv = func(key string) (string, bool) { + if key == "DUDELDU_AUTH" { + return "web:web", true + } + return "", false + } + + os.Args = []string{"dudeldu", "-port", "-1", "test.dpl"} flag.CommandLine = flag.NewFlagSet(os.Args[0], flag.ExitOnError) if ret, err := execMain(); err != nil || ret != ` diff --git a/server_test.go b/server_test.go index d72276f..cd27d72 100644 --- a/server_test.go +++ b/server_test.go @@ -14,7 +14,6 @@ import ( "bytes" "fmt" "io" - "log" "net" "sync" "testing" @@ -22,20 +21,31 @@ import ( var testport = "localhost:9090" +type TestDebugLogger struct { + DebugOutput bool + LogPrint func(v ...interface{}) +} + +func (ds *TestDebugLogger) IsDebugOutputEnabled() bool { + return ds.DebugOutput +} + +func (ds *TestDebugLogger) PrintDebug(v ...interface{}) { + if ds.DebugOutput { + ds.LogPrint(v...) + } +} + func TestServer(t *testing.T) { - DebugOutput = true + // Collect the print output var out bytes.Buffer - // Collect the print output - Print = func(v ...interface{}) { + debugLogger := &TestDebugLogger{true, func(v ...interface{}) { out.WriteString(fmt.Sprint(v...)) out.WriteString("\n") - } - defer func() { - Print = log.Print - }() + }} dds := NewServer(func(c net.Conn, err net.Error) { if err != nil { @@ -48,6 +58,9 @@ func TestServer(t *testing.T) { c.Close() }) + dds.DebugOutput = debugLogger.DebugOutput + dds.LogPrint = debugLogger.LogPrint + var wg sync.WaitGroup wg.Add(1)