valyala / fasthttp

Fast HTTP package for Go. Tuned for high performance. Zero memory allocations in hot paths. Up to 10x faster than net/http
MIT License
21.67k stars 1.75k forks source link

why skip err check in request.URI #1639

Closed adamzhoul closed 10 months ago

adamzhoul commented 10 months ago

Hi everyone

we encountered a problem : we use request.URI().Path() to get request path. get a: "/".(should be "/abcd").

from tcpflow we got:

010.024.078.088.39012-010.024.073.154.18072: POST /abcd HTTP/1.1
transfer-encoding: chunked
Host: this.is.the.host:-1

The Host part is wrong. But the URL path is correct. so we dig into the code, and find this:

https://github.com/valyala/fasthttp/blob/42bd7bb7e27a1c529b908892f9ce73f52cfd2292/http.go#L898-L901

https://github.com/valyala/fasthttp/blob/42bd7bb7e27a1c529b908892f9ce73f52cfd2292/uri.go#L314-L320

So, why do we skip the error handle here? The host is not important much, compared to the URL. how do we face this problem? since it covered the true problem. from the user's perspective, its URL is missing.

adamzhoul commented 10 months ago

can we skip the return err. since url parser is irrelevant to the above host parse.

erikdubbelboer commented 10 months ago

Can you show some basic code that shows what is going wrong?

I don't think we can just remove the return err that would be a backwards incompatible change of our API.

adamzhoul commented 10 months ago

I'm sorry for not getting back to you sooner.

think of this case, someone visits you like :

// a bad port in host header cur HTTP://1.2.3.4/api/v1/getresource/a -H"Host: this.is.the.host:-1"

as in our code:

// restful get resource a
func getResource( w ...,req  *Reques) {
  path := request.URI().Path()
  // and we get "/ " here, it should be " /api/v1/getresource/a"
  ....
}

if parser error, return "/" as the path covered the real problem.

erikdubbelboer commented 10 months ago

I get it now, but broken clients is not something we're going to break backwards compatibility for. Fasthttp is designed for speed where usually you have control over both the client and server. If you want to support all kinds of broken clients I would really recommend just using net/http which is designed to work with these cases.