[bug] http.NewRequest accepts a non-existent URL string

Question

[bug] http.NewRequest accepts a non-existent URL string

slaff-bg opened this issue 2 years ago · 4 comments

slaff-bg commented 2 years ago

Describe the bug

It doesn't matter what URL string we pass when we make a new request that is passed to the handler.

Versions

go version go1.19 linux/amd64
github.com/gorilla/mux v1.8.0

Steps to Reproduce

I think in case I pass a non-existent URL string I should get an error. But it always works correctly.

// endpoints_test.go
...
req, err := http.NewRequest("GET", "/health", nil)
...
rr := httptest.NewRecorder()
    handler := http.HandlerFunc(HealthCheckHandler)

... or ...

// endpoints_test.go
...
req, err := http.NewRequest("GET", "/somethingelse", nil)
...
rr := httptest.NewRecorder()
    handler := http.HandlerFunc(HealthCheckHandler)

Expected behavior

error

Answer 1 · 2022-11-03T21:19:45.000Z

I would have appreciated it if you shared the full test code. It's ok. I will try to explain in the best possible way. In the above code, I'm assuming there is a statement something like this handler.ServeHTTP(rr, req) which will call f as per docs ( HandlerFunc(f) is a Handler that calls f) and in our case HealthCheckHandler. So technically we're not setting any route so that it can validate the route with the match method.

If we want to validate the route as a part of the test case, we can write something like the below:

package main

import (
	"net/http"
	"net/http/httptest"
	"testing"

	"github.com/gorilla/mux"
)

func TestReproduce(t *testing.T) {
	r := mux.NewRouter()
	r.HandleFunc("/health", healthHandle).Methods("GET")

	req, err := http.NewRequest("POST", "/health", nil)
	if err != nil {
		t.Fatal(err)
	}

	// We create a ResponseRecorder (which satisfies http.ResponseWriter) to record the response.
	rr := httptest.NewRecorder()

	// Our handlers satisfy http.Handler, so we can call their ServeHTTP method
	// directly and pass in our Request and ResponseRecorder.
	r.ServeHTTP(rr, req)

	// Check the status code is what we expect.
	if status := rr.Code; status != http.StatusOK {
		t.Errorf("handler returned wrong status code: got %v want %v",
			status, http.StatusOK)
	}

	// Check the response body is what we expect.
	expected := `we're alive`
	if rr.Body.String() != expected {
		t.Errorf("handler returned unexpected body: got %v want %v",
			rr.Body.String(), expected)
	}

}

$:— go test -v
=== RUN   TestReproduce
    main_test.go:29: handler returned wrong status code: got 405 want 200
    main_test.go:36: handler returned unexpected body: got  want we're alive
--- FAIL: TestReproduce (0.00s)
FAIL
exit status 1

Test case failed because we configured to route for the GET method and while making a request we selected the method POST. I hope you understand what I'm trying to say.

If this answer your question feel free to close the issue. Looking forward to hearing from you!

Answer 2 · 2022-11-04T17:07:44.000Z

Please excuse me if I didn't express myself clearly in the previous post! I tried to be as brief as possible. :)

Yes, it is the same example you showed in your answer. But it's not about the protocol method. What I'm trying to explain is that it doesn't matter what the second argument I will pass to the function NewRequest().

package main

import (
	"net/http"
	"net/http/httptest"
	"testing"

	"github.com/gorilla/mux"
)

func TestReproduce(t *testing.T) {
	r := mux.NewRouter()
	r.HandleFunc("/health", healthHandle).Methods("GET")


	// PAY ATTENTION to any of these four lines. It doesn't matter which line you run
	// the test with, you won't get an error and the test always goes smoothly.
	req, err := http.NewRequest("GET", "/health", nil)
	// req, err := http.NewRequest("GET", "/nohealth", nil)
	// req, err := http.NewRequest("GET", "/somethingelse", nil)
	// req, err := http.NewRequest("GET", "/", nil)



	if err != nil {
		t.Fatal(err)
	}

	// We create a ResponseRecorder (which satisfies http.ResponseWriter) to record the response.
	rr := httptest.NewRecorder()

	// Our handlers satisfy http.Handler, so we can call their ServeHTTP method
	// directly and pass in our Request and ResponseRecorder.
	r.ServeHTTP(rr, req)

	// Check the status code is what we expect.
	if status := rr.Code; status != http.StatusOK {
		t.Errorf("handler returned wrong status code: got %v want %v",
			status, http.StatusOK)
	}

	// Check the response body is what we expect.
	expected := `we're alive`
	if rr.Body.String() != expected {
		t.Errorf("handler returned unexpected body: got %v want %v",
			rr.Body.String(), expected)
	}

}

If I'm right, during the creation of the new request I can just define the same thing every time. For example this: req, err := http.NewRequest("GET", "", nil)

In other words, it doesn't matter even if I write the exercise like this:

** Simple HTTP Handler **

// endpoints.go
package main

func HealthCheckHandler(w http.ResponseWriter, r *http.Request) {
    w.Header().Set("Content-Type", "application/json")
    w.WriteHeader(http.StatusOK)

    io.WriteString(w, `{"alive": true}`)
}

func HomepageHandler(w http.ResponseWriter, r *http.Request) {
    w.Header().Set("Content-Type", "application/json")
    w.WriteHeader(http.StatusOK)

    io.WriteString(w, `{"title": "IT IS MY HOMEPAGE"}`)
}

func main() {
    r := mux.NewRouter()
    r.HandleFunc("/health", HealthCheckHandler)
    r.HandleFunc("/home", HomepageHandler)

    log.Fatal(http.ListenAndServe("localhost:8080", r))
}

** Simple Test Code **

// endpoints_test.go
package main

import (
    "net/http"
    "net/http/httptest"
    "testing"
)

func TestHealthCheckHandler(t *testing.T) {
    // req, err := http.NewRequest("GET", "/health", nil)
    req, err := http.NewRequest("GET", "", nil) // <-- NOTE THE REPEATABILITY OF THIS LINE WITH THE EQUIVALENT IN THE TEST FUNCTION BELOW.
    if err != nil {
        t.Fatal(err)
    }

    rr := httptest.NewRecorder()
    handler := http.HandlerFunc(HealthCheckHandler)

    handler.ServeHTTP(rr, req)

    if status := rr.Code; status != http.StatusOK {
        t.Errorf("handler returned wrong status code: got %v want %v",
            status, http.StatusOK)
    }

    expected := `{"alive": true}`
    if rr.Body.String() != expected {
        t.Errorf("handler returned unexpected body: got %v want %v",
            rr.Body.String(), expected)
    }
}

func TestHomepageHandler(t *testing.T) {
    req, err := http.NewRequest("GET", "", nil) // <-- NOTE THE REPEATABILITY OF THIS LINE WITH THE EQUIVALENT IN THE PREVIOUS TEST FUNCTION.
    if err != nil {
        t.Fatal(err)
    }

    rr := httptest.NewRecorder()
    handler := http.HandlerFunc(HealthCheckHandler)

    handler.ServeHTTP(rr, req)

    if status := rr.Code; status != http.StatusOK {
        t.Errorf("handler returned wrong status code: got %v want %v",
            status, http.StatusOK)
    }

    expected := `{"title":"IT IS MY HOMEPAGE"}`
    if rr.Body.String() != expected {
        t.Errorf("handler returned unexpected body: got %v want %v",
            rr.Body.String(), expected)
    }
}

Am I right? And is it the correct behavior of the function or not?
Thanks for your clarification and time!

Answer 3 · 2022-11-04T20:08:59.000Z

Please excuse me if I didn't express myself clearly in the previous post! I tried to be as brief as possible. :)

hey, don't mind. I was a bit confused about what you were trying to do (it's not you, it's me 😅 ).

What I'm trying to explain is that it doesn't matter what the second argument I will pass to the function NewRequest().

Yes, you're right.

req, err := http.NewRequest("GET", "", nil), I would say to write it in each test case separately. Because it will save you from race condition problems going forward. Else you can create a function like this
https://github.com/gorilla/mux/blob/master/mux_test.go#L2870-L2904

Answer 4 · 2022-11-05T10:23:32.000Z

I found the answer to what I was looking for. NewRequest just wraps NewRequestWithContext using context.Background. It does not compare the actual URL request path defined in httpHandlers(). Apparently, it is normal behavior, so I'm closing the issue.

https://cs.opensource.google/go/go/+/refs/tags/go1.19.3:src/net/http/request.go;l=835
https://cs.opensource.google/go/go/+/refs/tags/go1.19.3:src/net/http/request.go;l=861

@amustaque97 Thank you for your help and recommendations!