Closed djyotta closed 1 month ago
@djyotta , I made a few changes, let me know if the results look good to you.
@lovasoa I'm getting "too many redirects" from my browser and URL like: http://127.0.0.1:8812/%2F/%2F/%2F/%2F/%2F/%2F/%2F/%2F/%2F/%2F/%2F/%2F/%2F/%2F/%2F/%2F/%2F/%2F/%2F/%2F/%2F/%2F/%2F/%2F/%2F/%2F/%2F/%2F/%2F/%2F/%2F/%2F/%2F/%2F/%2F/%2F/%2F/%2F/grocery/products.sql
Happens with following:
site_prefix | url |
---|---|
/ |
172.0.0.1:8812/, 172.0.0.1:8812/path/to/page.sql |
/app |
172.0.0.1:8812/, 172.0.0.1:8812/app/path/to/page.sql |
<empty-string> |
172.0.0.1:8812/, 172.0.0.1:8812/path/to/page.sql |
<not-specified> |
172.0.0.1:8812/, 172.0.0.1:8812/path/to/page.sql |
Following case seem fine: | site_prefix | url |
---|---|---|
app |
172.0.0.1:8812/, 172.0.0.1:8812/app/path/to/page.sql, 172.0.0.1:8812/path/to/page.sql |
Otherwise, as far as I can tell, the changes look good. The refactoring and tidy up looks sensible.
EDIT: I would suggest the redirect from url outside of site prefix is probably not necessary. My thinking is that if the developer is setting prefix, then likely knows how to configure the reverse proxy. Not sure what the use case for the redirct is. Not sure how much complexity it is adding.
The redirect is just a function. The default behavior of actix-web when no handler matches is to return an empty reponse, which confuses browsers and makes debugging difficult. It's mainly useful to help with debugging and local testing of the site_prefix configuration option.
I'll fix the bugs you noticed above.
@djyotta : I made the fixes. If everything looks ok to you, I'll merge.
@lovasoa looks good to merge!
Closes #279
I haven't any experience with rust, so I made as few changes as possible and did what the compiler told me to.
So this could look a bit weird or backwards, but it does work.
Probably needs some testing and way more scrutiny.
I've tested with:
Default:
None (same as default):