google / weasel

A simple frontend (App Engine app) that serves content from a Google Cloud Storage (GCS) bucket
Apache License 2.0
68 stars 19 forks source link

Absolute redirects #3

Closed raphael closed 8 years ago

raphael commented 8 years ago

There are a number of files in my GCS bucket that have moved or just don't exist anymore however there are outstanding external links to them.

It is fairly simple to add a section to config.json that lists "absolute" redirects where the redirect does not append the original request path.

I'm thinking something like: config.json:

  "absolute-redirects": {
    "/getting-started.html": "https://goa.design/learn/guide",
    "/goagen.html": "https://goa.design/implement/goagen",
    "/vice-framework": "https://goa.design",
    "/swagger.html": "https://goa.design/design/swagger"
  },

The server code would then be tweaked to not append the original request path for these, something like: server.go:

func init() {
// ...
    for path, redir := range config.AbsoluteRedirects {
        http.Handle(path, redirectHandler(redir, http.StatusMovedPermanently, true))
    }
    for host, redir := range config.Redirects {
        http.Handle(host, redirectHandler(redir, http.StatusMovedPermanently, false))
    }
//...
}

// ...

func redirectHandler(url string, code int, abs bool) http.Handler {
    return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
        u := url
        if !abs {
            u += r.URL.Path
        }
// ....

Would you consider a PR that does the above?

x1ddos commented 8 years ago

This is already implemented. See example config here: https://github.com/google/weasel/blob/a785d75dfea6c0f0233711e878a216038400de35/server/config.json.sample#L3

raphael commented 8 years ago

As far as I understand what redirects does is append the redirected path to the redirect target (what I would call a "relative" redirect). What I needed was an "absolute" redirect. Unless I'm missing something I don't think this issue should be closed.

x1ddos commented 8 years ago

if host is example.com and another.host is another.example.com, then

"host/": "https://another.host"

will redirect:

Isn't that what you need?

raphael commented 8 years ago

No I need to be able to redirect to a different path, not just a different host. If you look at my initial comment you'll see for example:

"/getting-started.html": "https://goa.design/learn/guide"

which redirects:

"goa.design/getting-started.html" to "https://goa.design/learn/guide"
x1ddos commented 8 years ago

Ah, I see. Sorry for misreading. That can be done with the std http package, no need for weasel:

http.Handle("/getting-started.html", http.RedirectHandler("https://goa.design/learn/guide", 301))
raphael commented 8 years ago

Right that's how I ended up implementing the "absolute redirect" feature, just thought it would be useful for these redirects to also appear in the config to keep all these things in one place.

x1ddos commented 8 years ago

Right. I'm trying to move weasel away from imposing a certain config format. Working on a change to leave the config part up to the user and provide some examples instead.

raphael commented 8 years ago

Sounds good, looking forward to it!

x1ddos commented 8 years ago

Cool. I'll mention you in a PR for a review. Thanks!