Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
scottfeldman opened this issue Oct 25, 2022 · 5 comments
Open

Comments

@scottfeldman
Copy link
Contributor

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).

@sago35
Copy link
Member

sago35 commented Oct 27, 2022

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.

_, err = rtl.Rpc_lwip_recv(socket, &buf, uint32(len(buf)), 8, 0)

I will check a little more.

@scottfeldman
Copy link
Contributor Author

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 {

@sago35
Copy link
Member

sago35 commented Oct 27, 2022

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.

@scottfeldman
Copy link
Contributor Author

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

@scottfeldman
Copy link
Contributor Author

scottfeldman commented May 17, 2023

Fixed by #537.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants