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:
1 parent
cdf81f6cf4
commit
7c7c08748e
2 files changed
+130
-12
No files matched your search
@@ -11,6 +11,7 @@ package main
|
|||||||
|
|
||||||
import (
|
import (
|
||||||
"bytes"
|
"bytes"
|
||||||
|
"net/url"
|
||||||
"context"
|
"context"
|
||||||
"encoding/json"
|
"encoding/json"
|
||||||
"fmt"
|
"fmt"
|
||||||
@@ -733,6 +734,22 @@ func TestPodcastPageSearchEpisodes(t *testing.T) {
|
|||||||
|
|
||||||
// ---- issue #3: Download All flips the CALLER's auto-download to all ----
|
// ---- 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) {
|
func TestPodcastDownloadAllPolicyFlip(t *testing.T) {
|
||||||
setup := setupTestServer(t)
|
setup := setupTestServer(t)
|
||||||
regularID := getRegularUserID(t, setup.DB)
|
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) ----
|
// ---- logout actually signs out (fake-signout bug, 2026-10-30) ----
|
||||||
|
|
||||||
func TestLogoutClearsSession(t *testing.T) {
|
func TestLogoutClearsSession(t *testing.T) {
|
||||||
|
|||||||
@@ -10,6 +10,7 @@ import (
|
|||||||
"context"
|
"context"
|
||||||
"encoding/json"
|
"encoding/json"
|
||||||
"fmt"
|
"fmt"
|
||||||
|
"log"
|
||||||
"sort"
|
"sort"
|
||||||
|
|
||||||
"bookhoard/internal/database"
|
"bookhoard/internal/database"
|
||||||
@@ -74,6 +75,7 @@ func (h *PodcastHandler) flipSubscriptionToAll(ctx context.Context, userID, feed
|
|||||||
AutoDownload: "all",
|
AutoDownload: "all",
|
||||||
AutoDownloadCount: pgtype.Int4{}, // all needs no count
|
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 {
|
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 ----
|
// ---- request/response shapes ----
|
||||||
|
|
||||||
type AddPodcastFeedRequest struct {
|
type AddPodcastFeedRequest struct {
|
||||||
LibraryID string `json:"library_id" form:"library_id" validate:"required"`
|
LibraryID string `json:"library_id" form:"library_id" validate:"required"`
|
||||||
URL string `json:"url" form:"url" validate:"required"`
|
URL string `json:"url" form:"url" validate:"required"`
|
||||||
DownloadAll bool `json:"download_all" form:"download_all"`
|
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 {
|
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).
|
// feed counts are tens, not thousands; no facet SQL needed).
|
||||||
type podcastFeedSummary struct {
|
type podcastFeedSummary struct {
|
||||||
Feed
|
Feed
|
||||||
SubscribedAt string `json:"subscribed_at"`
|
SubscribedAt string `json:"subscribed_at"`
|
||||||
EpisodeTotal int `json:"episode_total"`
|
EpisodeTotal int `json:"episode_total"`
|
||||||
DownloadedN int `json:"episode_downloaded"`
|
DownloadedN int `json:"episode_downloaded"`
|
||||||
UndownloadedN int `json:"episode_undownloaded"`
|
UndownloadedN int `json:"episode_undownloaded"`
|
||||||
NewEpisodes bool `json:"new_episodes"`
|
NewEpisodes bool `json:"new_episodes"`
|
||||||
}
|
}
|
||||||
|
|
||||||
// AddFeed POST /api/podcasts/feeds
|
// AddFeed POST /api/podcasts/feeds
|
||||||
@@ -245,10 +268,10 @@ func (h *PodcastHandler) GetFeed(c *echo.Context) error {
|
|||||||
episodeViews = append(episodeViews, episodeView(&eps[i]))
|
episodeViews = append(episodeViews, episodeView(&eps[i]))
|
||||||
}
|
}
|
||||||
return c.JSON(http.StatusOK, map[string]interface{}{
|
return c.JSON(http.StatusOK, map[string]interface{}{
|
||||||
"feed": feedWithSubscription(&feed, sub, subFound == nil),
|
"feed": feedWithSubscription(&feed, sub, subFound == nil),
|
||||||
"episodes": episodeViews,
|
"episodes": episodeViews,
|
||||||
"gap_count": countGaps(episodeViews),
|
"gap_count": countGaps(episodeViews),
|
||||||
"undownloaded": len(episodeViews) - countDownloaded(episodeViews),
|
"undownloaded": len(episodeViews) - countDownloaded(episodeViews),
|
||||||
"has_library_access": true, // RLS already gated the row; kept for client convenience
|
"has_library_access": true, // RLS already gated the row; kept for client convenience
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in new issue
Block a user