Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proper 404 handling #4

Open
raphael opened this issue Jul 2, 2016 · 2 comments
Open

Proper 404 handling #4

raphael opened this issue Jul 2, 2016 · 2 comments

Comments

@raphael
Copy link

raphael commented Jul 2, 2016

I could be missing something but it doesn't seem like weasel handles 404s properly. The 404 configured in GCS doesn't seem to take effect and looking at the code I don't see where it would handle it.

404s could be handled by adding a section to the config, something like:
config.json

  "not-found": "404.html"

Which could be used by the server:
server.go

func serveObject(w http.ResponseWriter, r *http.Request) {
    if !weasel.ValidMethod(r.Method) {
        http.Error(w, "", http.StatusMethodNotAllowed)
        return
    }
    if _, force := config.tlsOnly[r.Host]; force && r.TLS == nil {
        u := "https://" + r.Host + r.URL.Path
        if r.URL.RawQuery != "" {
            u += "?" + r.URL.RawQuery
        }
        http.Redirect(w, r, u, http.StatusMovedPermanently)
        return
    }

    ctx := newContext(r)
    bucket := bucketForHost(r.Host)
    oname := r.URL.Path[1:]

    o, err := storage.OpenFile(ctx, bucket, oname)
    if err != nil {
        code := http.StatusInternalServerError
        if errf, ok := err.(*weasel.FetchError); ok {
            code = errf.Code
        }
        if code == http.StatusNotFound {  // *** START OF ADDED CODE ***
            o, err = storage.OpenFile(ctx, bucket, config.NotFound)
            if err == nil {
                goto serve
            }
        }   // *** END OF ADDED CODE ***
        serveError(w, code, "")
        if code != http.StatusNotFound {
            log.Errorf(ctx, "%s/%s: %v", bucket, oname, err)
        }
        return
    }
serve:  // *** ADDED ***
    if err := storage.ServeObject(w, r, o); err != nil {
        log.Errorf(ctx, "%s/%s: %v", bucket, oname, err)
    }
    o.Body.Close()
}

Would you consider a PR that does the above?

@x1ddos
Copy link
Contributor

x1ddos commented Oct 10, 2016

Sounds like a good idea! Please, do create a PR.

@raphael
Copy link
Author

raphael commented Oct 11, 2016

OK I'll work on something. You mentioned you wanted to get rid of the need for a set config structure, how would you suggest storing/retrieving the value of the path to the 404 file? Maybe there's a way to retrieve it from GCS? - that would be ideal but I don't know the API well enough to know whether that's possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants