Refactor search

Optimise, error handling and avoid some exploits

router/apiHandler.go

@@ -12,6 +12,7 @@ import (
 	"github.com/ewhal/nyaa/db"
 	"github.com/ewhal/nyaa/model"
 	"github.com/ewhal/nyaa/service/torrent"
+	"github.com/ewhal/nyaa/util"
 	"github.com/gorilla/mux"
 )
 
@@ -30,7 +31,11 @@ func ApiHandler(w http.ResponseWriter, r *http.Request) {
 		pagenum = 1
 	}
 
-	torrents, nbTorrents := torrentService.GetAllTorrents(maxPerPage, maxPerPage*(pagenum-1))
+	torrents, nbTorrents, err := torrentService.GetAllTorrents(maxPerPage, maxPerPage*(pagenum-1))
+	if err != nil {
+		util.SendError(w, err, 400)
+		return
+	}
 
 	b := model.ApiResultJson{
 		Torrents: model.TorrentsToJSON(torrents),
@@ -38,8 +43,7 @@ func ApiHandler(w http.ResponseWriter, r *http.Request) {
 	b.QueryRecordCount = maxPerPage
 	b.TotalRecordCount = nbTorrents
 	w.Header().Set("Content-Type", "application/json")
-	err := json.NewEncoder(w).Encode(b)
+	err = json.NewEncoder(w).Encode(b)
-
 	if err != nil {
 		http.Error(w, err.Error(), http.StatusInternalServerError)
 		return

router/homeHandler.go

@@ -7,6 +7,7 @@ import (
 
 	"github.com/ewhal/nyaa/model"
 	"github.com/ewhal/nyaa/service/torrent"
+	"github.com/ewhal/nyaa/util"
 	"github.com/ewhal/nyaa/util/languages"
 	"github.com/ewhal/nyaa/util/log"
 	"github.com/gorilla/mux"
@@ -28,7 +29,11 @@ func HomeHandler(w http.ResponseWriter, r *http.Request) {
 		pagenum = 1
 	}
 
-	torrents, nbTorrents := torrentService.GetAllTorrents(maxPerPage, maxPerPage*(pagenum-1))
+	torrents, nbTorrents, err := torrentService.GetAllTorrents(maxPerPage, maxPerPage*(pagenum-1))
+	if err != nil {
+		util.SendError(w, err, 400)
+		return
+	}
 
 	b := model.TorrentsToJSON(torrents)
 
@@ -37,9 +42,8 @@ func HomeHandler(w http.ResponseWriter, r *http.Request) {
 	languages.SetTranslationFromRequest(homeTemplate, r, "en-us")
 	htv := HomeTemplateVariables{b, NewSearchForm(), navigationTorrents, GetUser(r), r.URL, mux.CurrentRoute(r)}
 
-	err := homeTemplate.ExecuteTemplate(w, "index.html", htv)
+	err = homeTemplate.ExecuteTemplate(w, "index.html", htv)
 	if err != nil {
 		log.Errorf("HomeHandler(): %s", err)
 	}
-
 }

router/rssHandler.go

@@ -1,17 +1,23 @@
 package router
 
-import(
+import (
-	"time"
 	"net/http"
-	"github.com/gorilla/feeds"
-	"github.com/ewhal/nyaa/config"
-	"github.com/ewhal/nyaa/util/search"
 	"strconv"
+	"time"
+
+	"github.com/ewhal/nyaa/config"
+	"github.com/ewhal/nyaa/util"
+	"github.com/ewhal/nyaa/util/search"
+	"github.com/gorilla/feeds"
 )
 
 func RssHandler(w http.ResponseWriter, r *http.Request) {
 
-	_, torrents, _ := search.SearchByQuery( r, 1 )
+	_, torrents, _, err := search.SearchByQuery(r, 1)
+	if err != nil {
+		util.SendError(w, err, 400)
+		return
+	}
 	created_as_time := time.Now()
 
 	if len(torrents) > 0 {

router/searchHandler.go

@@ -6,6 +6,7 @@ import (
 	"strconv"
 
 	"github.com/ewhal/nyaa/model"
+	"github.com/ewhal/nyaa/util"
 	"github.com/ewhal/nyaa/util/languages"
 	"github.com/ewhal/nyaa/util/search"
 	"github.com/gorilla/mux"
@@ -21,23 +22,25 @@ func SearchHandler(w http.ResponseWriter, r *http.Request) {
 		pagenum = 1
 	}
 
-	search_param, torrents, nbTorrents := search.SearchByQuery(r, pagenum)
+	search_param, torrents, nbTorrents, err := search.SearchByQuery(r, pagenum)
+	if err != nil {
+		util.SendError(w, err, 400)
+		return
+	}
 
 	b := model.TorrentsToJSON(torrents)
 
-	navigationTorrents := Navigation{nbTorrents, search_param.Max, pagenum, "search_page"}
+	navigationTorrents := Navigation{nbTorrents, int(search_param.Max), pagenum, "search_page"}
+	// Convert back to strings for now.
 	searchForm := SearchForm{
-		search_param.Query,
+		SearchParam:        search_param,
-		search_param.Status,
+		Category:           search_param.Category.String(),
-		search_param.Category,
+		HideAdvancedSearch: false,
-		search_param.Sort,
-		search_param.Order,
-		false,
 	}
 	htv := HomeTemplateVariables{b, searchForm, navigationTorrents, GetUser(r), r.URL, mux.CurrentRoute(r)}
 
 	languages.SetTranslationFromRequest(searchTemplate, r, "en-us")
-	err := searchTemplate.ExecuteTemplate(w, "index.html", htv)
+	err = searchTemplate.ExecuteTemplate(w, "index.html", htv)
 	if err != nil {
 		http.Error(w, err.Error(), http.StatusInternalServerError)
 	}

router/templateVariables.go

@@ -1,13 +1,14 @@
 package router
 
 import (
-	"net/url"
 	"net/http"
+	"net/url"
 
 	"github.com/ewhal/nyaa/model"
-	userForms "github.com/ewhal/nyaa/service/user/form"
-	"github.com/ewhal/nyaa/service/user"
 	"github.com/ewhal/nyaa/service/captcha"
+	"github.com/ewhal/nyaa/service/user"
+	userForms "github.com/ewhal/nyaa/service/user/form"
+	"github.com/ewhal/nyaa/util/search"
 	"github.com/gorilla/mux"
 )
 
@@ -45,48 +46,48 @@ type ViewTemplateVariables struct {
 
 type UserRegisterTemplateVariables struct {
 	RegistrationForm userForms.RegistrationForm
-	FormErrors 		 map[string][]string
+	FormErrors       map[string][]string
 	Search           SearchForm
 	Navigation       Navigation
-	User      		 *model.User
+	User             *model.User
 	URL              *url.URL   // For parsing Url in templates
 	Route            *mux.Route // For getting current route in templates
 }
 
 type UserVerifyTemplateVariables struct {
-	FormErrors 		 map[string][]string
+	FormErrors map[string][]string
-	Search           SearchForm
+	Search     SearchForm
-	Navigation       Navigation
+	Navigation Navigation
-	User       		 *model.User
+	User       *model.User
-	URL              *url.URL   // For parsing Url in templates
+	URL        *url.URL   // For parsing Url in templates
-	Route            *mux.Route // For getting current route in templates
+	Route      *mux.Route // For getting current route in templates
 }
 
 type UserLoginFormVariables struct {
-	LoginForm userForms.LoginForm
+	LoginForm  userForms.LoginForm
-	FormErrors 		 map[string][]string
+	FormErrors map[string][]string
-	Search           SearchForm
+	Search     SearchForm
-	Navigation       Navigation
+	Navigation Navigation
-	User      		 *model.User
+	User       *model.User
-	URL              *url.URL   // For parsing Url in templates
+	URL        *url.URL   // For parsing Url in templates
-	Route            *mux.Route // For getting current route in templates
+	Route      *mux.Route // For getting current route in templates
 }
 
 type UserProfileVariables struct {
-	UserProfile  	 *model.User
+	UserProfile *model.User
-	FormErrors 		 map[string][]string
+	FormErrors  map[string][]string
-	Search           SearchForm
+	Search      SearchForm
-	Navigation       Navigation
+	Navigation  Navigation
-	User      		 *model.User
+	User        *model.User
-	URL              *url.URL   // For parsing Url in templates
+	URL         *url.URL   // For parsing Url in templates
-	Route            *mux.Route // For getting current route in templates
+	Route       *mux.Route // For getting current route in templates
 }
 
 type HomeTemplateVariables struct {
 	ListTorrents []model.TorrentsJson
 	Search       SearchForm
 	Navigation   Navigation
-	User      	 *model.User
+	User         *model.User
 	URL          *url.URL   // For parsing Url in templates
 	Route        *mux.Route // For getting current route in templates
 }
@@ -111,42 +112,19 @@ type Navigation struct {
 }
 
 type SearchForm struct {
-	Query              string
+	search.SearchParam
-	Status             string
 	Category           string
-	Sort               string
-	Order              string
 	HideAdvancedSearch bool
 }
 
 // Some Default Values to ease things out
-func NewSearchForm(params ...string) (searchForm SearchForm) {
+func NewSearchForm() SearchForm {
-	if len(params) > 1 {
+	return SearchForm{
-		searchForm.Category = params[0]
+		Category: "_",
-	} else {
-		searchForm.Category = "_"
 	}
-	if len(params) > 2 {
-		searchForm.Sort = params[1]
-	} else {
-		searchForm.Sort = "torrent_id"
-	}
-	if len(params) > 3 {
-		order := params[2]
-		if order == "DESC" {
-			searchForm.Order = order
-		} else if order == "ASC" {
-			searchForm.Order = order
-		} else {
-			// TODO: handle invalid value (?)
-		}
-	} else {
-		searchForm.Order = "DESC"
-	}
-	return
 }
 
 func GetUser(r *http.Request) *model.User {
-	user, _ , _ := userService.RetrieveCurrentUser(r)
+	user, _, _ := userService.RetrieveCurrentUser(r)
 	return &user
 }

service/torrent/torrent.go

@@ -67,9 +67,7 @@ func GetTorrentById(id string) (model.Torrents, error) {
 	return torrent, nil
 }
 
-func GetTorrentsOrderBy(parameters *WhereParams, orderBy string, limit int, offset int) ([]model.Torrents, int) {
+func GetTorrentsOrderBy(parameters *WhereParams, orderBy string, limit int, offset int) (torrents []model.Torrents, count int, err error) {
-	var torrents []model.Torrents
-	var count int
 	var conditionArray []string
 	if strings.HasPrefix(orderBy, "filesize") {
 		// torrents w/ NULL filesize fuck up the sorting on postgres
@@ -83,7 +81,12 @@ func GetTorrentsOrderBy(parameters *WhereParams, orderBy string, limit int, offs
 		params = parameters.Params
 	}
 	conditions := strings.Join(conditionArray, " AND ")
-	db.ORM.Model(&torrents).Where(conditions, params...).Count(&count)
+	err = db.ORM.Model(&torrents).Where(conditions, params...).Count(&count).Error
+	if err != nil {
+		return
+	}
+
+	// TODO: Vulnerable to injections. Use query builder.
 
 	// build custom db query for performance reasons
 	dbQuery := "SELECT * FROM torrents"
@@ -101,37 +104,36 @@ func GetTorrentsOrderBy(parameters *WhereParams, orderBy string, limit int, offs
 	if limit != 0 || offset != 0 { // if limits provided
 		dbQuery = dbQuery + " LIMIT " + strconv.Itoa(limit) + " OFFSET " + strconv.Itoa(offset)
 	}
-	db.ORM.Raw(dbQuery, params...).Find(&torrents)
+	err = db.ORM.Raw(dbQuery, params...).Find(&torrents).Error
-	return torrents, count
+	return
 }
 
 /* Functions to simplify the get parameters of the main function
  *
  * Get Torrents with where parameters and limits, order by default
  */
-func GetTorrents(parameters WhereParams, limit int, offset int) ([]model.Torrents, int) {
+func GetTorrents(parameters WhereParams, limit int, offset int) ([]model.Torrents, int, error) {
 	return GetTorrentsOrderBy(&parameters, "", limit, offset)
 }
 
 /* Get Torrents with where parameters but no limit and order by default (get all the torrents corresponding in the db)
  */
-func GetTorrentsDB(parameters WhereParams) ([]model.Torrents, int) {
+func GetTorrentsDB(parameters WhereParams) ([]model.Torrents, int, error) {
 	return GetTorrentsOrderBy(&parameters, "", 0, 0)
 }
 
 /* Function to get all torrents
  */
 
-func GetAllTorrentsOrderBy(orderBy string, limit int, offset int) ([]model.Torrents, int) {
+func GetAllTorrentsOrderBy(orderBy string, limit int, offset int) ([]model.Torrents, int, error) {
-
 	return GetTorrentsOrderBy(nil, orderBy, limit, offset)
 }
 
-func GetAllTorrents(limit int, offset int) ([]model.Torrents, int) {
+func GetAllTorrents(limit int, offset int) ([]model.Torrents, int, error) {
 	return GetTorrentsOrderBy(nil, "", limit, offset)
 }
 
-func GetAllTorrentsDB() ([]model.Torrents, int) {
+func GetAllTorrentsDB() ([]model.Torrents, int, error) {
 	return GetTorrentsOrderBy(nil, "", 0, 0)
 }
 

templates/_search.html

@@ -26,23 +26,23 @@
     <option value="1_2" {{if eq .Search.Category "1_2"}}selected{{end}}>{{T "software_games"}}</option>
   </select>
   <select name="s" class="form-control input-sm">
-    <option value="">{{T "show_all"}}</option>
+    <option value="0">{{T "show_all"}}</option>
-    <option value="2" {{if eq .Search.Status "2"}}selected{{end}}>{{T "filter_remakes"}}</option>
+    <option value="1" {{if eq .Search.Status 1}}selected{{end}}>{{T "filter_remakes"}}</option>
-    <option value="3" {{if eq .Search.Status "3"}}selected{{end}}>{{T "trusted"}}</option>
+    <option value="2" {{if eq .Search.Status 2}}selected{{end}}>{{T "trusted"}}</option>
-    <option value="4" {{if eq .Search.Status "4"}}selected{{end}}>A+</option>
+    <option value="3" {{if eq .Search.Status 3}}selected{{end}}>A+</option>
   </select>
 {{end}}
 {{define "search_advanced"}}
   <select name="sort" class="form-control input-sm">
-    <option value="torrent_id" {{if eq .Search.Sort "torrent_id"}}selected{{end}}>{{T "id"}}</option>
+    <option value="0" {{if eq .Search.Sort 0}}selected{{end}}>{{T "id"}}</option>
-    <option value="torrent_name" {{if eq .Search.Sort "torrent_name"}}selected{{end}}>{{T "name"}}</option>
+    <option value="1" {{if eq .Search.Sort 1}}selected{{end}}>{{T "name"}}</option>
-    <option value="date" {{if eq .Search.Sort "date"}}selected{{end}}>{{T "date"}}</option>
+    <option value="2" {{if eq .Search.Sort 2}}selected{{end}}>{{T "date"}}</option>
-    <option value="downloads" {{if eq .Search.Sort "downloads"}}selected{{end}}>{{T "downloads"}}</option>
+    <option value="3" {{if eq .Search.Sort 3}}selected{{end}}>{{T "downloads"}}</option>
-    <option value="filesize" {{if eq .Search.Sort "filesize"}}selected{{end}}>{{T "size"}}</option>
+    <option value="4" {{if eq .Search.Sort 4}}selected{{end}}>{{T "size"}}</option>
   </select>
   <select name="order" class="form-control input-sm">
-    <option value="desc" {{if eq .Search.Order "desc"}}selected{{end}}>{{T "descending"}}</option>
+    <option value="false" {{if eq .Search.Order false}}selected{{end}}>{{T "descending"}}</option>
-    <option value="asc" {{if eq .Search.Order "asc"}}selected{{end}}>{{T "ascending"}}</option>
+    <option value="true" {{if eq .Search.Order true}}selected{{end}}>{{T "ascending"}}</option>
   </select>
   <select name="max" class="form-control input-sm">
     <option value="5" {{if eq .Navigation.MaxItemPerPage 5}}selected{{end}}>5</option>

util/search/search.go

@@ -1,7 +1,6 @@
 package search
 
 import (
-	"html"
 	"net/http"
 	"strconv"
 	"strings"
@@ -13,95 +12,145 @@ import (
 	"github.com/ewhal/nyaa/util/log"
 )
 
-type SearchParam struct {
+type Status uint8
-	Category string
+
-	Order    string
+const (
-	Query    string
+	ShowAll Status = iota
-	Max      int
+	FilterRemakes
-	Status   string
+	Trusted
-	Sort     string
+	APlus
+)
+
+type SortMode uint8
+
+const (
+	ID SortMode = iota
+	Name
+	Date
+	Downloads
+	Size
+)
+
+type Category struct {
+	Main, Sub uint8
 }
 
-func SearchByQuery(r *http.Request, pagenum int) (SearchParam, []model.Torrents, int) {
+func (c Category) String() (s string) {
-	maxPerPage, errConv := strconv.Atoi(r.URL.Query().Get("max"))
+	if c.Main != 0 {
-	if errConv != nil {
+		s += strconv.Itoa(int(c.Main))
-		maxPerPage = 50 // default Value maxPerPage
+	}
+	s += "_"
+	if c.Sub != 0 {
+		s += strconv.Itoa(int(c.Sub))
+	}
+	return
+}
+
+type SearchParam struct {
+	Order    bool // True means acsending
+	Status   Status
+	Sort     SortMode
+	Category Category
+	Max      uint
+	Query    string
+}
+
+func SearchByQuery(r *http.Request, pagenum int) (search SearchParam, tor []model.Torrents, count int, err error) {
+	max, err := strconv.ParseUint(r.URL.Query().Get("max"), 10, 32)
+	if err != nil {
+		err = nil
+		max = 50 // default Value maxPerPage
+	} else if max > 300 {
+		max = 300
+	}
+	search.Max = uint(max)
+
+	search.Query = r.URL.Query().Get("q")
+
+	switch s := r.URL.Query().Get("s"); s {
+	case "1":
+		search.Status = FilterRemakes
+	case "2":
+		search.Status = Trusted
+	case "3":
+		search.Status = APlus
 	}
 
-	if maxPerPage > 300 {
+	catString := r.URL.Query().Get("c")
-		maxPerPage = 300
+	if s := catString; len(s) > 1 && s != "_" {
+		var tmp uint64
+		tmp, err = strconv.ParseUint(string(s[0]), 10, 8)
+		if err != nil {
+			return
+		}
+		search.Category.Main = uint8(tmp)
+
+		if len(s) == 3 {
+			tmp, err = strconv.ParseUint(string(s[2]), 10, 8)
+			if err != nil {
+				return
+			}
+			search.Category.Sub = uint8(tmp)
+		}
 	}
-	search_param := SearchParam{}
+
-	search_param.Max = maxPerPage
+	order_by := ""
-	search_param.Query = r.URL.Query().Get("q")
+
-	search_param.Category = r.URL.Query().Get("c")
+	switch s := r.URL.Query().Get("sort"); s {
-	search_param.Status = r.URL.Query().Get("s")
+	case "1":
-	search_param.Sort = r.URL.Query().Get("sort")
+		search.Sort = Name
-	search_param.Order = r.URL.Query().Get("order")
+		order_by += "torrent_name"
+	case "2":
+		search.Sort = Date
+		order_by += "date"
+	case "3":
+		search.Sort = Downloads
+		order_by += "downloads"
+	case "4":
+		search.Sort = Size
+		order_by += "filesize"
+	default:
+		order_by += "torrent_id"
+	}
+
+	order_by += " "
+
+	switch s := r.URL.Query().Get("order"); s {
+	case "true":
+		search.Order = true
+		order_by += "asc"
+	default:
+		order_by += "desc"
+	}
+
 	userId := r.URL.Query().Get("userId")
 
-	catsSplit := strings.Split(search_param.Category, "_")
-	// need this to prevent out of index panics
-	var searchCatId, searchSubCatId string
-	if len(catsSplit) == 2 {
-
-		searchCatId = html.EscapeString(catsSplit[0])
-		searchSubCatId = html.EscapeString(catsSplit[1])
-	}
-
-	switch search_param.Sort {
-	case "torrent_name":
-		search_param.Sort = "torrent_name"
-		break
-	case "date":
-		search_param.Sort = "date"
-		break
-	case "downloads":
-		search_param.Sort = "downloads"
-		break
-	case "filesize":
-		search_param.Sort = "filesize"
-	case "torrent_id":
-	default:
-		search_param.Sort = "torrent_id"
-	}
-
-	switch search_param.Order {
-	case "asc":
-		search_param.Order = "asc"
-		break
-	case "desc":
-	default:
-		search_param.Order = "desc"
-	}
-
-	order_by := search_param.Sort + " " + search_param.Order
-
 	parameters := torrentService.WhereParams{
 		Params: make([]interface{}, 0, 64),
 	}
 	conditions := make([]string, 0, 64)
-	if searchCatId != "" {
+	if search.Category.Main != 0 {
 		conditions = append(conditions, "category = ?")
-		parameters.Params = append(parameters.Params, searchCatId)
+		parameters.Params = append(parameters.Params, string(catString[0]))
 	}
-	if searchSubCatId != "" {
+	if search.Category.Sub != 0 {
 		conditions = append(conditions, "sub_category = ?")
-		parameters.Params = append(parameters.Params, searchSubCatId)
+		parameters.Params = append(parameters.Params, string(catString[2]))
 	}
 	if userId != "" {
 		conditions = append(conditions, "uploader = ?")
 		parameters.Params = append(parameters.Params, userId)
 	}
-	if search_param.Status != "" {
+	if search.Status != 0 {
-		if search_param.Status == "2" {
+		if search.Status == 3 {
 			conditions = append(conditions, "status != ?")
 		} else {
 			conditions = append(conditions, "status = ?")
 		}
-		parameters.Params = append(parameters.Params, search_param.Status)
+		parameters.Params = append(parameters.Params, strconv.Itoa(int(search.Status)+1))
 	}
-	searchQuerySplit := strings.Fields(search_param.Query)
+
+	searchQuerySplit := strings.Fields(search.Query)
 	for i, word := range searchQuerySplit {
 		firstRune, _ := utf8.DecodeRuneInString(word)
 		if len(word) == 1 && unicode.IsPunct(firstRune) {
@@ -119,6 +168,6 @@ func SearchByQuery(r *http.Request, pagenum int) (SearchParam, []model.Torrents,
 
 	parameters.Conditions = strings.Join(conditions[:], " AND ")
 	log.Infof("SQL query is :: %s\n", parameters.Conditions)
-	torrents, n := torrentService.GetTorrentsOrderBy(&parameters, order_by, maxPerPage, maxPerPage*(pagenum-1))
+	tor, count, err = torrentService.GetTorrentsOrderBy(&parameters, order_by, int(search.Max), int(search.Max)*(pagenum-1))
-	return search_param, torrents, n
+	return
 }

util/server.go

@@ -0,0 +1,13 @@
+package util
+
+import (
+	"net/http"
+	"runtime/debug"
+
+	"github.com/ewhal/nyaa/util/log"
+)
+
+func SendError(w http.ResponseWriter, err error, code int) {
+	log.Warnf("%s:\n%s\n", err, debug.Stack())
+	http.Error(w, err.Error(), code)
+}