How can I solve race condition for Golang WAF service?

user_15368176 0
Viewed 147

Firstly, sorry all for bad English.

I try to develop a WAF(Web Application Firewall) service in Golang. Everything is in the map[string]*Struct{} in memory. When request has come, I set request header's host to map in handler function.

host,err := GetHost(r.Host)

func GetHost(host string) (*Host,error){
    split, _, err := net.SplitHostPort(host)
    if err == nil {
        host = split
    }
    if data, val := hosts[host]; val { 
        return data, nil
    }
    return nil,errors.New("host not found!)
}
//hosts is a map for all host, key is host and value is host struct.

The problem is, map is messing when service has got a lot of request. For example; host is example.com but hosts["example.com"] gives an another value which is irrelevant.

type Server struct {
    mu   sync.RWMutex
    Host *models.Host
}

func (c *Server) handler(handler http.Handler) http.Handler {
    return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
    mutex.Lock()
    defer mutex.Unlock()
    var err error
    c.Host, err = GetHost(r.Host)
    if err != nil {
        w.WriteHeader(http.StatusBadGateway)
        w.Write([]byte(r.Host + " not found!!"))
        return
    }
    //it's going on..Edited part

So, I try to use mutex and wg for solving this problem but it didn't work. Open for any suggestion.

go race-condition
1 Answers

You're right in thinking that you need to serialise access to a shared state from a HTTP handler (because the server handles each HTTP request in a dedicated goroutine). Otherwise, your program would indeed suffer from a synchronization bug that would likely manifest itself as a data race during execution, as you seem to have experienced; the race detector provided with the Go toolchain would likely have picked up on it.

Arguably, the simplest way to serialize access to that shared state is to use some mutex. However, you need to be careful. Your deferred call to mutex.Unlock is problematic, for at least one, possibly two reasons:

  1. In general, you should endeavour to keep the critical section (the part of your code that surrounded by a call to Lock and Unlock) as "small" and "cheap" as possible. In short, the critical section should only do in-memory treatments, not I/O stuff. Here, the lock needs to be held during the entire treatment of each request, which is likely to cause a great deal of contention for your server.
  2. Although you omitted the end of the code in your handler, I'm guessing (?) that you also acquire the lock later in order to update the map (if the current request's host hasn't been encountered before). But, because none of the mutex types exported by package sync are re-entrant, you're likely to get a deadlock: due to the call to Unlock being deferred, the mutex will only get released when your handler terminates.

One solution consists in eschewing defer and restricting the critical section to the call to your GetHost function.

Another improvement would be to eliminate global state, for better testability, etc. You could make your hosts map non-global by simply smuggling storing it in a field of your Server struct and declaring GetHost as a method on *Server.

type Server struct {
    mu   sync.RWMutex
    Host *models.Host
    hosts map[string]*Host
}

func (srv *Server) GetHost(host string) (*Host, error){
    split, _, err := net.SplitHostPort(host)
    if err == nil {
        host = split
    }
    if data, exists := srv.hosts[host]; exists { 
        return data, nil
    }
    return nil, errors.New("host not found!")
}

func (srv *Server) handler(handler http.Handler) http.Handler {
    return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
        mutex.Lock()
        srv.Host, err := srv.GetHost(r.Host)
        mutex.Unlock()
        if err != nil {
            w.WriteHeader(http.StatusBadGateway)
            w.Write([]byte(r.Host + " not found!!"))
            return
        }
        // possibly acquire and release the lock again
        // for further treatment of the hosts map
    })
}

I may be missing something, but must admit I don't see the point in updating the Host field of your Server struct for each request...

user_25415730
Related