umputun / remark42

comment engine
https://remark42.com
MIT License
4.77k stars 375 forks source link

Admin Notification Webhook receives an additional \n that causes a malformed JSON #1791

Closed davidchua closed 2 weeks ago

davidchua commented 2 weeks ago

Version: 1.13.0 OS: Linux Envvar:

NOTIFY_ADMINS=webhook
NOTIFY_WEBHOOK_HEADERS="Content-Type:application/json"
NOTIFY_WEBHOOK_URL=http://xxx```

Issue

When configuring an admin notification webhook, remark seem to add an additional \n to the webhook content causing the JSON to be malformed at the webhook side.

image

When the webhook is called, the raw body reads as

"{\"text\": \"<p>testme</p>\n\"}"  

Even though the comment was submitted without any linebreaks after testme.

This causes a JSON parsing error on the webhook end.

Some initial investigation

I've also looked into the notify library that remark uses https://github.com/go-pkgz/notify/blob/master/webhook.go#L52-L53 and with the following test that hardcodes a \n in the text that is sent, the webhook receives it with the proper escaped \\n.

func main() {
        httpposturl := "http://xxx"
        fmt.Println("HTTP JSON POST URL:", httpposturl)

        var txt = `{"name": "morpheus<p>meow</p>\n"}`

        request, error := http.NewRequest("POST", httpposturl, bytes.NewBufferString(txt))
         request.Header.Set("Content-Type", "application/json")

        client := &http.Client{}
        response, error := client.Do(request)
        if error != nil {
                panic(error)
        }
        defer response.Body.Close()

        fmt.Println("response Status:", response.Status)
        fmt.Println("response Headers:", response.Header)
        body, _ := ioutil.ReadAll(response.Body)
        fmt.Println("response Body:", string(body))

}

Expected

Even if there's a linebreak, the linebreak should be escaped (\\n) when sending it over to the webhook.

I'm not sure why remark is sending an unescaped \n. Is there a way to prevent the linebreak from being sent over?

paskal commented 2 weeks ago

I'll check it by adding a test to go-pkg/notify to reproduce the issue. Can you please describe how you attempt to parse it? Because I suspect that parsing in Go itself might work properly, it would mean that the issue is somewhere on the receiving end and tied to JSON parsing implementation in a particular language or library or particular code, which should account for this case but doesn't.

davidchua commented 2 weeks ago

Thanks!

The receiving end is in rails, and I'm using its stdlib to parse it.

def webhook
  JSON.parse request.raw_post
end

the payload which is received as request.raw_post returns as:

<p>meow</p>\n
paskal commented 2 weeks ago

I can reproduce the error, and I'll prepare the fix shortly.

davidchua commented 2 weeks ago

Thanks. I was going to also report that with a receiving golang http server

package main

import (
        "encoding/json"
        "fmt"
        "log"
        "net/http"
)

func main() {

        var (
                portNum string = ":3008"

                mymap = make(map[string]interface{})
        )
        http.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
                fmt.Println("here")
                err := json.NewDecoder(r.Body).Decode(&mymap)
                if err != nil {
                        fmt.Printf("error is %#v", err)                                                                                                                                                                                                                                                         w.WriteHeader(400)
                        return
                }
        })

        log.Println("Started on port", portNum)
        fmt.Println("To close connection CTRL+C :-)")

        // Spinning up the server.
        err := http.ListenAndServe(portNum, nil)                                                                                                                                                                                                                                                if err != nil {
                log.Fatal(err)
        }

}

I also got an

error is &json.SyntaxError{msg:"invalid character '\\n' in string literal", Offset:25}
paskal commented 2 weeks ago

In brief, the problem arises because the webhook from the notify package sends whatever is sent to it. The default in Remark42 is to send something which looks like JSON. However, we do not escape the comment text, which manifests even with the default template but might otherwise manifest itself in other cases.