starred/mailpit

Fork 0

mirror of https://github.com/axllent/mailpit.git synced 2026-04-26 00:35:51 +03:00

[GH-ISSUE #207] Using websocket causes socket leaks in nginx #138

New issue

Closed

opened 2026-03-15 12:48:21 +03:00 by kerem · 5 comments

kerem commented

2026-03-15 12:48:21 +03:00

Owner

Originally created by @andreymal on GitHub (Nov 10, 2023).
Original GitHub issue: https://github.com/axllent/mailpit/issues/207

I use nginx with configuration similar to this:

http {
  # ...

  map $http_upgrade $connection_upgrade {
    default  upgrade;
    ''       close;
  }

  server {
    # ...

    location /mailpit/ {
      proxy_pass http://127.0.0.1:8025;

      proxy_http_version 1.1;
      proxy_set_header Upgrade $http_upgrade;
      proxy_set_header Connection $connection_upgrade;

      proxy_set_header Host $http_host;
      proxy_set_header X-Real-IP $remote_addr;
      proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
      proxy_set_header X-Forwarded-Proto $scheme;
    }
  }
}

and after these steps:

Open mailpit in the browser (which creates a websocket connection);
Close the tab (which closes the connection);
Restart nginx within ~20 seconds —

I've noticed that restarting nginx is taking longer than usual. I checked /var/log/nginx/error.log and found this:

2023/11/10 17:04:37 [alert] 7854#7854: *8 open socket #26 left in connection 6
2023/11/10 17:04:37 [alert] 7854#7854: *7 open socket #25 left in connection 7
2023/11/10 17:04:37 [alert] 7854#7854: aborting

I tested several other projects that also use websockets, but could not reproduce it. Only Mailpit causes this error.

I think this is because, unlike other projects, Mailpit doesn't try to read anything from the websocket and therefore doesn't know it's already closed.

If my assumption is correct, this error could be solved by adding a loop that reads the websocket and does nothing else, something like (disclaimer — I'm not a Go expert):

for {
    _, r, err := c.conn.NextReader()
    if err != nil {
        // gracefully close the connection
        // c.conn.Close(), unregister in the hub or something
        break
    }

    // drop the websocket message
    _, err = io.Copy(io.Discard, r)
}

Originally created by @andreymal on GitHub (Nov 10, 2023). Original GitHub issue: https://github.com/axllent/mailpit/issues/207 I use nginx with configuration similar to this: <details> ``` http { # ... map $http_upgrade $connection_upgrade { default upgrade; '' close; } server { # ... location /mailpit/ { proxy_pass http://127.0.0.1:8025; proxy_http_version 1.1; proxy_set_header Upgrade $http_upgrade; proxy_set_header Connection $connection_upgrade; proxy_set_header Host $http_host; proxy_set_header X-Real-IP $remote_addr; proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; proxy_set_header X-Forwarded-Proto $scheme; } } } ``` </details> and after these steps: 1. Open mailpit in the browser (which creates a websocket connection); 2. Close the tab (which closes the connection); 3. Restart nginx within ~20 seconds — I've noticed that restarting nginx is taking longer than usual. I checked `/var/log/nginx/error.log` and found this: ``` 2023/11/10 17:04:37 [alert] 7854#7854: *8 open socket #26 left in connection 6 2023/11/10 17:04:37 [alert] 7854#7854: *7 open socket #25 left in connection 7 2023/11/10 17:04:37 [alert] 7854#7854: aborting ``` I tested several other projects that also use websockets, but could not reproduce it. Only Mailpit causes this error. I think this is because, unlike other projects, Mailpit doesn't try to read anything from the websocket and therefore doesn't know it's already closed. If my assumption is correct, this error could be solved by adding a loop that reads the websocket and does nothing else, something like (disclaimer — I'm not a Go expert): ```golang for { _, r, err := c.conn.NextReader() if err != nil { // gracefully close the connection // c.conn.Close(), unregister in the hub or something break } // drop the websocket message _, err = io.Copy(io.Discard, r) } ```

kerem

2026-03-15 12:48:21 +03:00

closed this issue
added the
bug
label

kerem commented

2026-03-15 12:48:37 +03:00

Author

Owner

@axllent commented on GitHub (Nov 10, 2023):

I believe you're absolutely correct @andreymal, the websockets do appear to be left open it seems (disregarding the nginx entirely). I think they time out eventually, but a proactive approach is needed. I think I've found a solution but I need to do a bit more testing first.

@axllent commented on GitHub (Nov 10, 2023): I believe you're absolutely correct @andreymal, the websockets do appear to be left open it seems (disregarding the nginx entirely). I think they time out eventually, but a proactive approach is needed. I think I've found a solution but I need to do a bit more testing first.

kerem commented

2026-03-15 12:48:42 +03:00

Author

Owner

@axllent commented on GitHub (Nov 11, 2023):

@andreymal A fix for this has just been released in v1.10.0. Please confirm this solves your issue? Thanks.

@axllent commented on GitHub (Nov 11, 2023): @andreymal A fix for this has just been released in [v1.10.0](https://github.com/axllent/mailpit/releases/tag/v1.10.0). Please confirm this solves your issue? Thanks.

kerem commented

2026-03-15 12:48:47 +03:00

Author

Owner

@andreymal commented on GitHub (Nov 11, 2023):

@axllent yes 👍

_{(but using ReadMessage instead of NextReader allocates memory, and an evil client could kill Mailpit by sending a giant message like websocket.send(new ArrayBuffer(1024 * 1024 * 1024)), but if you don't care about evil clients then okay)}

@andreymal commented on GitHub (Nov 11, 2023): @axllent yes 👍 <sub>(but using `ReadMessage` instead of `NextReader` allocates memory, and an evil client could kill Mailpit by sending a giant message like `websocket.send(new ArrayBuffer(1024 * 1024 * 1024))`, but if you don't care about evil clients then okay)</sub>

kerem commented

2026-03-15 12:48:52 +03:00

Author

Owner

@axllent commented on GitHub (Nov 11, 2023):

Awesome, thanks for the quick feedback, and the note re: NextReader() - you're right about that (hadn't crossed my mind). I have made that change which will be included in the next release.

@axllent commented on GitHub (Nov 11, 2023): Awesome, thanks for the quick feedback, and the note re: `NextReader()` - you're right about that (hadn't crossed my mind). I have made that change which will be included in the next release.

kerem commented

2026-03-15 12:48:57 +03:00

Author

Owner

@axllent commented on GitHub (Nov 19, 2023):

Just a FYI - the switch to NextReader() was included in the v1.10.1 release 👍

@axllent commented on GitHub (Nov 19, 2023): Just a FYI - the switch to `NextReader()` was included in the v1.10.1 release :+1:

kerem referenced this issue

2026-03-15 14:19:38 +03:00

[PR #138] [CLOSED] Bump esbuild from 0.18.10 to 0.18.11 #447