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

fasthttp not writing headers on error #444

Closed
kostrahb opened this issue Oct 25, 2018 · 3 comments
Closed

fasthttp not writing headers on error #444

kostrahb opened this issue Oct 25, 2018 · 3 comments

Comments

@kostrahb
Copy link

kostrahb commented Oct 25, 2018

I am using fasthttp-routing to make a server side application and I have just tried to add a header with allowed authentication when returning http error 401. However, I can't see the header in the response. After some investigation, I concluded that the problem is in this library in file server.go in function writeFastError.

func (s *Server) writeFastError(w io.Writer, statusCode int, msg string) {
	w.Write(statusLine(statusCode))

	server := ""
	if !s.NoDefaultServerHeader {
		server = fmt.Sprintf("Server: %s\r\n", s.getServerName())
	}

	fmt.Fprintf(w, "Connection: close\r\n"+
		server+
		"Date: %s\r\n"+
		"Content-Type: text/plain\r\n"+
		"Content-Length: %d\r\n"+
		"\r\n"+
		"%s",
		serverDate.Load(), len(msg), msg)
}

Would it be possible for you to add some function to write all headers together with the message?

@erikdubbelboer
Copy link
Collaborator

writeFastError is only used in case there is an internal error in fasthttp, you can't call it in your code. Also if writeFastError is called the request handler isn't called so your code isn't executed. Are you sure this function is causing your issue?

@kostrahb
Copy link
Author

kostrahb commented Oct 25, 2018

Well, after thorough debugging I am not exactly sure anymore if the issue is in fasthttp or fasthttp-routing. Certainly it is not in mentioned writeFastError. What I have found is that in fasthttp-routing, the handler function is the following:

func (r *Router) HandleRequest(ctx *fasthttp.RequestCtx) {
	c := r.pool.Get().(*Context)
	c.init(ctx)
	c.handlers, c.pnames = r.find(string(ctx.Method()), string(ctx.Path()), c.pvalues)
	if err := c.Next(); err != nil {
		r.handleError(c, err)
	}
	r.pool.Put(c)
}

All registered handlers for each path are evaluated in c.Next() method and in this function one of my handlers returns an error and c.Next() returns that error too. The returned error is routing.httpError which is basically just standard error with the code number (and a function to print it). Anyway, this does not matter that much because in r.handleError() function

func (r *Router) handleError(c *Context, err error) {
	if httpError, ok := err.(HTTPError); ok {
		c.Error(httpError.Error(), httpError.StatusCode())
	} else {
		c.Error(err.Error(), http.StatusInternalServerError)
	}
}

c.Error(msg string, statusCode int) is run either with standard error and code 500 or with HTTP error with the corresponding number.
Now the c is of type *routing.Context and this type has embedded *fasthttp.RequestCtx which implements function Error(msg string, statusCode int). As this function was not overwritten, the piece of code which is run is following and you can find it in this repository in file server.go on line 943:

func (ctx *RequestCtx) Error(msg string, statusCode int) {
	ctx.Response.Reset()
	ctx.SetStatusCode(statusCode)
	ctx.SetContentTypeBytes(defaultContentType)
	ctx.SetBodyString(msg)
}

Apparently, no custom headers are added to the response, only status code and content type. However, the question is if this is intentional or not.
Is there possibly any other error function which would make the response contain all headers that were set by handlers?

@erikdubbelboer
Copy link
Collaborator

Judging by the code I guess it was intentional. It is possible that the user already set a header such as Content-Encoding which would then completely mess up the response. I guess for this reason it was decided to reset the response. If people want to have more control over how an error response is generated they can do it themselves. This is just a simple helper method it doesn't use any internal methods so everything can be done by the users code as well.

Now the question is why fasthttp-routing choose to use this method instead of giving the user more control over the response. For that I suggest you open an issue there.

The fact remains that we can't change this behavior anymore now without breaking things for other users.

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