fix(podcasts): download_all binds on every wire — FlexBool + flip log
Reproduced the §0 item 2 suspect: echo v5 bool-binds form values via strconv.ParseBool, which accepts "true"/"1" but REJECTS the browser-native <input type=checkbox> value "on" — and a bind failure kills the WHOLE AddFeed request (400 "invalid request"), not just the checkbox. The add modal posts urlencoded (htmx), so any checkbox-serialization that emits "on" made Download All silently unusable. Fix: FlexBool (encoding.TextUnmarshaler-style UnmarshalParam) binds download_all permissively on every wire — "true"/"on"/"yes"/"checked" => true, explicit false/0/off => false, ABSENCE (unchecked box on any wire) => false; JSON bool binding is native and unaffected. Tolerates the exact wires htmx and browsers emit. AddFeed's subscription flip now logs "[PODCAST] flip ... <feed> — user" so the live flip is provable in app logs (user suspected it never fired). Integration test TestPodcastDownloadAllFormEncoding pins the urlencoded wire (what htmx actually sends): true + on both flip the caller's policy to 'all', an unchecked add leaves the default.
This commit is contained in:
John O'Keefe committed
1 parent
cdf81f6cf4
commit
7c7c08748e
2 files changed
+130
-12
No files matched your search
@@ -11,6 +11,7 @@ package main
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"net/url"
|
||||
"context"
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
@@ -733,6 +734,22 @@ func TestPodcastPageSearchEpisodes(t *testing.T) {
|
||||
|
||||
// ---- issue #3: Download All flips the CALLER's auto-download to all ----
|
||||
|
||||
// podcastAuthedForm posts urlencoded (what htmx actually sends for non-GET —
|
||||
// the bug class in §0 item 2: the add modal posts a form, not JSON).
|
||||
func podcastAuthedForm(t *testing.T, setup *TestServerSetup, token, method, path string, form url.Values) (int, []byte) {
|
||||
t.Helper()
|
||||
req, _ := http.NewRequest(method, setup.Server.URL+path,
|
||||
strings.NewReader(form.Encode()))
|
||||
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
|
||||
req.Header.Set("Authorization", "Bearer "+token)
|
||||
resp, err := http.DefaultClient.Do(req)
|
||||
require.NoError(t, err)
|
||||
defer func() { _ = resp.Body.Close() }()
|
||||
var buf bytes.Buffer
|
||||
_, _ = buf.ReadFrom(resp.Body)
|
||||
return resp.StatusCode, buf.Bytes()
|
||||
}
|
||||
|
||||
func TestPodcastDownloadAllPolicyFlip(t *testing.T) {
|
||||
setup := setupTestServer(t)
|
||||
regularID := getRegularUserID(t, setup.DB)
|
||||
@@ -807,6 +824,84 @@ func TestPodcastDownloadAllPolicyFlip(t *testing.T) {
|
||||
})
|
||||
}
|
||||
|
||||
// ---- §0 item 2: download_all via the WIRE the add modal uses (urlencoded) ----
|
||||
|
||||
func TestPodcastDownloadAllFormEncoding(t *testing.T) {
|
||||
setup := setupTestServer(t)
|
||||
lib, _ := createPodcastsLibrary(t, setup, "test FormEnc Podcasts")
|
||||
|
||||
mux := http.NewServeMux()
|
||||
mux.HandleFunc("/feed.xml", func(w http.ResponseWriter, _ *http.Request) {
|
||||
w.Header().Set("Content-Type", "application/rss+xml")
|
||||
_, _ = w.Write([]byte(podcastFixture("FormEnc Feed", map[string]string{"fe-1": "FormEnc One"})))
|
||||
})
|
||||
mux.HandleFunc("/feed2.xml", func(w http.ResponseWriter, _ *http.Request) {
|
||||
w.Header().Set("Content-Type", "application/rss+xml")
|
||||
_, _ = w.Write([]byte(podcastFixture("FormEnc Default Feed", map[string]string{"fd-1": "FormEnc Default One"})))
|
||||
})
|
||||
mux.HandleFunc("/feed3.xml", func(w http.ResponseWriter, _ *http.Request) {
|
||||
w.Header().Set("Content-Type", "application/rss+xml")
|
||||
_, _ = w.Write([]byte(podcastFixture("FormEnc On Feed", map[string]string{"fo-1": "FormEnc On One"})))
|
||||
})
|
||||
srv := swapFeedServer(t, mux.ServeHTTP)
|
||||
|
||||
t.Run("urlencoded_add_with_download_all_true_flips_policy", func(t *testing.T) {
|
||||
form := url.Values{}
|
||||
form.Set("library_id", uuid.UUID(lib.ID.Bytes).String())
|
||||
form.Set("url", srv.URL+"/feed.xml")
|
||||
form.Set("download_all", "true") // what the add modal's value="true" checkbox submits
|
||||
resp, body := podcastAuthedForm(t, setup, setup.RegularToken, "POST", "/api/podcasts/feeds", form)
|
||||
require.Equal(t, http.StatusCreated, resp, string(body))
|
||||
|
||||
ignore := assertPodcastPolicy(t, setup, "FormEnc Feed", srv.URL+"/feed.xml")
|
||||
assert.Equal(t, "all", ignore,
|
||||
"PIN: urlencoded download_all=true binds exactly like JSON — htmx posts forms, not JSON")
|
||||
})
|
||||
|
||||
t.Run("urlencoded_add_with_browser_native_on_also_flips", func(t *testing.T) {
|
||||
form := url.Values{}
|
||||
form.Set("library_id", uuid.UUID(lib.ID.Bytes).String())
|
||||
form.Set("url", srv.URL+"/feed3.xml")
|
||||
form.Set("download_all", "on") // the raw <input type=checkbox> value — echo's ParseBool chokes on it and failed the WHOLE bind (400) before the FlexBool fix
|
||||
resp, body := podcastAuthedForm(t, setup, setup.RegularToken, "POST", "/api/podcasts/feeds", form)
|
||||
require.Equal(t, http.StatusCreated, resp, string(body))
|
||||
|
||||
assert.Equal(t, "all", assertPodcastPolicy(t, setup, "FormEnc On Feed", srv.URL+"/feed3.xml"))
|
||||
})
|
||||
|
||||
t.Run("urlencoded_add_without_download_all_keeps_default", func(t *testing.T) {
|
||||
form := url.Values{}
|
||||
form.Set("library_id", uuid.UUID(lib.ID.Bytes).String())
|
||||
form.Set("url", srv.URL+"/feed2.xml")
|
||||
resp, body := podcastAuthedForm(t, setup, setup.RegularToken, "POST", "/api/podcasts/feeds", form)
|
||||
require.Equal(t, http.StatusCreated, resp, string(body))
|
||||
|
||||
assert.Equal(t, "off", assertPodcastPolicy(t, setup, "FormEnc Default Feed", srv.URL+"/feed2.xml"),
|
||||
"PIN: an UNchecked add must NOT flip the policy (absent field ≠ true)")
|
||||
})
|
||||
}
|
||||
|
||||
// assertPodcastPolicy returns the regular user's auto_download for the
|
||||
// subscribed feed identified by (title, url).
|
||||
func assertPodcastPolicy(t *testing.T, setup *TestServerSetup, title, feedURL string) string {
|
||||
t.Helper()
|
||||
regularID := getRegularUserID(t, setup.DB)
|
||||
regularPG := pgtype.UUID{Bytes: regularID, Valid: true}
|
||||
feeds, _ := setup.DB.ListPodcastFeedsForPolling(rls.WithUser(context.Background(), regularPG))
|
||||
var feedID pgtype.UUID
|
||||
for _, f := range feeds {
|
||||
if f.Title == title && f.FeedUrl == feedURL {
|
||||
feedID = f.ID
|
||||
}
|
||||
}
|
||||
require.True(t, feedID.Valid, "subscribed feed %q (%s) must exist", title, feedURL)
|
||||
sub, err := setup.DB.GetPodcastSubscription(rls.WithUser(context.Background(), regularPG), database.GetPodcastSubscriptionParams{
|
||||
UserID: regularPG, FeedID: feedID,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
return sub.AutoDownload
|
||||
}
|
||||
|
||||
// ---- logout actually signs out (fake-signout bug, 2026-10-30) ----
|
||||
|
||||
func TestLogoutClearsSession(t *testing.T) {
|
||||
|
||||
@@ -10,6 +10,7 @@ import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"log"
|
||||
"sort"
|
||||
|
||||
"bookhoard/internal/database"
|
||||
@@ -74,6 +75,7 @@ func (h *PodcastHandler) flipSubscriptionToAll(ctx context.Context, userID, feed
|
||||
AutoDownload: "all",
|
||||
AutoDownloadCount: pgtype.Int4{}, // all needs no count
|
||||
})
|
||||
log.Printf("[PODCAST] flip: user %s → policy 'all' for feed %s (Download All / add checkbox)", userID, feedID)
|
||||
}
|
||||
|
||||
func NewPodcastHandler(db *database.Queries, svc *services.PodcastService, worker *services.PodcastDownloadWorker) *PodcastHandler {
|
||||
@@ -83,9 +85,30 @@ func NewPodcastHandler(db *database.Queries, svc *services.PodcastService, worke
|
||||
// ---- request/response shapes ----
|
||||
|
||||
type AddPodcastFeedRequest struct {
|
||||
LibraryID string `json:"library_id" form:"library_id" validate:"required"`
|
||||
URL string `json:"url" form:"url" validate:"required"`
|
||||
DownloadAll bool `json:"download_all" form:"download_all"`
|
||||
LibraryID string `json:"library_id" form:"library_id" validate:"required"`
|
||||
URL string `json:"url" form:"url" validate:"required"`
|
||||
DownloadAll FlexBool `json:"download_all" form:"download_all"`
|
||||
}
|
||||
|
||||
// FlexBool binds checkbox-style booleans on every wire. echo bool-binds
|
||||
// via strconv.ParseBool, which accepts "true"/"1" but REJECTS the
|
||||
// browser-native checkbox value "on" — and a bind failure is fatal for
|
||||
// the WHOLE request (the add modal would 400 "invalid request" on any
|
||||
// form variant that posts "on"). §0 item 2 (2026-10-30).
|
||||
//
|
||||
// A checkbox only contributes its field when CHECKED, so for form
|
||||
// posts presence really is the signal — JSON's native bool binding
|
||||
// ("false"→false) is unaffected. Absence on either wire = false.
|
||||
type FlexBool bool
|
||||
|
||||
func (b *FlexBool) UnmarshalParam(v string) error {
|
||||
switch strings.ToLower(strings.TrimSpace(v)) {
|
||||
case "false", "0", "off", "n", "no":
|
||||
*b = false
|
||||
default:
|
||||
*b = true // "true", "on", "yes", "checked", "1" — or any truthy variant
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
type UpdatePodcastSubscriptionRequest struct {
|
||||
@@ -130,11 +153,11 @@ func podcastErrorResponse(c *echo.Context, err error) error {
|
||||
// feed counts are tens, not thousands; no facet SQL needed).
|
||||
type podcastFeedSummary struct {
|
||||
Feed
|
||||
SubscribedAt string `json:"subscribed_at"`
|
||||
EpisodeTotal int `json:"episode_total"`
|
||||
DownloadedN int `json:"episode_downloaded"`
|
||||
UndownloadedN int `json:"episode_undownloaded"`
|
||||
NewEpisodes bool `json:"new_episodes"`
|
||||
SubscribedAt string `json:"subscribed_at"`
|
||||
EpisodeTotal int `json:"episode_total"`
|
||||
DownloadedN int `json:"episode_downloaded"`
|
||||
UndownloadedN int `json:"episode_undownloaded"`
|
||||
NewEpisodes bool `json:"new_episodes"`
|
||||
}
|
||||
|
||||
// AddFeed POST /api/podcasts/feeds
|
||||
@@ -245,10 +268,10 @@ func (h *PodcastHandler) GetFeed(c *echo.Context) error {
|
||||
episodeViews = append(episodeViews, episodeView(&eps[i]))
|
||||
}
|
||||
return c.JSON(http.StatusOK, map[string]interface{}{
|
||||
"feed": feedWithSubscription(&feed, sub, subFound == nil),
|
||||
"episodes": episodeViews,
|
||||
"gap_count": countGaps(episodeViews),
|
||||
"undownloaded": len(episodeViews) - countDownloaded(episodeViews),
|
||||
"feed": feedWithSubscription(&feed, sub, subFound == nil),
|
||||
"episodes": episodeViews,
|
||||
"gap_count": countGaps(episodeViews),
|
||||
"undownloaded": len(episodeViews) - countDownloaded(episodeViews),
|
||||
"has_library_access": true, // RLS already gated the row; kept for client convenience
|
||||
})
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user