Return HTTP 500 on Matrix discovery GET if base-url not configured; log entire HTTP request when TRACE enabled

This commit is contained in:
Philipp Heckel 2022-06-19 21:25:35 -04:00
parent e578f01e5b
commit 25a4b29ffc
5 changed files with 108 additions and 1 deletions

View File

@ -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) ## ntfy Android app v1.14.0 (UNRELEASED)
**Features:** **Features:**
* Polling is now done with since=<id> API, which makes deduping easier ([#165](https://github.com/binwiederhier/ntfy/issues/165)) * 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:** **Bugs:**

View File

@ -246,6 +246,9 @@ func (s *Server) Stop() {
func (s *Server) handle(w http.ResponseWriter, r *http.Request) { func (s *Server) handle(w http.ResponseWriter, r *http.Request) {
v := s.visitor(r) v := s.visitor(r)
log.Debug("%s Dispatching request", logHTTPPrefix(v, 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 err := s.handleInternal(w, r, v); err != nil {
if websocket.IsWebSocketUpgrade(r) { if websocket.IsWebSocketUpgrade(r) {
isNormalError := strings.Contains(err.Error(), "i/o timeout") 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 { func (s *Server) handleMatrixDiscovery(w http.ResponseWriter) error {
if s.config.BaseURL == "" {
return errHTTPInternalErrorMissingBaseURL
}
return writeMatrixDiscoveryResponse(w) return writeMatrixDiscoveryResponse(w)
} }

View File

@ -907,13 +907,23 @@ func TestServer_PublishUnifiedPushText(t *testing.T) {
require.Equal(t, "this is a unifiedpush text message", m.Message) 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)) s := newTestServer(t, newTestConfig(t))
response := request(t, s, "GET", "/_matrix/push/v1/notify", "", nil) response := request(t, s, "GET", "/_matrix/push/v1/notify", "", nil)
require.Equal(t, 200, response.Code) require.Equal(t, 200, response.Code)
require.Equal(t, `{"unifiedpush":{"gateway":"matrix"}}`+"\n", response.Body.String()) 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) { func TestServer_MatrixGateway_Push_Success(t *testing.T) {
s := newTestServer(t, newTestConfig(t)) s := newTestServer(t, newTestConfig(t))
notification := `{"notification":{"devices":[{"pushkey":"http://127.0.0.1:12345/mytopic?up=1"}]}}` notification := `{"notification":{"devices":[{"pushkey":"http://127.0.0.1:12345/mytopic?up=1"}]}}`

View File

@ -3,8 +3,10 @@ package server
import ( import (
"fmt" "fmt"
"github.com/emersion/go-smtp" "github.com/emersion/go-smtp"
"heckel.io/ntfy/util"
"net/http" "net/http"
"strings" "strings"
"unicode/utf8"
) )
func readBoolParam(r *http.Request, defaultValue bool, names ...string) bool { 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 { func logSMTPPrefix(state *smtp.ConnectionState) string {
return fmt.Sprintf("%s/%s SMTP", state.Hostname, state.RemoteAddr.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)
}

View File

@ -1,8 +1,12 @@
package server package server
import ( import (
"bytes"
"fmt"
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
"math/rand"
"net/http" "net/http"
"strings"
"testing" "testing"
) )
@ -27,3 +31,47 @@ func TestReadBoolParam(t *testing.T) {
require.Equal(t, false, up) require.Equal(t, false, up)
require.Equal(t, true, firebase) 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))
}