fix: Status Line parsing and writing

[ShivanshVij]

This is a reference to #1132 and it corrects a number of bugs (while keeping zero-allocation)

First, it corrects loopholelabs@36ba831#diff-445b7d49a79bb217c576f137077e18262a0a2d58e424239fcc01a437397188f7R1883 where instead of comparing the status message to the formatted status message (which is what is returned by the statusLine function), it instead compares the status message to the statusLines map entry.

Next, it modifies the SetStatusLine function (loopholelabs@36ba831#diff-445b7d49a79bb217c576f137077e18262a0a2d58e424239fcc01a437397188f7R149) to require a status code as well as a status message, and this is used to generate a properly formatted status line using the new formatStatusLine function (loopholelabs@36ba831#diff-99087af835ba003ecaa8e8e178095e102304dd3b2aef74476ab06ea1e615601cR176).

Finally, in order to convert the statusCode integer to a byte slice with zero-allocations, a new i2b function has been created (loopholelabs@36ba831#diff-24779539a7f9c30594f395bab66c981b5069e92b84157cea9fa77ca1a6dbc9f1R369), which is just a slightly modified version of strconv.formatBits

[erikdubbelboer]

I have been thinking about this and I'm really wondering if we should get rid of the pre-computed status messages so our code gets a bit cleaner. I have been running some benchmarks and calling formatStatusLine takes around 22 nanoseconds on my machine vs 3 nanoseconds for the pre-computed lookup. The code being 20 nanoseconds slower is something body is ever going to notice.

Can you modify your pull request to change statusLines and just always call formatStatusLine. That way this whole if statement isn't needed either and it can just always call SetStatusMessage:

fasthttp/header.go

Line 1905 in 0c70e19

if len(b) > n+1 && !bytes.Equal(b[n+1:], s2b(StatusMessage(h.statusCode))) {

[ShivanshVij]

@erikdubbelboer That makes sense. I'm sure doing the whole bytes.Equal is taking a couple nanoseconds as well that it won't anymore.

I've gone ahead and changed the StatusMessage function so that it returns a byte slice (as opposed to a string), that we can then feed directly into formatStatusLine.

Then, as suggested, I changed the parseFirstLine function so it always calls the SetStatusMessage function.

The reason I didn't just delete the StatusMessage function and instead modified it, is because it's used in the server.writeFastError function:

func (s *Server) writeFastError(w io.Writer, statusCode int, msg string) {
	w.Write(formatStatusLine(nil, strHTTP11, statusCode, StatusMessage(statusCode))) //nolint:errcheck
	...
}

Another option is for us to convert the msg parameter in the server.writeFastError function to a byte slice, and use that instead of calling StatusMessage:

func (s *Server) writeFastError(w io.Writer, statusCode int, msg string) {
	w.Write(formatStatusLine(nil, strHTTP11, statusCode, s2b(msg))) //nolint:errcheck
	...
}

But I don't think that's a valid approach since writeFastError is often called with non-standard messages:

func (s *Server) ServeConn(c net.Conn) error {
        ...
	if n > uint32(s.getConcurrency()) {
		atomic.AddUint32(&s.concurrency, ^uint32(0))
		s.writeFastError(c, StatusServiceUnavailable, "The connection cannot be served because Server.Concurrency limit exceeded")
		c.Close()
		return ErrConcurrencyLimit
	}
	...
}

[ShivanshVij]

Alright, I think that should be everything. Sorry for the delay in those last few changes!

[erikdubbelboer]

Just one last test that is failing, after that I'll merge it.

[erikdubbelboer]

Thanks!