drivers icon indicating copy to clipboard operation
drivers copied to clipboard

wioterminal: webserver example fails with 'malformed HTTP request "G"'

Open scottfeldman opened this issue 3 years ago • 5 comments

In testing the wioterminal webserver example I bumped into a bug.

tinygo flash -target wioterminal -serial usb examples/rtl8720dn/webserver/main.go

The test is to repeatedly make an HTTP request for /hello. The webserver will return an HTTP response of "hello". The test runs for a few (10-100) iterations, returning the correct HTTP response of "hello", but then dies with:

malformed HTTP request "G"

The test is:

$ while true; do curl 192.168.1.60/hello; done,

where 192.168.1.60 is my wioterminal webserver address.

Attached is a minicom capture with debug=true and a few extra printfs I added to analyze. (Go to bottom to see error).

minicom.cap.malformed.log

Analysis

rtl87820dn/http.go:handleHTTP() is ignoring return result (-1) when calling Rpc_lwip_recv(). It looks like Rcp_lwip_recv() reads 1 byte into buf, so len(buf) > 0 and reading into buf is done. The one byte in buf is 'G'. (My guess is this is left over from a previous successful read request of "Get /hello HTTP/1.1"). But, the result value from the recv() call is -1, which handleHTTP() ignores. (I don't have documentation on the Rpc firmware to know the meanings on the result value, so some speculation on my part here as to what is a good result vs. bad result).

scottfeldman avatar Oct 25 '22 06:10 scottfeldman

I think I reproduced it in my environment. But I have not been able to check much yet. Perhaps, as you say, we need to correctly ignore the 0xFFFFFFFFFFFF shown in red.

image

Perhaps a return value check is needed here. And the return value could be implemented incorrectly as said in slack.

https://github.com/tinygo-org/drivers/blob/81bc1bcad1862f719556d3c7ff411c3f56b143dd/rtl8720dn/http.go#L69

I will check a little more.

sago35 avatar Oct 27 '22 00:10 sago35

Hi, thanks for looking into this.

I have a rough fix that ran over night. The diff is below. Again, I don't have any documentation on the rpc firmware, so just making some educated guesses here.

This tries up to 10x to read into the buffer. If the read result == -1, we retry again. The worst case I saw was 4 retries. This only works if the whole http request is contained in the first successful read. If the read returns only a portion of the http request, we need to continue reading until end of the request (CR/LF/CR/LF) + body. but for my testing, it seems each http request is fetched with a single read.

I'm not sure why the second read into buf2 is there, or why four more reads happen before close, so I commented out that code and things still work.

diff --git a/rtl8720dn/http.go b/rtl8720dn/http.go
index 921ee9d..fdd9fd2 100644
--- a/rtl8720dn/http.go
+++ b/rtl8720dn/http.go
@@ -65,12 +65,20 @@ func (rtl *RTL8720DN) handleHTTP() error {
        }
 
        buf := make([]byte, 4096)
-       for {
-               _, err = rtl.Rpc_lwip_recv(socket, &buf, uint32(len(buf)), 8, 0)
+       for i := 0; i < 10; i++ {
+               buf_copy := buf
+               result, err := rtl.Rpc_lwip_recv(socket, &buf_copy, uint32(len(buf_copy)), 8, 0)
                if err != nil {
                        return nil
                }
-               if len(buf) > 0 {
+               if result == -1 {
+                       fmt.Printf("i=%d result=-1\r\n", i)
+                       time.Sleep(100 * time.Millisecond)
+                       continue
+               }
+               if len(buf_copy) > 0 {
+                       buf = buf_copy
+                       fmt.Printf("len(buf_copy)=%d len(buf)=%d\r\n", len(buf_copy), len(buf))
                        break
                }
 
@@ -82,6 +90,7 @@ func (rtl *RTL8720DN) handleHTTP() error {
                time.Sleep(100 * time.Millisecond)
        }
 
+       /*
        buf2 := make([]byte, 4096)
        result, err := rtl.Rpc_lwip_recv(socket, &buf2, uint32(len(buf2)), 8, 0)
        if err != nil {
@@ -98,6 +107,7 @@ func (rtl *RTL8720DN) handleHTTP() error {
        if result != 11 {
                return fmt.Errorf("Rpc_lwip_errno error")
        }
+       */
 
        b := bufio.NewReader(bytes.NewReader(buf))
        req, err := http.ReadRequest(b)
@@ -173,6 +183,7 @@ func (rtl *RTL8720DN) handleHTTP() error {
                }
        }
 
+       /*
        for i := 0; i < 4; i++ {
                buf := make([]byte, 4096)
                _, err = rtl.Rpc_lwip_recv(socket, &buf, uint32(len(buf)), 8, 0)
@@ -185,6 +196,7 @@ func (rtl *RTL8720DN) handleHTTP() error {
                        return nil
                }
        }
+       */
 
        _, err = rtl.Rpc_lwip_close(socket)
        if err != nil {

scottfeldman avatar Oct 27 '22 01:10 scottfeldman

I have created a tinygo driver in the following way

  • Run the Arduino example and dump the actual UART values being sent and received
  • Adjust UART to be the same as much as possible
  • erpc is generated by go-erpc

Much of the information can be found below. However, I think there is no detailed documentation.

  • rtl8720dn
    • https://github.com/Seeed-Studio/seeed-ambd-firmware
  • wioterminal
    • https://github.com/Seeed-Studio/Seeed_Arduino_rpcUnified/blob/cf612d4382d62ada585ebfd1b53e5fc0e3bce5cb/src/erpc/rpc_wifi_api_client.cpp#L5538
  • erpc idl
    • https://github.com/Seeed-Studio/seeed-ambd-firmware/tree/7334817a6fcf311fbe3c9a8c5644b805ae12bdab/erpc_idl
  • other info
    • https://wiki.seeedstudio.com/Wio-Terminal-Network-Overview/

sago35 avatar Oct 27 '22 03:10 sago35

Just a note for now: this issue is fixed with the netdev branch.

scottfeldman avatar Jan 31 '23 07:01 scottfeldman

Fixed by #537.

scottfeldman avatar May 17 '23 06:05 scottfeldman