Procházet zdrojové kódy

fix: avoid panic when creating a feed without any category

Deleting the last category leaves the user with no category at all.
Creating a feed with category_id omitted or set to zero then
dereferenced the nil result of FirstCategory and panicked. The same
pattern existed in the OPML import and Google Reader subscribe paths.

FirstCategory now returns ErrNoCategory instead of (nil, nil) when
the user has no category, and all three call sites translate this
error into a bad request response instead of panicking.
Frédéric Guillot před 1 měsícem
rodič
revize
a7f93d8b8e

+ 50 - 0
internal/api/api_integration_test.go

@@ -1508,6 +1508,56 @@ func TestCannotCreateDuplicatedFeed(t *testing.T) {
 	}
 	}
 }
 }
 
 
+func TestFeedCreationWithoutCategoryReturnsBadRequest(t *testing.T) {
+	t.Parallel()
+
+	testConfig := newIntegrationTestConfig()
+	if !testConfig.isConfigured() {
+		t.Skip(skipIntegrationTestsMessage)
+	}
+
+	adminClient := miniflux.NewClient(testConfig.testBaseURL, testConfig.testAdminUsername, testConfig.testAdminPassword)
+
+	regularTestUser, err := adminClient.CreateUser(testConfig.genRandomUsername(), testConfig.testRegularPassword, false)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer adminClient.DeleteUser(regularTestUser.ID)
+
+	regularUserClient := miniflux.NewClient(testConfig.testBaseURL, regularTestUser.Username, testConfig.testRegularPassword)
+	categories, err := regularUserClient.Categories()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	for _, category := range categories {
+		if err := regularUserClient.DeleteCategory(category.ID); err != nil {
+			t.Fatal(err)
+		}
+	}
+
+	t.Run("REST API", func(t *testing.T) {
+		_, err := regularUserClient.CreateFeed(&miniflux.FeedCreationRequest{FeedURL: testConfig.testFeedURL})
+		if !errors.Is(err, miniflux.ErrBadRequest) {
+			t.Fatalf(`Expected a bad request error, got %v`, err)
+		}
+	})
+
+	t.Run("OPML import", func(t *testing.T) {
+		data := `<?xml version="1.0" encoding="UTF-8"?>
+<opml version="2.0">
+    <body>
+        <outline title="Test" text="Test" xmlUrl="` + testConfig.testFeedURL + `"></outline>
+    </body>
+</opml>`
+
+		err := regularUserClient.Import(io.NopCloser(bytes.NewReader([]byte(data))))
+		if !errors.Is(err, miniflux.ErrBadRequest) {
+			t.Fatalf(`Expected a bad request error, got %v`, err)
+		}
+	})
+}
+
 func TestCreateFeedWithInexistingCategory(t *testing.T) {
 func TestCreateFeedWithInexistingCategory(t *testing.T) {
 	t.Parallel()
 	t.Parallel()
 
 

+ 10 - 0
internal/api/feed_handlers.go

@@ -13,8 +13,10 @@ import (
 	"miniflux.app/v2/internal/config"
 	"miniflux.app/v2/internal/config"
 	"miniflux.app/v2/internal/http/request"
 	"miniflux.app/v2/internal/http/request"
 	"miniflux.app/v2/internal/http/response"
 	"miniflux.app/v2/internal/http/response"
+	"miniflux.app/v2/internal/locale"
 	"miniflux.app/v2/internal/model"
 	"miniflux.app/v2/internal/model"
 	feedHandler "miniflux.app/v2/internal/reader/handler"
 	feedHandler "miniflux.app/v2/internal/reader/handler"
+	"miniflux.app/v2/internal/storage"
 	"miniflux.app/v2/internal/validator"
 	"miniflux.app/v2/internal/validator"
 )
 )
 
 
@@ -31,9 +33,17 @@ func (h *handler) createFeedHandler(w http.ResponseWriter, r *http.Request) {
 	if feedCreationRequest.CategoryID == 0 {
 	if feedCreationRequest.CategoryID == 0 {
 		category, err := h.store.FirstCategory(userID)
 		category, err := h.store.FirstCategory(userID)
 		if err != nil {
 		if err != nil {
+			if errors.Is(err, storage.ErrNoCategory) {
+				response.JSONBadRequest(w, r, locale.NewLocalizedError("error.feed_category_not_found").Error())
+				return
+			}
 			response.JSONServerError(w, r, err)
 			response.JSONServerError(w, r, err)
 			return
 			return
 		}
 		}
+		if category == nil {
+			response.JSONBadRequest(w, r, locale.NewLocalizedError("error.feed_category_not_found").Error())
+			return
+		}
 		feedCreationRequest.CategoryID = category.ID
 		feedCreationRequest.CategoryID = category.ID
 	}
 	}
 
 

+ 7 - 0
internal/api/opml_handlers.go

@@ -4,11 +4,14 @@
 package api // import "miniflux.app/v2/internal/api"
 package api // import "miniflux.app/v2/internal/api"
 
 
 import (
 import (
+	"errors"
 	"net/http"
 	"net/http"
 
 
 	"miniflux.app/v2/internal/http/request"
 	"miniflux.app/v2/internal/http/request"
 	"miniflux.app/v2/internal/http/response"
 	"miniflux.app/v2/internal/http/response"
+	"miniflux.app/v2/internal/locale"
 	"miniflux.app/v2/internal/reader/opml"
 	"miniflux.app/v2/internal/reader/opml"
+	"miniflux.app/v2/internal/storage"
 )
 )
 
 
 func (h *handler) exportFeedsHandler(w http.ResponseWriter, r *http.Request) {
 func (h *handler) exportFeedsHandler(w http.ResponseWriter, r *http.Request) {
@@ -27,6 +30,10 @@ func (h *handler) importFeedsHandler(w http.ResponseWriter, r *http.Request) {
 	err := opmlHandler.Import(request.UserID(r), r.Body)
 	err := opmlHandler.Import(request.UserID(r), r.Body)
 	defer r.Body.Close()
 	defer r.Body.Close()
 	if err != nil {
 	if err != nil {
+		if errors.Is(err, storage.ErrNoCategory) {
+			response.JSONBadRequest(w, r, locale.NewLocalizedError("error.feed_category_not_found").Error())
+			return
+		}
 		response.JSONServerError(w, r, err)
 		response.JSONServerError(w, r, err)
 		return
 		return
 	}
 	}

+ 16 - 2
internal/googlereader/handler.go

@@ -371,7 +371,11 @@ func (h *greaderHandler) quickAddHandler(w http.ResponseWriter, r *http.Request)
 	category := Stream{NoStream, ""}
 	category := Stream{NoStream, ""}
 	newFeed, err := subscribe(toSubscribe, category, "", h.store, userID)
 	newFeed, err := subscribe(toSubscribe, category, "", h.store, userID)
 	if err != nil {
 	if err != nil {
-		response.JSONServerError(w, r, err)
+		if errors.Is(err, errCategoryNotFound) {
+			response.JSONBadRequest(w, r, err)
+		} else {
+			response.JSONServerError(w, r, err)
+		}
 		return
 		return
 	}
 	}
 
 
@@ -415,8 +419,14 @@ func getOrCreateCategory(streamCategory Stream, store *storage.Storage, userID i
 func subscribe(newFeed Stream, category Stream, title string, store *storage.Storage, userID int64) (*model.Feed, error) {
 func subscribe(newFeed Stream, category Stream, title string, store *storage.Storage, userID int64) (*model.Feed, error) {
 	destCategory, err := getOrCreateCategory(category, store, userID)
 	destCategory, err := getOrCreateCategory(category, store, userID)
 	if err != nil {
 	if err != nil {
+		if errors.Is(err, storage.ErrNoCategory) {
+			return nil, errCategoryNotFound
+		}
 		return nil, err
 		return nil, err
 	}
 	}
+	if destCategory == nil {
+		return nil, errCategoryNotFound
+	}
 
 
 	feedRequest := model.FeedCreationRequest{
 	feedRequest := model.FeedCreationRequest{
 		FeedURL:    newFeed.ID,
 		FeedURL:    newFeed.ID,
@@ -557,7 +567,11 @@ func (h *greaderHandler) editSubscriptionHandler(w http.ResponseWriter, r *http.
 	case "subscribe":
 	case "subscribe":
 		_, err := subscribe(streamIds[0], newLabel, title, h.store, userID)
 		_, err := subscribe(streamIds[0], newLabel, title, h.store, userID)
 		if err != nil {
 		if err != nil {
-			response.JSONServerError(w, r, err)
+			if errors.Is(err, errCategoryNotFound) {
+				response.JSONBadRequest(w, r, err)
+			} else {
+				response.JSONServerError(w, r, err)
+			}
 			return
 			return
 		}
 		}
 	case "unsubscribe":
 	case "unsubscribe":

+ 3 - 0
internal/reader/opml/handler.go

@@ -100,6 +100,9 @@ func (h *Handler) resolveCategory(userID int64, categoryName string) (*model.Cat
 		if err != nil {
 		if err != nil {
 			return nil, fmt.Errorf("opml: unable to find first category: %w", err)
 			return nil, fmt.Errorf("opml: unable to find first category: %w", err)
 		}
 		}
+		if category == nil {
+			return nil, storage.ErrNoCategory
+		}
 		return category, nil
 		return category, nil
 	}
 	}
 
 

+ 5 - 1
internal/storage/category.go

@@ -12,6 +12,9 @@ import (
 	"miniflux.app/v2/internal/model"
 	"miniflux.app/v2/internal/model"
 )
 )
 
 
+// ErrNoCategory is returned when a user does not have any category.
+var ErrNoCategory = errors.New("store: no category found")
+
 // AnotherCategoryExists checks if another category exists with the same title.
 // AnotherCategoryExists checks if another category exists with the same title.
 func (s *Storage) AnotherCategoryExists(userID, categoryID int64, title string) bool {
 func (s *Storage) AnotherCategoryExists(userID, categoryID int64, title string) bool {
 	var result bool
 	var result bool
@@ -59,6 +62,7 @@ func (s *Storage) Category(userID, categoryID int64) (*model.Category, error) {
 }
 }
 
 
 // FirstCategory returns the first category for the given user.
 // FirstCategory returns the first category for the given user.
+// It returns ErrNoCategory if the user does not have any category.
 func (s *Storage) FirstCategory(userID int64) (*model.Category, error) {
 func (s *Storage) FirstCategory(userID int64) (*model.Category, error) {
 	query := `SELECT id, user_id, title, hide_globally FROM categories WHERE user_id=$1 ORDER BY title ASC LIMIT 1`
 	query := `SELECT id, user_id, title, hide_globally FROM categories WHERE user_id=$1 ORDER BY title ASC LIMIT 1`
 
 
@@ -67,7 +71,7 @@ func (s *Storage) FirstCategory(userID int64) (*model.Category, error) {
 
 
 	switch {
 	switch {
 	case errors.Is(err, sql.ErrNoRows):
 	case errors.Is(err, sql.ErrNoRows):
-		return nil, nil
+		return nil, ErrNoCategory
 	case err != nil:
 	case err != nil:
 		return nil, fmt.Errorf(`store: unable to fetch category: %v`, err)
 		return nil, fmt.Errorf(`store: unable to fetch category: %v`, err)
 	default:
 	default: