Add access and error logging for requests made to the server. v1 PROPOSED

Eric Bower: 1
 Add access and error logging for requests made to the server.

 3 files changed, 39 insertions(+), 14 deletions(-)

[PATCH] Add access and error logging for requests made to the server. Export this patch

---
I used nginx as a template for access and error logs.  The access logs go to
stdout and the error logs go to stdin.

ex access logs:
12.123.123.12:58282 [2021-03-06 05:03:38.467249638 +0000 UTC m=+13.910850447] "GET index.gmi" [20]
12.123.123.12:58284 [2021-03-06 05:15:34.430020167 +0000 UTC m=+729.873620943] "GET /sdfsf" [51]

ex error logs:
2021-03-06 05:15:34.430449671 +0000 UTC m=+729.874050458 [error] Not found, client: 12.123.123.12:58284, request: "GET /sdfsf"

 gemini.go  | 15 +++++++++------
 logging.go | 21 +++++++++++++++++++++
 server.go  | 17 +++++++++--------
 3 files changed, 39 insertions(+), 14 deletions(-)
 create mode 100644 logging.go

diff --git a/gemini.go b/gemini.go
index 4f01574..97f3553 100644
--- a/gemini.go
+++ b/gemini.go
@@ -36,7 +36,7 @@ func handleRequest(c net.Conn, di int, parsedURL *url.URL) {
 		redirectPermanent(c, parsedURL.String())
 		return
 	} else if parsedURL.Path != path.Clean(parsedURL.Path) {
-		sendError(c, BadRequest, "Path error")
+		sendError(c, BadRequest, "Path error", parsedURL.Path)
 		return
 	}

@@ -52,14 +52,14 @@ func serve(c net.Conn, di int, filepath string) {

 	pathInfo, err := os.Stat(fullPath)
 	if err != nil {
-		sendError(c, NotFound, "Not found")
+		sendError(c, NotFound, "Not found", filepath)
 		return
 	}

 	if pathInfo.IsDir() {
 		subDirIndex := path.Join(fullPath, IndexFile)
 		if _, err := os.Stat(subDirIndex); os.IsNotExist(err) {
-			sendError(c, NotFound, "Not found")
+			sendError(c, NotFound, "Not found", filepath)
 			return
 		}

@@ -69,17 +69,18 @@ func serve(c net.Conn, di int, filepath string) {
 	mimeType := getMimeType(fullPath)

 	if mimeType == "" {
-		sendError(c, NotFound, "")
+		sendError(c, NotFound, "", filepath)
 		return
 	}

 	file, err := os.Open(fullPath)
 	if err != nil {
-		sendError(c, NotFound, "")
+		sendError(c, NotFound, "", filepath)
 		return
 	}
 	defer file.Close()

+	accessLog(c, Success, filepath)
 	success(c, file, mimeType)
 }

@@ -87,7 +88,9 @@ func redirectPermanent(c net.Conn, url string) {
 	c.Write(getResponseBytes(RedirectPermanent, url))
 }

-func sendError(c net.Conn, code int, message string) {
+func sendError(c net.Conn, code int, message string, url string) {
+	accessLog(c, code, url)
+	errorLog(c, url, message)
 	c.Write(getResponseBytes(code, message))
 }

diff --git a/logging.go b/logging.go
new file mode 100644
index 0000000..652bd47
--- /dev/null
+++ b/logging.go
@@ -0,0 +1,21 @@
+package main
+
+import (
+	"log"
+	"os"
+	"net"
+	"time"
+)
+
+var (
+	accessLogger = log.New(os.Stdout, "", 0)
+	errorLogger = log.New(os.Stderr, "", 0)
+)
+
+func accessLog(c net.Conn, code int, url string) {
+	accessLogger.Printf("%s [%s] \"GET %s\" [%d]", c.RemoteAddr(), time.Now(), url, code)
+}
+
+func errorLog(c net.Conn, url string, msg string) {
+	errorLogger.Printf("%s [error] %s, client: %s, request: \"GET %s\"", time.Now(), msg, c.RemoteAddr(), url)
+}
diff --git a/server.go b/server.go
index eacdc16..85410c0 100644
--- a/server.go
+++ b/server.go
@@ -59,33 +59,34 @@ func handleConnection(c net.Conn) {

 	rawURL := strings.TrimSpace(req)
 	if rawURL == "" {
-		sendError(c, BadRequest, "Empty URL")
+		sendError(c, BadRequest, "Empty URL", rawURL)
 		return
 	} else if !utf8.ValidString(rawURL) {
-		sendError(c, BadRequest, "Non UTF-8 Request")
+		sendError(c, BadRequest, "Non UTF-8 Request", rawURL)
 		return
 	} else if len(rawURL) > MaxURLSize {
-		sendError(c, BadRequest, "URL Larger than 1024 bytes")
+		sendError(c, BadRequest, "URL Larger than 1024 bytes", rawURL)
 		return
 	}

 	parsedURL, err := url.Parse(rawURL)
 	if err != nil {
-		sendError(c, BadRequest, "Bad URL")
+		sendError(c, BadRequest, "Bad URL", rawURL)
+		return
 	}
 	if parsedURL.Scheme == "" {
 		parsedURL.Scheme = "gemini"
 	}
 	if parsedURL.Scheme != "gemini" {
-		sendError(c, ProxyRequestRefused, fmt.Sprintf("Unknown scheme '%s'", parsedURL.Scheme))
+		sendError(c, ProxyRequestRefused, fmt.Sprintf("Unknown scheme '%s'", parsedURL.Scheme), parsedURL.Path)
 		return
 	} else if parsedURL.Host == "" {
-		sendError(c, BadRequest, "Host not supplied")
+		sendError(c, BadRequest, "Host not supplied", parsedURL.Path)
 		return
 	}
 	_, port, _ := net.SplitHostPort(c.LocalAddr().String())
 	if parsedURL.Port() != "" && parsedURL.Port() != port {
-		sendError(c, ProxyRequestRefused, "Wrong port")
+		sendError(c, ProxyRequestRefused, "Wrong port", parsedURL.Path)
 		return
 	}

@@ -96,5 +97,5 @@ func handleConnection(c net.Conn) {
 		}
 	}
 	sendError(c, ProxyRequestRefused, fmt.Sprintf("Host not found '%s'",
-		parsedURL.Host))
+		parsedURL.Host), parsedURL.Path)
 }
--
2.30.1

Gustavo Heinz <gustavo@gsthnz.com>

2 years ago

Hi there, thanks for another patch!

I have one suggestion.

Could you make logging configurable and inactive by default? It should
be just the case of adding a "Logging" entry to the Config struct and
accepting "off" and "nginx" (Not sure if we use "nginx" or "clf" from
Common Logging Format, your choice :p). This would allow us to add more
logging templates in the future.

I was reading through the gemini mailing list and it doesn't seem to be
a consensus if logging is needed/required. So we should keep this as a
decision for the server admin. Since keeping logs could be a liability
for some.

Otherwise this patch looks good!

Thanks a lot!

--
Gustavo Heinz