~gsthnz/public-inbox

This thread contains a patchset. You're looking at the original emails, but you may wish to use the patch review UI. Review patch
1

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

Details
Message ID
<20210306051910.40982-1-me@erock.io>
DKIM signature
pass
Download raw message
Patch: +39 -14
---
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
Details
Message ID
<C9Q8WA33ATTS.2CDJCQBEO5O9L@valkyrie>
In-Reply-To
<20210306051910.40982-1-me@erock.io> (view parent)
DKIM signature
pass
Download raw message
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
Reply to thread Export thread (mbox)