fix CORS handling, #840

server/hdl_files.go

@@ -39,6 +39,32 @@ func largeFileServe(wrt http.ResponseWriter, req *http.Request) {
 		}
 	}
 
+	// Preflight request: process before any security checks.
+	if req.Method == http.MethodOptions {
+		headers, statusCode, err := mh.Headers(req, true)
+		if err != nil {
+			writeHttpResponse(decodeStoreError(err, "", now, nil), err)
+			return
+		}
+		for name, values := range headers {
+			for _, value := range values {
+				wrt.Header().Add(name, value)
+			}
+		}
+		if statusCode <= 0 {
+			statusCode = http.StatusNoContent
+		}
+		wrt.WriteHeader(statusCode)
+		logs.Info.Println("media serve: preflight completed")
+		return
+	}
+
+	// Check if this is a GET/HEAD request.
+	if req.Method != http.MethodGet && req.Method != http.MethodHead {
+		writeHttpResponse(ErrOperationNotAllowed("", "", now), errors.New("method '"+req.Method+"' not allowed"))
+		return
+	}
+
 	// Check for API key presence
 	if isValid, _ := checkAPIKey(getAPIKey(req)); !isValid {
 		writeHttpResponse(ErrAPIKeyRequired(now), errors.New("invalid or missing API key"))
@@ -63,12 +89,6 @@ func largeFileServe(wrt http.ResponseWriter, req *http.Request) {
 		return
 	}
 
-	// Check if this is a GET/OPTIONS/HEAD request.
-	if req.Method != http.MethodGet && req.Method != http.MethodHead && req.Method != http.MethodOptions {
-		writeHttpResponse(ErrOperationNotAllowed("", "", now), errors.New("method '"+req.Method+"' not allowed"))
-		return
-	}
-
 	// Check if media handler redirects or adds headers.
 	headers, statusCode, err := mh.Headers(req, true)
 	if err != nil {
@@ -98,7 +118,7 @@ func largeFileServe(wrt http.ResponseWriter, req *http.Request) {
 		return
 	}
 
-	if req.Method == http.MethodHead || req.Method == http.MethodOptions {
+	if req.Method == http.MethodHead {
 		wrt.WriteHeader(http.StatusOK)
 		logs.Info.Println("media serve: completed", req.Method, "uid=", uid)
 		return
@@ -137,9 +157,28 @@ func largeFileReceive(wrt http.ResponseWriter, req *http.Request) {
 		}
 	}
 
-	// Check if this is a POST/PUT/OPTIONS/HEAD request.
-	if req.Method != http.MethodPost && req.Method != http.MethodPut &&
-		req.Method != http.MethodHead && req.Method != http.MethodOptions {
+	// Preflight request: process before any security checks.
+	if req.Method == http.MethodOptions {
+		headers, statusCode, err := mh.Headers(req, false)
+		if err != nil {
+			writeHttpResponse(decodeStoreError(err, "", now, nil), err)
+			return
+		}
+		for name, values := range headers {
+			for _, value := range values {
+				wrt.Header().Add(name, value)
+			}
+		}
+		if statusCode <= 0 {
+			statusCode = http.StatusNoContent
+		}
+		wrt.WriteHeader(statusCode)
+		logs.Info.Println("media upload: preflight completed")
+		return
+	}
+
+	// Check if this is a POST/PUT/HEAD request.
+	if req.Method != http.MethodPost && req.Method != http.MethodPut && req.Method != http.MethodHead {
 		writeHttpResponse(ErrOperationNotAllowed("", "", now), errors.New("method '"+req.Method+"' not allowed"))
 		return
 	}
@@ -175,7 +214,7 @@ func largeFileReceive(wrt http.ResponseWriter, req *http.Request) {
 	// Check if uploads are handled elsewhere.
 	headers, statusCode, err := mh.Headers(req, false)
 	if err != nil {
-		logs.Info.Println("Headers check failed", err)
+		logs.Info.Println("media upload: headers check failed", err)
 		writeHttpResponse(decodeStoreError(err, "", now, nil), err)
 		return
 	}
@@ -210,7 +249,7 @@ func largeFileReceive(wrt http.ResponseWriter, req *http.Request) {
 
 	file, _, err := req.FormFile("file")
 	if err != nil {
-		logs.Info.Println("Invalid multipart form", err)
+		logs.Info.Println("media upload: invalid multipart form", err)
 		if strings.Contains(err.Error(), "request body too large") {
 			writeHttpResponse(ErrTooLarge(msgID, "", now), err)
 		} else {
@@ -240,7 +279,7 @@ func largeFileReceive(wrt http.ResponseWriter, req *http.Request) {
 
 	url, size, err := mh.Upload(fdef, file)
 	if err != nil {
-		logs.Info.Println("Upload failed", file, "key", fdef.Location, err)
+		logs.Info.Println("media upload: failed", file, "key", fdef.Location, err)
 		store.Files.FinishUpload(fdef, false, 0)
 		writeHttpResponse(decodeStoreError(err, msgID, now, nil), err)
 		return
@@ -248,7 +287,7 @@ func largeFileReceive(wrt http.ResponseWriter, req *http.Request) {
 
 	fdef, err = store.Files.FinishUpload(fdef, true, size)
 	if err != nil {
-		logs.Info.Println("Failed to finalize upload", file, "key", fdef.Location, err)
+		logs.Info.Println("media upload: failed to finalize", file, "key", fdef.Location, err)
 		// Best effort cleanup.
 		mh.Delete([]string{fdef.Location})
 		writeHttpResponse(decodeStoreError(err, msgID, now, nil), err)

server/media/media.go

@@ -70,8 +70,9 @@ func matchCORSOrigin(allowed []string, origin string) string {
 		return "*"
 	}
 
+	origin = strings.ToLower(origin)
 	for _, val := range allowed {
-		if val == origin {
+		if strings.ToLower(val) == origin {
 			return origin
 		}
 	}
@@ -87,7 +88,7 @@ func matchCORSMethod(allowMethods []string, method string) bool {
 
 	method = strings.ToUpper(method)
 	for _, mm := range allowMethods {
-		if mm == method {
+		if strings.ToUpper(mm) == method {
 			return true
 		}
 	}
@@ -102,17 +103,6 @@ func CORSHandler(req *http.Request, allowedOrigins []string, serve bool) (http.H
 		return nil, 0
 	}
 
-	headers := map[string][]string{
-		// Always add Vary because of possible intermediate caches.
-		"Vary": {"Origin", "Access-Control-Request-Method"},
-	}
-
-	allowedOrigin := matchCORSOrigin(allowedOrigins, req.Header.Get("Origin"))
-	if allowedOrigin == "" {
-		// CORS policy does not match the origin.
-		return headers, http.StatusOK
-	}
-
 	var allowMethods []string
 	if serve {
 		allowMethods = []string{http.MethodGet, http.MethodHead, http.MethodOptions}
@@ -120,16 +110,27 @@ func CORSHandler(req *http.Request, allowedOrigins []string, serve bool) (http.H
 		allowMethods = []string{http.MethodPost, http.MethodPut, http.MethodHead, http.MethodOptions}
 	}
 
+	headers := map[string][]string{
+		// Always add Vary because of possible intermediate caches.
+		"Vary":                             {"Origin", "Access-Control-Request-Method"},
+		"Access-Control-Allow-Headers":     {"*"},
+		"Access-Control-Max-Age":           {"86400"},
+		"Access-Control-Allow-Credentials": {"true"},
+		"Access-Control-Allow-Methods":     {strings.Join(allowMethods, ", ")},
+	}
+
 	if !matchCORSMethod(allowMethods, req.Header.Get("Access-Control-Request-Method")) {
 		// CORS policy does not allow this method.
-		return headers, http.StatusOK
+		return headers, http.StatusNoContent
+	}
+
+	allowedOrigin := matchCORSOrigin(allowedOrigins, req.Header.Get("Origin"))
+	if allowedOrigin == "" {
+		// CORS policy does not match the origin.
+		return headers, http.StatusNoContent
 	}
 
 	headers["Access-Control-Allow-Origin"] = []string{allowedOrigin}
-	headers["Access-Control-Allow-Headers"] = []string{"*"}
-	headers["Access-Control-Allow-Methods"] = []string{strings.Join(allowMethods, ",")}
-	headers["Access-Control-Max-Age"] = []string{"86400"}
-	headers["Access-Control-Allow-Credentials"] = []string{"true"}
 
-	return headers, http.StatusOK
+	return headers, http.StatusNoContent
 }