From a0ae1f7e6ab4a86bbc2aafee32253d972f04460f Mon Sep 17 00:00:00 2001 From: bakape Date: Tue, 9 May 2017 14:31:58 +0300 Subject: [PATCH] Refactor search Optimise, error handling and avoid some exploits --- router/apiHandler.go | 10 +- router/homeHandler.go | 10 +- router/rssHandler.go | 18 ++-- router/searchHandler.go | 21 ++-- router/templateVariables.go | 86 +++++++---------- service/torrent/torrent.go | 26 ++--- templates/_search.html | 22 ++--- util/search/search.go | 185 +++++++++++++++++++++++------------- util/server.go | 13 +++ 9 files changed, 225 insertions(+), 166 deletions(-) create mode 100644 util/server.go diff --git a/router/apiHandler.go b/router/apiHandler.go index 7e95ad1a..962c7861 100644 --- a/router/apiHandler.go +++ b/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 diff --git a/router/homeHandler.go b/router/homeHandler.go index a5c8f7bc..7ffdf097 100644 --- a/router/homeHandler.go +++ b/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) } - } diff --git a/router/rssHandler.go b/router/rssHandler.go index c86656a9..102ffc89 100644 --- a/router/rssHandler.go +++ b/router/rssHandler.go @@ -1,17 +1,23 @@ package router -import( - "time" +import ( "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 { diff --git a/router/searchHandler.go b/router/searchHandler.go index 9d4fdbe9..dc0713f3 100644 --- a/router/searchHandler.go +++ b/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, - search_param.Status, - search_param.Category, - search_param.Sort, - search_param.Order, - false, + SearchParam: search_param, + Category: search_param.Category.String(), + HideAdvancedSearch: 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) } diff --git a/router/templateVariables.go b/router/templateVariables.go index 9edccc65..85c4a6b1 100644 --- a/router/templateVariables.go +++ b/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 - Search SearchForm - Navigation Navigation - User *model.User - URL *url.URL // For parsing Url in templates - Route *mux.Route // For getting current route in templates + FormErrors map[string][]string + Search SearchForm + Navigation Navigation + User *model.User + URL *url.URL // For parsing Url in templates + Route *mux.Route // For getting current route in templates } type UserLoginFormVariables struct { - LoginForm userForms.LoginForm - FormErrors map[string][]string - Search SearchForm - Navigation Navigation - User *model.User - URL *url.URL // For parsing Url in templates - Route *mux.Route // For getting current route in templates + LoginForm userForms.LoginForm + FormErrors map[string][]string + Search SearchForm + Navigation Navigation + User *model.User + URL *url.URL // For parsing Url in templates + Route *mux.Route // For getting current route in templates } type UserProfileVariables struct { - UserProfile *model.User - FormErrors map[string][]string - Search SearchForm - Navigation Navigation - User *model.User - URL *url.URL // For parsing Url in templates - Route *mux.Route // For getting current route in templates + UserProfile *model.User + FormErrors map[string][]string + Search SearchForm + Navigation Navigation + User *model.User + URL *url.URL // For parsing Url 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 - Status string + search.SearchParam Category string - Sort string - Order string HideAdvancedSearch bool } // Some Default Values to ease things out -func NewSearchForm(params ...string) (searchForm SearchForm) { - if len(params) > 1 { - searchForm.Category = params[0] - } else { - searchForm.Category = "_" +func NewSearchForm() SearchForm { + return 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 } diff --git a/service/torrent/torrent.go b/service/torrent/torrent.go index fb32f9b3..72b83e7a 100644 --- a/service/torrent/torrent.go +++ b/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) { - var torrents []model.Torrents - var count int +func GetTorrentsOrderBy(parameters *WhereParams, orderBy string, limit int, offset int) (torrents []model.Torrents, count int, err error) { 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) - return torrents, count + err = db.ORM.Raw(dbQuery, params...).Find(&torrents).Error + 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(¶meters, "", 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(¶meters, "", 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) } diff --git a/templates/_search.html b/templates/_search.html index bdc9f222..f1f11ad9 100644 --- a/templates/_search.html +++ b/templates/_search.html @@ -26,23 +26,23 @@ {{end}} {{define "search_advanced"}}