From c9f3884f92ce556d6da89cd16dc73ceb17a4d7f2 Mon Sep 17 00:00:00 2001 From: iacore Date: Tue, 27 Aug 2024 16:21:11 +0000 Subject: [PATCH] replace ?r=/redirect_to with something else. can't shake the fakey feeling. net/http expects one handler max. the way we currently detect redirect param is not one handler. we should refactor the code out to be called from different handlers also, wtf is /settings/novelFontType?noredirect=t&r= (redirect or no redirect?) suggestion: Use Referer. the header exists for a reason. might need to turn off CSP for that. --- assets/views/components/artwork.jet.html | 6 +++--- assets/views/novel.jet.html | 5 ++--- doc/dev/framework-migration.md | 27 +++++------------------- main.go | 15 ++++--------- routes/actions.go | 9 +++++--- routes/render.go | 2 -- routes/render_test.go | 1 - routes/settings.go | 15 +++++++------ utils/compat.go | 11 ---------- utils/partialURL.go | 17 +++++---------- 10 files changed, 34 insertions(+), 74 deletions(-) diff --git a/assets/views/components/artwork.jet.html b/assets/views/components/artwork.jet.html index ee4a3df..61f5bce 100644 --- a/assets/views/components/artwork.jet.html +++ b/assets/views/components/artwork.jet.html @@ -36,11 +36,11 @@
{{ if isset(.BookmarkData) && LoggedIn }} - + Bookmarked {{ else }} - + Bookmark {{ end }} @@ -49,7 +49,7 @@ Liked {{ else }} - + Like {{ end }} diff --git a/assets/views/novel.jet.html b/assets/views/novel.jet.html index a3b6b25..ba6cfb0 100644 --- a/assets/views/novel.jet.html +++ b/assets/views/novel.jet.html @@ -64,7 +64,7 @@
- {{ url := "/settings/novelFontType?noredirect=t&r=" + OriginalURL + "#content" }} + {{ url := "/settings/novelFontType" }} Font size - {{ url := "/settings/novelViewMode?r=" + OriginalURL + "#content" }} + {{ url := "/settings/novelViewMode" }}
diff --git a/doc/dev/framework-migration.md b/doc/dev/framework-migration.md index 94e0dbc..b7008e8 100644 --- a/doc/dev/framework-migration.md +++ b/doc/dev/framework-migration.md @@ -1,24 +1,5 @@ # Migrating from gofiber to net/http -- the plan -- Config [already decoupled] -- Templating [decoupled, waiting for integration] -- Router - features - - /users/:id/:category? (optional path segment) - - /i.pximg.net/* (wildcard) -- Middleware - - Logging - - Rate limit (optional, could be loosely-coupled) - - Caching (optional, could be loosely-coupled) - -## Problem - -net/http handlers don't return errors. We have to make our own ServeMux that allows functions to return `error`, possibly. - -net/http expects handlers to panic on error, while we don't panic. We need to log the errors anyway. - -Idea: we create a compat layer of {w, r} that has the same API as *fiber.Ctx. - ## Tips - To access `/:abc`, use `r.PathValue("abc")`. @@ -28,6 +9,8 @@ Idea: we create a compat layer of {w, r} that has the same API as *fiber.Ctx. ## todo - correct redirect status codes. currently i put in whatever. -- test ?r=/redirect_to -- limiter -- caching + - 303 StatusSeeOther: set method to GET + - 307 Temporary: method and body not changed + - 308 Permanent: method and body not changed +- add limiter (maybe it should be in nginx) +- add caching (maybe it should be in nginx) diff --git a/main.go b/main.go index 499343c..20b09bf 100644 --- a/main.go +++ b/main.go @@ -72,17 +72,11 @@ func main() { setGlobalHeaders(w, r) if r.URL.Path != "/" && strings.HasSuffix(r.URL.Path, "/") { + // strip trailing / to make router behave url := r.URL url.Path, _ = strings.CutSuffix(url.Path, "/") - http.Redirect(w, r, url.String(), http.StatusFound) + http.Redirect(w, r, url.String(), http.StatusPermanentRedirect) } else { - // redirect any request with ?r=url - redirect_to := r.URL.Query().Get("r") - if redirect_to != "" { - // could this be unsafe since this redirects to any website? - http.Redirect(w, r, redirect_to, http.StatusTemporaryRedirect) - } - // all the routes are listed here router.ServeHTTP(w, r) } @@ -91,7 +85,7 @@ func main() { CatchError(func(w http.ResponseWriter, r utils.CompatRequest) error { err := GetUserContext(r.Request).err if err != nil { // error handler - log.Println(err) + log.Println("Within handler: ", err) code := http.StatusInternalServerError w.WriteHeader(code) // Send custom error page @@ -168,7 +162,7 @@ func setGlobalHeaders(w http.ResponseWriter, r *http.Request) { header.Add("X-Frame-Options", "DENY") // use this if need iframe: `X-Frame-Options: SAMEORIGIN` header.Add("X-Content-Type-Options", "nosniff") - header.Add("Referrer-Policy", "no-referrer") + header.Add("Referrer-Policy", "same-origin") // needed for settings redirect header.Add("Strict-Transport-Security", "max-age=31536000; includeSubDomains; preload") header.Add("Content-Security-Policy", fmt.Sprintf("base-uri 'self'; default-src 'none'; script-src 'self'; style-src 'self'; img-src 'self' %s; media-src 'self' %s; connect-src 'self'; form-action 'self'; frame-ancestors 'none';", session.GetImageProxyOrigin(r), session.GetImageProxyOrigin(r))) // use this if need iframe: `frame-ancestors 'self'` @@ -213,7 +207,6 @@ func defineRoutes() *mux.Router { // Settings group router.HandleFunc("/settings", CatchError(routes.SettingsPage)).Methods("GET") router.HandleFunc("/settings/{type}", CatchError(routes.SettingsPost)).Methods("POST") - router.HandleFunc("/settings/{type}/{noredirect}?", CatchError(routes.SettingsPost)).Methods("POST") // Personal group router.HandleFunc("/self", CatchError(routes.LoginUserPage)).Methods("GET") diff --git a/routes/actions.go b/routes/actions.go index de63914..6fde8a8 100644 --- a/routes/actions.go +++ b/routes/actions.go @@ -82,7 +82,8 @@ func AddBookmarkRoute(w http.ResponseWriter, r CompatRequest) error { return err } - return SendString(w, "Success") + RedirectToWhenceYouCame(w, r) + return nil } func DeleteBookmarkRoute(w http.ResponseWriter, r CompatRequest) error { @@ -105,7 +106,8 @@ func DeleteBookmarkRoute(w http.ResponseWriter, r CompatRequest) error { return err } - return SendString(w, "Success") + RedirectToWhenceYouCame(w, r) + return nil } func LikeRoute(w http.ResponseWriter, r CompatRequest) error { @@ -127,5 +129,6 @@ func LikeRoute(w http.ResponseWriter, r CompatRequest) error { return err } - return SendString(w, "Success") + RedirectToWhenceYouCame(w, r) + return nil } diff --git a/routes/render.go b/routes/render.go index 29c90c7..92b76f0 100644 --- a/routes/render.go +++ b/routes/render.go @@ -58,7 +58,6 @@ func GetTemplatingVariables(r CompatRequest) jet.VarMap { // Pass in values that we want to be available to all pages here token := session.GetPixivToken(r.Request) baseURL := r.BaseURL() - originalURL := r.OriginalURL() pageURL := r.PageURL() cookies := map[string]string{} @@ -69,7 +68,6 @@ func GetTemplatingVariables(r CompatRequest) jet.VarMap { return jet.VarMap{}. Set("BaseURL", baseURL). - Set("OriginalURL", originalURL). Set("PageURL", pageURL). Set("LoggedIn", token != ""). Set("Queries", r.URL.Query().Encode()). diff --git a/routes/render_test.go b/routes/render_test.go index eafa757..88b5fa6 100644 --- a/routes/render_test.go +++ b/routes/render_test.go @@ -71,7 +71,6 @@ func manualTest[T any](t *testing.T, data T) { for k, v := range map[string]any{ "BaseURL": fakeData[string](), - "OriginalURL": fakeData[string](), "PageURL": fakeData[string](), "LoggedIn": fakeData[bool](), "Queries": fakeData[map[string]string](), diff --git a/routes/settings.go b/routes/settings.go index 5e78cb1..e249128 100644 --- a/routes/settings.go +++ b/routes/settings.go @@ -160,7 +160,6 @@ func SettingsPage(w http.ResponseWriter, r CompatRequest) error { func SettingsPost(w http.ResponseWriter, r CompatRequest) error { t := r.Params("type") - noredirect := r.FormValue("noredirect") == "" var err error switch t { @@ -192,10 +191,14 @@ func SettingsPost(w http.ResponseWriter, r CompatRequest) error { return err } - if !noredirect { - return nil - } - - http.Redirect(w, r.Request, "/settings", http.StatusSeeOther) + RedirectToWhenceYouCame(w, r) return nil } + +func RedirectToWhenceYouCame(w http.ResponseWriter, r CompatRequest) { + if strings.HasPrefix(r.Referer(), r.BaseURL()) { + http.Redirect(w, r.Request, r.Referer(), http.StatusSeeOther) + } else { + w.WriteHeader(200) + } +} diff --git a/utils/compat.go b/utils/compat.go index 35e176a..e36edb0 100644 --- a/utils/compat.go +++ b/utils/compat.go @@ -20,17 +20,6 @@ func (r CompatRequest) BaseURL() string { Host: r.URL.Host, }).String() } -func (r CompatRequest) OriginalURL() string { - return (&url.URL{ - Path: r.URL.Path, - RawPath: r.URL.RawPath, - OmitHost: r.URL.OmitHost, - ForceQuery: r.URL.ForceQuery, - RawQuery: r.URL.RawQuery, - Fragment: r.URL.Fragment, - RawFragment: r.URL.RawFragment, - }).String() -} func (r CompatRequest) PageURL() string { return r.URL.String() } diff --git a/utils/partialURL.go b/utils/partialURL.go index aaadd4c..b152b16 100644 --- a/utils/partialURL.go +++ b/utils/partialURL.go @@ -11,14 +11,11 @@ type PartialURL struct { func unfinishedQuery(url PartialURL, key string) string { result := fmt.Sprintf("/%s", url.Path) first_query_pair := true - query_param_exists := false for k, v := range url.Query { k = lowercaseFirstChar(k) if k == key { - // Reserve this - query_param_exists = true continue } @@ -37,17 +34,13 @@ func unfinishedQuery(url PartialURL, key string) string { } // This is to move the matched query to the end of the URL - if query_param_exists { - var t string - if first_query_pair { - t = "?" - } else { - t = "&" - } - result += fmt.Sprintf("%s%s=", t, key) + var t string + if first_query_pair { + t = "?" } else { - // todo: what now? if it doesn't exist, it's a no-op? that doesn't make sense given how navigation works. + t = "&" } + result += fmt.Sprintf("%s%s=", t, key) return result }