From 25a4b29ffc99c2f4e2a526afd0bd0ac9b7f56066 Mon Sep 17 00:00:00 2001
From: Philipp Heckel <pheckel@datto.com>
Date: Sun, 19 Jun 2022 21:25:35 -0400
Subject: [PATCH] Return HTTP 500 on Matrix discovery GET if base-url not
 configured; log entire HTTP request when TRACE enabled

---
 docs/releases.md      | 12 +++++++++++
 server/server.go      |  6 ++++++
 server/server_test.go | 12 ++++++++++-
 server/util.go        | 31 ++++++++++++++++++++++++++++
 server/util_test.go   | 48 +++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 108 insertions(+), 1 deletion(-)

diff --git a/docs/releases.md b/docs/releases.md
index 1efc61fb..064e7d07 100644
--- a/docs/releases.md
+++ b/docs/releases.md
@@ -4,11 +4,23 @@ and the [ntfy Android app](https://github.com/binwiederhier/ntfy-android/release
 
 <!--
 
+## ntfy server v1.27.0 (UNRELEASED)
+
+**Features:**
+
+* Trace: Log entire HTTP request to simplify debugging (no ticket)
+
+**Bugs:**
+
+* Return HTTP 500 for GET /_matrix/push/v1/notify when base-url is not configured (no ticket)
+
+
 ## ntfy Android app v1.14.0 (UNRELEASED)
 
 **Features:**
 
 * Polling is now done with since=<id> API, which makes deduping easier ([#165](https://github.com/binwiederhier/ntfy/issues/165))
+* Turned JSON stream deprecation banner into "Use WebSockets" banner (no ticket)
 
 **Bugs:**
 
diff --git a/server/server.go b/server/server.go
index 37744fec..dacfa743 100644
--- a/server/server.go
+++ b/server/server.go
@@ -246,6 +246,9 @@ func (s *Server) Stop() {
 func (s *Server) handle(w http.ResponseWriter, r *http.Request) {
 	v := s.visitor(r)
 	log.Debug("%s Dispatching request", logHTTPPrefix(v, r))
+	if log.IsTrace() {
+		log.Trace("%s Entire request (headers and body):\n%s", logHTTPPrefix(v, r), renderHTTPRequest(r))
+	}
 	if err := s.handleInternal(w, r, v); err != nil {
 		if websocket.IsWebSocketUpgrade(r) {
 			isNormalError := strings.Contains(err.Error(), "i/o timeout")
@@ -425,6 +428,9 @@ func (s *Server) handleFile(w http.ResponseWriter, r *http.Request, v *visitor)
 }
 
 func (s *Server) handleMatrixDiscovery(w http.ResponseWriter) error {
+	if s.config.BaseURL == "" {
+		return errHTTPInternalErrorMissingBaseURL
+	}
 	return writeMatrixDiscoveryResponse(w)
 }
 
diff --git a/server/server_test.go b/server/server_test.go
index 6c58a871..66ad6e1b 100644
--- a/server/server_test.go
+++ b/server/server_test.go
@@ -907,13 +907,23 @@ func TestServer_PublishUnifiedPushText(t *testing.T) {
 	require.Equal(t, "this is a unifiedpush text message", m.Message)
 }
 
-func TestServer_MatrixGateway_Discovery(t *testing.T) {
+func TestServer_MatrixGateway_Discovery_Success(t *testing.T) {
 	s := newTestServer(t, newTestConfig(t))
 	response := request(t, s, "GET", "/_matrix/push/v1/notify", "", nil)
 	require.Equal(t, 200, response.Code)
 	require.Equal(t, `{"unifiedpush":{"gateway":"matrix"}}`+"\n", response.Body.String())
 }
 
+func TestServer_MatrixGateway_Discovery_Failure_Unconfigured(t *testing.T) {
+	c := newTestConfig(t)
+	c.BaseURL = ""
+	s := newTestServer(t, c)
+	response := request(t, s, "GET", "/_matrix/push/v1/notify", "", nil)
+	require.Equal(t, 500, response.Code)
+	err := toHTTPError(t, response.Body.String())
+	require.Equal(t, 50003, err.Code)
+}
+
 func TestServer_MatrixGateway_Push_Success(t *testing.T) {
 	s := newTestServer(t, newTestConfig(t))
 	notification := `{"notification":{"devices":[{"pushkey":"http://127.0.0.1:12345/mytopic?up=1"}]}}`
diff --git a/server/util.go b/server/util.go
index d6b95a73..269a9d59 100644
--- a/server/util.go
+++ b/server/util.go
@@ -3,8 +3,10 @@ package server
 import (
 	"fmt"
 	"github.com/emersion/go-smtp"
+	"heckel.io/ntfy/util"
 	"net/http"
 	"strings"
+	"unicode/utf8"
 )
 
 func readBoolParam(r *http.Request, defaultValue bool, names ...string) bool {
@@ -58,3 +60,32 @@ func logHTTPPrefix(v *visitor, r *http.Request) string {
 func logSMTPPrefix(state *smtp.ConnectionState) string {
 	return fmt.Sprintf("%s/%s SMTP", state.Hostname, state.RemoteAddr.String())
 }
+
+func renderHTTPRequest(r *http.Request) string {
+	peekLimit := 4096
+	lines := fmt.Sprintf("%s %s %s\n", r.Method, r.URL.RequestURI(), r.Proto)
+	for key, values := range r.Header {
+		for _, value := range values {
+			lines += fmt.Sprintf("%s: %s\n", key, value)
+		}
+	}
+	lines += "\n"
+	body, err := util.Peek(r.Body, peekLimit)
+	if err != nil {
+		lines = fmt.Sprintf("(could not read body: %s)\n", err.Error())
+	} else if utf8.Valid(body.PeekedBytes) {
+		lines += string(body.PeekedBytes)
+		if body.LimitReached {
+			lines += fmt.Sprintf(" ... (peeked %d bytes)", peekLimit)
+		}
+		lines += "\n"
+	} else {
+		if body.LimitReached {
+			lines += fmt.Sprintf("(peeked bytes not UTF-8, peek limit of %d bytes reached, hex: %x ...)\n", peekLimit, body.PeekedBytes)
+		} else {
+			lines += fmt.Sprintf("(peeked bytes not UTF-8, %d bytes, hex: %x)\n", len(body.PeekedBytes), body.PeekedBytes)
+		}
+	}
+	r.Body = body // Important: Reset body, so it can be re-read
+	return strings.TrimSpace(lines)
+}
diff --git a/server/util_test.go b/server/util_test.go
index 63bc6b40..3d062b4d 100644
--- a/server/util_test.go
+++ b/server/util_test.go
@@ -1,8 +1,12 @@
 package server
 
 import (
+	"bytes"
+	"fmt"
 	"github.com/stretchr/testify/require"
+	"math/rand"
 	"net/http"
+	"strings"
 	"testing"
 )
 
@@ -27,3 +31,47 @@ func TestReadBoolParam(t *testing.T) {
 	require.Equal(t, false, up)
 	require.Equal(t, true, firebase)
 }
+
+func TestRenderHTTPRequest_ValidShort(t *testing.T) {
+	r, _ := http.NewRequest("POST", "http://ntfy.sh/mytopic?p=2", strings.NewReader("some message"))
+	r.Header.Set("Title", "A title")
+	expected := `POST /mytopic?p=2 HTTP/1.1
+Title: A title
+
+some message`
+	require.Equal(t, expected, renderHTTPRequest(r))
+}
+
+func TestRenderHTTPRequest_ValidLong(t *testing.T) {
+	body := strings.Repeat("a", 5000)
+	r, _ := http.NewRequest("POST", "http://ntfy.sh/mytopic?p=2", strings.NewReader(body))
+	r.Header.Set("Accept", "*/*")
+	expected := `POST /mytopic?p=2 HTTP/1.1
+Accept: */*
+
+` + strings.Repeat("a", 4096) + " ... (peeked 4096 bytes)"
+	require.Equal(t, expected, renderHTTPRequest(r))
+}
+
+func TestRenderHTTPRequest_InvalidShort(t *testing.T) {
+	body := []byte{0xc3, 0x28}
+	r, _ := http.NewRequest("GET", "http://ntfy.sh/mytopic/json?since=all", bytes.NewReader(body))
+	r.Header.Set("Accept", "*/*")
+	expected := `GET /mytopic/json?since=all HTTP/1.1
+Accept: */*
+
+(peeked bytes not UTF-8, 2 bytes, hex: c328)`
+	require.Equal(t, expected, renderHTTPRequest(r))
+}
+
+func TestRenderHTTPRequest_InvalidLong(t *testing.T) {
+	body := make([]byte, 5000)
+	rand.Read(body)
+	r, _ := http.NewRequest("GET", "http://ntfy.sh/mytopic/json?since=all", bytes.NewReader(body))
+	r.Header.Set("Accept", "*/*")
+	expected := `GET /mytopic/json?since=all HTTP/1.1
+Accept: */*
+
+(peeked bytes not UTF-8, peek limit of 4096 bytes reached, hex: ` + fmt.Sprintf("%x", body[:4096]) + ` ...)`
+	require.Equal(t, expected, renderHTTPRequest(r))
+}