From 02f7d3f3e2525ef8338c2636dc248f94dbd682e4 Mon Sep 17 00:00:00 2001
From: Chris Lu <chrislusf@users.noreply.github.com>
Date: Tue, 16 Dec 2025 13:21:15 -0800
Subject: [PATCH] Fix S3 server panic when -s3.port.https equals -s3.port
 (#7794)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* Fix volume repeatedly toggling between crowded and uncrowded

Fixes #6712

The issue was that removeFromCrowded() was called in removeFromWritable(),
which is invoked whenever a volume temporarily becomes unwritable (due to
replica count fluctuations, heartbeat issues, or read-only state changes).

This caused unnecessary toggling:
1. Volume becomes temporarily unwritable → removeFromWritable() →
   removeFromCrowded() logs 'becomes uncrowded'
2. Volume becomes writable again
3. CollectDeadNodeAndFullVolumes() runs → setVolumeCrowded() logs
   'becomes crowded'

The fix:
- Remove removeFromCrowded() call from removeFromWritable()
- Only clear crowded status when volume is fully unregistered from
  the layout (when location.Length() == 0 in UnRegisterVolume)

This ensures transient state changes don't cause log spam and the
crowded status accurately reflects the volume's size relative to
the grow threshold.

* Refactor test to use subtests for better readability

Address review feedback: use t.Run subtests to make the test's intent
clearer by giving each verification step a descriptive name.

* Fix S3 server panic when -s3.port.https equals -s3.port

When starting the S3 server with -s3.port.https=8333 (same as default
-s3.port), the server would panic with nil pointer dereference because:

1. The HTTP listener was already bound to port 8333
2. NewIpAndLocalListeners for HTTPS failed but error was discarded
3. ServeTLS was called on nil listener causing panic

This fix:
- Adds early validation to prevent using same port for HTTP and HTTPS
- Properly handles the error from NewIpAndLocalListeners for HTTPS

Fixes #7792
---
 weed/command/s3.go | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/weed/command/s3.go b/weed/command/s3.go
index 5691489f4..0a1296097 100644
--- a/weed/command/s3.go
+++ b/weed/command/s3.go
@@ -341,6 +341,11 @@ func (s3opt *S3Options) startS3Server() bool {
 	go grpcS.Serve(grpcL)
 
 	if *s3opt.tlsPrivateKey != "" {
+		// Check for port conflict when both HTTP and HTTPS are enabled on the same port
+		if *s3opt.portHttps > 0 && *s3opt.portHttps == *s3opt.port {
+			glog.Fatalf("S3 API Server error: -s3.port.https (%d) cannot be the same as -s3.port (%d)", *s3opt.portHttps, *s3opt.port)
+		}
+
 		pemfileOptions := pemfile.Options{
 			CertFile:        *s3opt.tlsCertificate,
 			KeyFile:         *s3opt.tlsPrivateKey,
@@ -388,8 +393,11 @@ func (s3opt *S3Options) startS3Server() bool {
 			}
 		} else {
 			glog.V(0).Infof("Start Seaweed S3 API Server %s at https port %d", version.Version(), *s3opt.portHttps)
-			s3ApiListenerHttps, s3ApiLocalListenerHttps, _ := util.NewIpAndLocalListeners(
+			s3ApiListenerHttps, s3ApiLocalListenerHttps, err := util.NewIpAndLocalListeners(
 				*s3opt.bindIp, *s3opt.portHttps, time.Duration(*s3opt.idleTimeout)*time.Second)
+			if err != nil {
+				glog.Fatalf("S3 API HTTPS listener on %s:%d error: %v", *s3opt.bindIp, *s3opt.portHttps, err)
+			}
 			if s3ApiLocalListenerHttps != nil {
 				go func() {
 					if err = newHttpServer(router, tlsConfig).ServeTLS(s3ApiLocalListenerHttps, "", ""); err != nil {