aetaric / checkrr

Checkrr Scans your library files for corrupt media and replace the files via sonarr and radarr
MIT License
202 stars 10 forks source link

[BUG] Issue with subdirectory in baseurl #68

Closed shanehughes1990 closed 1 year ago

shanehughes1990 commented 1 year ago

Describe the bug webserver from yaml file, this is json with yamlencoding (terraform)

"webserver" : {
  "port" : 8585,
  "baseurl" : "/checkrr"
},

My Ingress for this service checkrr.example.com/checkrr is essentially what it resolves too.

domain_match_pattern = "Host(`checkrr.${var.cloudflare_config.zone_name}`) && PathPrefix(`/checkrr`)"

The static site itself doesn't even load just 404, but the api responses don't have the directory delimiter "checkrr/api" between the baseurl and api routes, if you change baseurl to /checkrr/ the api works static site does not.

The issue atleast for the api is on line 84 webserver.go router.Group(w.BaseURL + "api") = /checkrrapi NOT /checkrr/api

To Reproduce Steps to reproduce the behavior:

  1. Change baseurl to /checkrr
  2. go to http://localhost:8585/checkrrapi/files/bad
  3. You will see the api response

Expected behavior I want subdirectories for my services so I would want the UI & api both to work when changing the baseurl from the default /

Screenshots image

Desktop (please complete the following information):

Additional context Havent worked with static.Serve from gincontrib at all, but I build golang api's regularly for work (done gin countless times). I would be willing to do a PR for this issue (I'm sure Ill figure out the static site given enough time)

aetaric commented 1 year ago

Changing the baseurl to /checkrr/ should work fine though, no?

shanehughes1990 commented 1 year ago

As stated in my comment it fixes api not the static stuff.

shanehughes1990 commented 1 year ago

This will fix your api routes regardless of whether or not there is a trailing slash in the baseurl

// BaseURL is the base URL for the webserver
type BaseURL string

// enforceTrailingSlash will ensure there is a trailing slash on the base URL
func (b BaseURL) enforceTrailingSlash() BaseURL {
    if !strings.HasSuffix(string(b), "/") {
        return BaseURL(fmt.Sprintf("%s/", b))
    }
    return b
}

// enforceTruncedTrailingSlash will ensure there is not a trailing slash on the base URL
func (b BaseURL) enforceTruncedTrailingSlash() BaseURL {
    if strings.HasSuffix(string(b), "/") {
        return BaseURL(strings.TrimSuffix(string(b), "/"))
    }
    return b
}

// String returns BaseURL as a string
func (b BaseURL) String() string {
    return string(b)
}

Then the router.Group is api := router.Group(w.BaseURL.enforceTrailingSlash().String() + "api")

Note the enforeceTruncedTrailingSlash was me playing around with the static serve side seeing if vite needed it or not.

This doesn't fix the static site though not a frontend developer by any means but it looks like it has something to do with the defineConfig.base in the vite.config.js, I was playing around with it but no dice, vitejs shared options

shanehughes1990 commented 1 year ago

I have a PR comming your way, QQ though would you be willing to integrate go-task for local contirbutors to get kickstarted easier? (Basically an amped up Make). I can write out all the steps to create the config yaml, the db file yarn i, yarn build etc etc. Also air for hotreload?

aetaric commented 1 year ago

go-task and air would be fine to add tbh! I mostly just spin up and down post go build and/or yarn build as needed, but if it makes the repo more approachable, then it's probably for the best.

shanehughes1990 commented 1 year ago

Okay some update on the static side of things,

replacing the static.Serve for router.StaticFS(w.BaseURL.EnforceTrailingSlash().String(), embeddedBuildFolder) + base: "/checkrr/", in the vite.config.js will successfully host that site from the subdirectory,

Problem being as that StaticFS conflicts with EVERY other route under baseURL including API, this was me just trying to figure it out.

I'm out of ideas today on how to fix this further. ๐Ÿ˜† but that poses another problem that if baseurl is anything BUT / in the config you need to also update the vite config & yarn build again, There may be runtime env or otherwise you can apply to the docker container not sure not too familiar with vite or frontend dev for that matter ๐Ÿคฃ

aetaric commented 1 year ago

Hey @hugo-vrijswijk, Is there some sane way to fix this in vite?

shanehughes1990 commented 1 year ago

I have this software deployed either way I would just really like for it to follow suit with the rest of my mediacenter ๐Ÿ˜‚๐Ÿ˜‚, it's already correct many video files so kudos for the work already done ๐Ÿ‘Œ

hugo-vrijswijk commented 1 year ago

Hey @hugo-vrijswijk, Is there some sane way to fix this in vite?

I'm not sure. The path gets hardcoded in compile-time. Maybe if the base-path is set to ./ it might work? As there is only one page in this application it shouldn't mess with too much

shanehughes1990 commented 1 year ago

I can try it again tonight with your suggestion Ill let you know the outcome.

aetaric commented 1 year ago

I made some progress on this. It's partly golang, and partly vite. I've solved the golang with

--- a/webserver/webserver.go
+++ b/webserver/webserver.go
@@ -24,6 +24,8 @@ var staticFS embed.FS

 var fileInfo [][]string

+var baseurl string
+
 var db *bolt.DB
 var scheduler *cron.Cron
 var cronEntry cron.EntryID
@@ -40,6 +42,7 @@ type Webserver struct {
 func (w *Webserver) FromConfig(conf *viper.Viper, c chan []string, checkrr *check.Checkrr) {
        w.Port = conf.GetInt("Port")
        w.BaseURL = conf.GetString("baseurl")
+        baseurl = w.BaseURL
        if conf.GetStringSlice("trustedproxies") != nil {
                w.trustedProxies = conf.GetStringSlice("trustedproxies")
        } else {
@@ -221,7 +224,12 @@ func newStaticFileSystem() *staticFileSystem {
 }

 func (s *staticFileSystem) Exists(prefix string, path string) bool {
-       buildpath := fmt.Sprintf("build%s", path)
+        buildpath := ""
+        if baseurl == "/" {
+               buildpath = fmt.Sprintf("build%s", path)
+        } else {
+               buildpath = fmt.Sprintf("build%s", strings.TrimPrefix(path, baseurl))
+        }

        // support for folders
        if strings.HasSuffix(path, "/") {

The problem I am still hitting is the assets being linked to by vite as /assets/... instead of assets/.... I'm not 100% sure how to get vite to comply there. EDIT: setting base to ./ solved that.... Just gotta sort out the api now.

This also broke API things, but that part seems trivial to resolve. EDIT 2: No it didn't. This is the very problem Shane has code to resolve, just needs to be applied to Exists for staticFileSystem as well instead of the TrimPrefix hack.

aetaric commented 1 year ago

@shanehughes1990 I'm gonna open a branch for 3.2.0, feel free to PR anything you wished to add into that.

shanehughes1990 commented 1 year ago

Sorry I haven't had time to test the above been smoked with work lately. I will have some time this weekend I can add the air and taskfile stuff to it ๐Ÿ‘ as for this issue I'm not sure it's easily solvable.

aetaric commented 1 year ago

Well, the good news is the vite code is sorted 100%. @hugo-vrijswijk was right, setting base to ./ was the play. The only thing that's really needed is to make sure the trailing slash is included if the user leaves it off. It can be done with your enforce code when w.BaseURL is being set in FromConfig() for the DRY-est code.

All other code I intend to add for 3.2.0 is already in the branch too.

shanehughes1990 commented 1 year ago

perfect I can push that code for the enforced trailing slash then that's perfect!

shanehughes1990 commented 1 year ago

@aetaric ^^^ The above PR for the trailing slash, should cover the rest of this issue, we should be able to close this out now with the merge of that PR, thanks again for your help in debugging this I'm excited for 3.2.0 Ill be the first to deploy it.

shanehughes1990 commented 1 year ago

Also I'm testing it I know I should have done this before the PR ๐Ÿ˜†, is there something else I'm supposed to be doing to check /checkrr subdirectory working??

The baseURL Right out of the gate is enforcing the trailing slash

func (w *Webserver) FromConfig(conf *viper.Viper, c chan []string, checkrr *check.Checkrr) {
    w.Port = conf.GetInt("Port")
    w.BaseURL = BaseURL(conf.GetString("baseurl")).EnforceTrailingSlash()

Ive added a log on your Exists method just to see and it is infact stripping the path from the url it outputs

[GIN] 2023/10/05 - 00:19:56 | 200 |      190.82ยตs |       127.0.0.1 | GET      "/checkrr/"
2023/10/05 00:19:56 build/assets/index-c7ae7eb6.js
[GIN] 2023/10/05 - 00:19:56 | 404 |      96.014ยตs |       127.0.0.1 | GET      "/assets/index-c7ae7eb6.js"

Still the white screen on the frontend console tab is just 404 for the index js file

aetaric commented 1 year ago

3.2.0 is out now! it's all sorted (and I did it in the PR if you want to see the final fix).

shanehughes1990 commented 1 year ago

nice I did take a peak, and have it rolled out now image Ill be removing the subdomain now and just using the subdirectory ๐Ÿš€

aetaric commented 1 year ago

There was a tiny post release patch, but that doesn't impact users with a custom base url set... Only people who used the default. So you shouldn't have to worry about it, really. It's validated to work with both with the latest build attached to the release.

shanehughes1990 commented 1 year ago

๐Ÿ˜ฒ lmfao doesn't affect me anyway I'm not on root ๐Ÿ˜†, JK goot catch I already love this software it has saved me so much pain so far..