docs: analysis diff
- commit
- 60d0f7660f91951fa6064b3cf84b9bf4adcfd8fd
- author
- bamen · · 2025-03-24 06:36 UTC
- parent
- a1d9a53
- signature
- unsigned
1 file · +71 -0
A PROXY_ANALYSIS.md +71 -0
| @@ -0,0 +1,71 @@ | |||
| 1 | + | # Proxy Server Analysis: `feat/templ-integration` vs `dev` Branch | |
| 2 | + | ||
| 3 | + | ## Key Issue | |
| 4 | + | The proxy server in the `feat/templ-integration` branch is no longer receiving requests on both HTTP and HTTPS ports, while it worked properly in the `dev` branch. | |
| 5 | + | ||
| 6 | + | ## Root Causes | |
| 7 | + | ||
| 8 | + | 1. **Server Binding Method Changed**: | |
| 9 | + | - **dev branch (working)**: Uses standard approach with `httpServer.ListenAndServe()` and `httpsServer.ListenAndServeTLS("", "")` | |
| 10 | + | - **templ-integration branch (broken)**: Uses explicit IPv4 binding with `net.Listen("tcp4", "0.0.0.0:port")` first, then falls back if that fails | |
| 11 | + | ||
| 12 | + | 2. **Port Configuration Modifications**: | |
| 13 | + | - **templ-integration branch** added a new `configureProxyPorts()` function that automatically changes privileged ports to non-privileged ones for non-root users | |
| 14 | + | - Port 80 → 8080 and port 443 → 8443 when running outside containers as non-root | |
| 15 | + | ||
| 16 | + | 3. **Enhanced Admin Route Handling**: | |
| 17 | + | - More complex admin route handling with several verification steps and recovery mechanisms | |
| 18 | + | - Multiple connection tests and fallbacks can create race conditions | |
| 19 | + | ||
| 20 | + | 4. **Port Conflict Detection**: | |
| 21 | + | - More aggressive conflict detection which might be incorrectly disabling the proxy | |
| 22 | + | ||
| 23 | + | ## Specific Code Differences | |
| 24 | + | ||
| 25 | + | ### Server Binding | |
| 26 | + | ```go | |
| 27 | + | // templ-integration (broken) | |
| 28 | + | // First try explicit IPv4 binding | |
| 29 | + | httpsAddr := fmt.Sprintf("0.0.0.0:%s", p.config.Port) | |
| 30 | + | tcpListener, err := net.Listen("tcp4", httpsAddr) | |
| 31 | + | ||
| 32 | + | if err != nil { | |
| 33 | + | logger.Warn("Failed to create HTTPS listener...") | |
| 34 | + | // Try simple binding as fallback | |
| 35 | + | if err := httpsServer.ListenAndServeTLS("", ""); err != nil { | |
| 36 | + | logger.Error("HTTPS server failed", "error", err) | |
| 37 | + | } | |
| 38 | + | return | |
| 39 | + | } | |
| 40 | + | ||
| 41 | + | // Create TLS listener | |
| 42 | + | tlsListener := tls.NewListener(tcpListener, httpsServer.TLSConfig) | |
| 43 | + | if err := httpsServer.Serve(tlsListener); err != nil { | |
| 44 | + | logger.Error("HTTPS server failed", "error", err) | |
| 45 | + | } | |
| 46 | + | ||
| 47 | + | // dev branch (working) | |
| 48 | + | logger.Info("Starting HTTPS server", "address", httpsServer.Addr) | |
| 49 | + | // Using empty strings for cert and key files since we're using GetCertificate | |
| 50 | + | if err := httpsServer.ListenAndServeTLS("", ""); err != nil && err != http.ErrServerClosed { | |
| 51 | + | logger.Error("HTTPS server failed", "error", err) | |
| 52 | + | } | |
| 53 | + | ``` | |
| 54 | + | ||
| 55 | + | ### Testing and Verification | |
| 56 | + | The `templ-integration` branch adds extensive connection testing code: | |
| 57 | + | - Port binding verification | |
| 58 | + | - Multiple hostname tests | |
| 59 | + | - Complex fallback mechanisms | |
| 60 | + | - Periodic admin route verification | |
| 61 | + | ||
| 62 | + | ## Recommended Fix | |
| 63 | + | ||
| 64 | + | To fix the issue, we should: | |
| 65 | + | ||
| 66 | + | 1. Revert to the simple binding method from the `dev` branch in both `startHTTPSServer()` and `startHTTPServer()` functions | |
| 67 | + | 2. Simplify the admin connection testing logic | |
| 68 | + | 3. Keep the port conflict detection but ensure it doesn't incorrectly disable the proxy | |
| 69 | + | 4. Review the container environment detection to ensure it works properly | |
| 70 | + | ||
| 71 | + | The simplest fix would be to modify the server binding methods to use the direct approach from the `dev` branch that was working correctly.⏎̸ | |
| @@ -0,0 +1,71 @@ | |||||
| 1 | + | # Proxy Server Analysis: `feat/templ-integration` vs `dev` Branch | |||
| 2 | + | ||||
| 3 | + | ## Key Issue | |||
| 4 | + | The proxy server in the `feat/templ-integration` branch is no longer receiving requests on both HTTP and HTTPS ports, while it worked properly in the `dev` branch. | |||
| 5 | + | ||||
| 6 | + | ## Root Causes | |||
| 7 | + | ||||
| 8 | + | 1. **Server Binding Method Changed**: | |||
| 9 | + | - **dev branch (working)**: Uses standard approach with `httpServer.ListenAndServe()` and `httpsServer.ListenAndServeTLS("", "")` | |||
| 10 | + | - **templ-integration branch (broken)**: Uses explicit IPv4 binding with `net.Listen("tcp4", "0.0.0.0:port")` first, then falls back if that fails | |||
| 11 | + | ||||
| 12 | + | 2. **Port Configuration Modifications**: | |||
| 13 | + | - **templ-integration branch** added a new `configureProxyPorts()` function that automatically changes privileged ports to non-privileged ones for non-root users | |||
| 14 | + | - Port 80 → 8080 and port 443 → 8443 when running outside containers as non-root | |||
| 15 | + | ||||
| 16 | + | 3. **Enhanced Admin Route Handling**: | |||
| 17 | + | - More complex admin route handling with several verification steps and recovery mechanisms | |||
| 18 | + | - Multiple connection tests and fallbacks can create race conditions | |||
| 19 | + | ||||
| 20 | + | 4. **Port Conflict Detection**: | |||
| 21 | + | - More aggressive conflict detection which might be incorrectly disabling the proxy | |||
| 22 | + | ||||
| 23 | + | ## Specific Code Differences | |||
| 24 | + | ||||
| 25 | + | ### Server Binding | |||
| 26 | + | ```go | |||
| 27 | + | // templ-integration (broken) | |||
| 28 | + | // First try explicit IPv4 binding | |||
| 29 | + | httpsAddr := fmt.Sprintf("0.0.0.0:%s", p.config.Port) | |||
| 30 | + | tcpListener, err := net.Listen("tcp4", httpsAddr) | |||
| 31 | + | ||||
| 32 | + | if err != nil { | |||
| 33 | + | logger.Warn("Failed to create HTTPS listener...") | |||
| 34 | + | // Try simple binding as fallback | |||
| 35 | + | if err := httpsServer.ListenAndServeTLS("", ""); err != nil { | |||
| 36 | + | logger.Error("HTTPS server failed", "error", err) | |||
| 37 | + | } | |||
| 38 | + | return | |||
| 39 | + | } | |||
| 40 | + | ||||
| 41 | + | // Create TLS listener | |||
| 42 | + | tlsListener := tls.NewListener(tcpListener, httpsServer.TLSConfig) | |||
| 43 | + | if err := httpsServer.Serve(tlsListener); err != nil { | |||
| 44 | + | logger.Error("HTTPS server failed", "error", err) | |||
| 45 | + | } | |||
| 46 | + | ||||
| 47 | + | // dev branch (working) | |||
| 48 | + | logger.Info("Starting HTTPS server", "address", httpsServer.Addr) | |||
| 49 | + | // Using empty strings for cert and key files since we're using GetCertificate | |||
| 50 | + | if err := httpsServer.ListenAndServeTLS("", ""); err != nil && err != http.ErrServerClosed { | |||
| 51 | + | logger.Error("HTTPS server failed", "error", err) | |||
| 52 | + | } | |||
| 53 | + | ``` | |||
| 54 | + | ||||
| 55 | + | ### Testing and Verification | |||
| 56 | + | The `templ-integration` branch adds extensive connection testing code: | |||
| 57 | + | - Port binding verification | |||
| 58 | + | - Multiple hostname tests | |||
| 59 | + | - Complex fallback mechanisms | |||
| 60 | + | - Periodic admin route verification | |||
| 61 | + | ||||
| 62 | + | ## Recommended Fix | |||
| 63 | + | ||||
| 64 | + | To fix the issue, we should: | |||
| 65 | + | ||||
| 66 | + | 1. Revert to the simple binding method from the `dev` branch in both `startHTTPSServer()` and `startHTTPServer()` functions | |||
| 67 | + | 2. Simplify the admin connection testing logic | |||
| 68 | + | 3. Keep the port conflict detection but ensure it doesn't incorrectly disable the proxy | |||
| 69 | + | 4. Review the container environment detection to ensure it works properly | |||
| 70 | + | ||||
| 71 | + | The simplest fix would be to modify the server binding methods to use the direct approach from the `dev` branch that was working correctly.⏎̸ | |||