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

fix: truncated body dump error #573

Merged
merged 1 commit into from
Jun 30, 2024
Merged

Conversation

yuweizzz
Copy link
Contributor

this is the extend of #572 .

The request/response body may be truncated in events, so the 'Content-Length' header value not match the real data length, ReadAll/Close will always raise UnexpectedEOF error. #572 can only fix the real data length is 0, the pr will fix the others.

@cfc4n
Copy link
Member

cfc4n commented Jun 22, 2024

Can you add a unit test? Cover both cases where Content-Length is 0 and not 0.

@cfc4n cfc4n added the enhancement New feature or request label Jun 22, 2024
@yuweizzz
Copy link
Contributor Author

package main

import (
	"bufio"
	"bytes"
	"fmt"
	"io"
	"net/http"
	"net/http/httptest"
	"net/http/httputil"
)

func main() {
	// normal
	const body = "abcde"
	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		w.Header().Set("Date", "Wed, 19 Jul 1972 19:00:00 GMT")
		fmt.Fprintln(w, body)
	}))
	defer ts.Close()
	resp, err := http.Get(ts.URL)
	if err != nil {
		fmt.Println(err)
	}
	defer resp.Body.Close()

	dump, err := httputil.DumpResponse(resp, true)
	if err != nil {
		fmt.Println(err)
	}
	fmt.Println(string(dump))

	rd := bytes.NewReader(dump)
	buf := bufio.NewReader(rd)
	read_resp, err := http.ReadResponse(buf, nil)
	if err != nil {
		fmt.Println(err)
	}
	data, err := io.ReadAll(read_resp.Body)
	// normal, err is nil
	fmt.Println(data, err)
	fmt.Println("-------------------")

	// normal, content-length is 0
	ts = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		w.Header().Set("Date", "Wed, 19 Jul 1972 19:00:00 GMT")
	}))
	defer ts.Close()
	resp, err = http.Get(ts.URL)
	if err != nil {
		fmt.Println(err)
	}
	defer resp.Body.Close()

	dump, err = httputil.DumpResponse(resp, true)
	if err != nil {
		fmt.Println(err)
	}
	fmt.Println(string(dump))

	rd = bytes.NewReader(dump)
	buf = bufio.NewReader(rd)
	read_resp, err = http.ReadResponse(buf, nil)
	if err != nil {
		fmt.Println(err)
	}
	data, err = io.ReadAll(read_resp.Body)
	// normal, err is nil
	fmt.Println(data, err)
	fmt.Println("-------------------")

	// head method respone, content-length is 6, real length is 0
	ts = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		w.Header().Set("Date", "Wed, 19 Jul 1972 19:00:00 GMT")
		fmt.Fprintln(w, body)
	}))
	defer ts.Close()
	resp, err = http.Head(ts.URL)
	if err != nil {
		fmt.Println(err)
	}
	defer resp.Body.Close()

	dump, err = httputil.DumpResponse(resp, true)
	if err != nil {
		fmt.Println(err)
	}
	// head method respone
	fmt.Println(string(dump))

	rd = bytes.NewReader(dump)
	buf = bufio.NewReader(rd)
	read_resp, err = http.ReadResponse(buf, nil)
	if err != nil {
		fmt.Println(err)
	}
	data, err = io.ReadAll(read_resp.Body)
	// head method respone, but DumpResponse required body, err is ErrUnexpectedEOF
	fmt.Println(data, err)
	fmt.Println("-------------------")

	// chunked respone, no content-length
	ts1 := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		w.Header().Set("Date", "Wed, 19 Jul 1972 19:00:00 GMT")
		w.Header().Set("Content-Length", "")
		fmt.Fprintln(w, body)
	}))
	defer ts1.Close()
	resp, err = http.Get(ts1.URL)
	if err != nil {
		fmt.Println(err)
	}
	defer resp.Body.Close()

	dump, err = httputil.DumpResponse(resp, true)
	if err != nil {
		fmt.Println(err)
	}
	fmt.Println(string(dump))

	rd = bytes.NewReader(dump)
	buf = bufio.NewReader(rd)
	read_resp, err = http.ReadResponse(buf, nil)
	if err != nil {
		fmt.Println(err)
	}
	data, err = io.ReadAll(read_resp.Body)
	// chunked respone, err is nil
	fmt.Println(data, err)
	fmt.Println("-------------------")

	// truncated respone, real data length smaller than content-length
	t := "HTTP/1.1 200 OK\r\nContent-Length: 9999\r\nContent-Type: text/plain; charset=utf-8\r\nDate: Wed, 19 Jul 1972 19:00:00 GMT\r\n\r\nabcde\n"
	rd = bytes.NewReader([]byte(t))
	buf = bufio.NewReader(rd)
	read_resp, err = http.ReadResponse(buf, nil)
	if err != nil {
		fmt.Println(err)
	}
	data, err = io.ReadAll(read_resp.Body)
	// truncated respone, err is ErrUnexpectedEOF
	fmt.Println(data, err)
}

hr.packerType = PacketTypeGzip
defer reader.Close()
default:
hr.packerType = PacketTypeNull
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the code from lines 108 to 143 merely for parsing packerType?

default:
hr.packerType = PacketTypeNull
}
b, err := httputil.DumpRequest(hr.request, false)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is 'false' used here?

@cfc4n
Copy link
Member

cfc4n commented Jun 23, 2024

@yuweizzz Thank you for your contribution. What I mean is "Could you add some unit tests in the PR to verify your code?"

@cfc4n
Copy link
Member

cfc4n commented Jun 30, 2024

Merge this PR first, and I will add unit tests. thanks.

@cfc4n cfc4n merged commit 253bea3 into gojue:master Jun 30, 2024
6 of 7 checks passed
@yuweizzz yuweizzz deleted the gzip_body_request branch July 4, 2024 07:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants