Add confirmation dialogue for deleting recordings. Normally I hate those "Are your really sure" dialogues, but this is a good place for one because you don't want to accidently delete a recording with no backup.

2026-07-07 23:12:03 -04:00
parent 0c852e8c41
commit 4fff2fec7f
3 changed files with 116 additions and 14 deletions
@@ -2,7 +2,7 @@
 # shellcheck shell=bash disable=SC2034,SC2154

 pkgname=skald-git
-pkgver=0.0.0.r1552.ge31bffd
+pkgver=0.0.0.r1553.g0c852e8
 pkgrel=1
 pkgdesc='Audio-only hall-based conferencing server'
 arch=('x86_64' 'aarch64')
@@ -689,17 +689,16 @@ func handleHallAction(w http.ResponseWriter, r *http.Request, hall string) {
 	q := r.Form.Get("q")

 	switch q {
-	case "delete":
-		filename := r.Form.Get("filename")
-		if hall == "" || filename == "" {
-			http.Error(w, "No filename provided",
-				http.StatusBadRequest)
+	case "confirm-delete":
+		filename, ok := recordingFilenameFromForm(w, r, hall)
+		if !ok {
 			return
 		}
-		if strings.ContainsRune(filename, '/') ||
-			strings.ContainsRune(filename, filepath.Separator) {
-			http.Error(w, "Bad character in filename",
-				http.StatusBadRequest)
+		serveDeleteRecordingConfirmation(w, hall, filename)
+		return
+	case "delete":
+		filename, ok := recordingFilenameFromForm(w, r, hall)
+		if !ok {
 			return
 		}
 		err := os.Remove(
@@ -713,7 +712,7 @@ func handleHallAction(w http.ResponseWriter, r *http.Request, hall string) {
 			httpError(w, err)
 			return
 		}
-		http.Redirect(w, r, "/recordings/"+hall+"/",
+		http.Redirect(w, r, recordingsHallPath(hall),
 			http.StatusSeeOther)
 		return
 	default:
@@ -721,6 +720,22 @@ func handleHallAction(w http.ResponseWriter, r *http.Request, hall string) {
 	}
 }

+func recordingFilenameFromForm(w http.ResponseWriter, r *http.Request, hall string) (string, bool) {
+	filename := r.Form.Get("filename")
+	if hall == "" || filename == "" {
+		http.Error(w, "No filename provided",
+			http.StatusBadRequest)
+		return "", false
+	}
+	if strings.ContainsRune(filename, '/') ||
+		strings.ContainsRune(filename, filepath.Separator) {
+		http.Error(w, "Bad character in filename",
+			http.StatusBadRequest)
+		return "", false
+	}
+	return filename, true
+}
+
 func checkHallPermissions(w http.ResponseWriter, r *http.Request, hallname string) bool {
 	user, pass, ok := r.BasicAuth()
 	if !ok {
@@ -758,6 +773,37 @@ func checkHallPermissions(w http.ResponseWriter, r *http.Request, hallname strin
 	return true
 }

+func recordingsHallPath(hall string) string {
+	parts := strings.Split(hall, "/")
+	for i, part := range parts {
+		parts[i] = url.PathEscape(part)
+	}
+	return "/recordings/" + strings.Join(parts, "/") + "/"
+}
+
+func serveDeleteRecordingConfirmation(w http.ResponseWriter, hall string, filename string) {
+	hallPath := recordingsHallPath(hall)
+	escapedFilename := html.EscapeString(filename)
+
+	w.Header().Set("content-type", "text/html; charset=utf-8")
+	w.Header().Set("cache-control", "no-cache")
+
+	fmt.Fprintf(w, "<!DOCTYPE html>\n<html><head>\n")
+	fmt.Fprintf(w, "<title>Delete recording %v</title>\n", escapedFilename)
+	fmt.Fprintf(w, "<link rel=\"stylesheet\" type=\"text/css\" href=\"/common.css\"/>")
+	fmt.Fprintf(w, "</head><body>\n")
+	fmt.Fprintf(w, "<h1>Delete recording?</h1>\n")
+	fmt.Fprintf(w, "<p>Delete %v? This cannot be undone.</p>\n", escapedFilename)
+	fmt.Fprintf(w,
+		"<form action=\"%v\" method=\"post\">"+
+			"<input type=\"hidden\" name=\"filename\" value=\"%v\">"+
+			"<button type=\"submit\" name=\"q\" value=\"delete\">Delete recording</button>"+
+			"</form>\n",
+		hallPath, escapedFilename)
+	fmt.Fprintf(w, "<p><a href=\"%v\">Cancel</a></p>\n", hallPath)
+	fmt.Fprintf(w, "</body></html>\n")
+}
+
 func serveHallRecordings(w http.ResponseWriter, r *http.Request, f *os.File, hall string) {
 	// read early, so we return permission errors to HEAD
 	fis, err := f.Readdir(-1)
@@ -793,11 +839,12 @@ func serveHallRecordings(w http.ResponseWriter, r *http.Request, f *os.File, hal
 			fi.Size(),
 		)
 		fmt.Fprintf(w,
-			"<td><form action=\"/recordings/%v/\" method=\"post\">"+
+			"<td><form action=\"%v\" method=\"post\">"+
 				"<input type=\"hidden\" name=\"filename\" value=\"%v\">"+
-				"<button type=\"submit\" name=\"q\" value=\"delete\">Delete</button>"+
+				"<button type=\"submit\" name=\"q\" value=\"confirm-delete\">Delete</button>"+
 				"</form></td></tr>\n",
-			url.PathEscape(hall), fi.Name())
+			recordingsHallPath(hall),
+			html.EscapeString(fi.Name()))
 	}
 	fmt.Fprintf(w, "</table>\n")
 	fmt.Fprintf(w, "</body></html>\n")
@@ -59,6 +59,61 @@ func TestRecordingsAuthenticationFailureMessage(t *testing.T) {
 	}
 }

+func TestHallRecordingsDeleteButtonOpensConfirmation(t *testing.T) {
+	dir := t.TempDir()
+	recordingName := `skald-recording-20260707-"010203".ogg`
+	err := os.WriteFile(filepath.Join(dir, recordingName), nil, 0600)
+	if err != nil {
+		t.Fatalf("write recording: %v", err)
+	}
+	f, err := os.Open(dir)
+	if err != nil {
+		t.Fatalf("open recordings dir: %v", err)
+	}
+	defer f.Close()
+
+	w := httptest.NewRecorder()
+	r := httptest.NewRequest("GET", "/recordings/class/audio/", nil)
+	serveHallRecordings(w, r, f, "class/audio")
+
+	body := w.Body.String()
+	if !strings.Contains(body, `action="/recordings/class/audio/"`) {
+		t.Fatalf("recordings list missing encoded hall action: %q", body)
+	}
+	if !strings.Contains(body, `name="q" value="confirm-delete">Delete</button>`) {
+		t.Fatalf("delete button does not open confirmation: %q", body)
+	}
+	if strings.Contains(body, `name="q" value="delete">Delete</button>`) {
+		t.Fatalf("recordings list still has direct delete button: %q", body)
+	}
+	if !strings.Contains(body, `value="skald-recording-20260707-&#34;010203&#34;.ogg"`) {
+		t.Fatalf("recordings list did not escape filename value: %q", body)
+	}
+}
+
+func TestHallRecordingsDeleteConfirmationRequiresSecondPost(t *testing.T) {
+	w := httptest.NewRecorder()
+	r := httptest.NewRequest("POST", "/recordings/class/audio/",
+		strings.NewReader("q=confirm-delete&filename=skald-recording-20260707-010203.ogg"))
+	r.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+
+	handleHallAction(w, r, "class/audio")
+
+	body := w.Body.String()
+	if !strings.Contains(body, "<h1>Delete recording?</h1>") {
+		t.Fatalf("confirmation page missing heading: %q", body)
+	}
+	if !strings.Contains(body, `action="/recordings/class/audio/"`) {
+		t.Fatalf("confirmation page missing encoded hall action: %q", body)
+	}
+	if !strings.Contains(body, `name="q" value="delete">Delete recording</button>`) {
+		t.Fatalf("confirmation page missing final delete button: %q", body)
+	}
+	if !strings.Contains(body, `<a href="/recordings/class/audio/">Cancel</a>`) {
+		t.Fatalf("confirmation page missing cancel link: %q", body)
+	}
+}
+
 func TestBase(t *testing.T) {
 	a := []struct {
 		p      string