knob to set clean url with a 308 redirect

[toravir]

fixes issue described in #420

I am a little unsure if there should be a check to see if the method is non-GET and then only do this or is it good to do this for everything (current pr) - please comment.

[toravir]

Just noticed that #463 conflicts with this PR.. will rework this PR if the other one is merged first.

[elithrar]

It's not clear that this is a useful addition for most users. Those who really desire to use 308's can wrap the ResponseWriter with middleware.

[elithrar]

RE: this -

• How do you know the URL is cleaned? (note: uppercase "URL")
• Why is this useful to a user to enable, if they haven't read this PR or the history?

[elithrar]

As a user, what would a 308 get me over a 301?

(Note: I know, but someone trying to learn mux will not)

[toravir]

It's not clear that this is a useful addition for most users. Those who really desire to use 308's can wrap the ResponseWriter with middleware.

Middleware is simple for a blind (301->308) - but it becomes complicated if it needs to be done when URL cleaning was done.. wrote up one: https://gist.github.com/toravir/8c9bab5756f33963fa745d977d85f94e - but still it does not handle/relay other status codes - then there is the 'multiple response.WriteHeader calls' error if middleware tries to overwrite the status. Agree, most users will not need this - but few who want to do the 308, this knob provides a simple way to do. Will address comments about the docs later in the day..

[elithrar]

You need to handle the 'wrote header' case - the standard library implementations of http.ResponseWriter often have a wroteHeader bool field: https://golang.org/src/net/http/server.go

Working example that rewrites 301/302 and wraps the entire router:

package main

import (
	"fmt"
	"log"
	"net/http"

	"github.com/gorilla/mux"
)

type statusResponseWriter struct {
	http.ResponseWriter
	wroteHeader bool
}

func (srw *statusResponseWriter) WriteHeader(code int) {
	if !srw.wroteHeader {
		switch code {
		case 301:
			code = 308
		case 302:
			code = 307
		}

		srw.wroteHeader = true
		srw.ResponseWriter.WriteHeader(code)
	}
}

func statusRewriter(next http.Handler) http.Handler {
	fn := func(w http.ResponseWriter, r *http.Request) {
		wrappedWriter := &statusResponseWriter{ResponseWriter: w}
		next.ServeHTTP(wrappedWriter, r)
	}

	return http.HandlerFunc(fn)
}

func handler(w http.ResponseWriter, r *http.Request) {
	http.Redirect(w, r, "/target", 302)
}

func target(w http.ResponseWriter, r *http.Request) {
	fmt.Fprintln(w, "hello")
}

func main() {
	r := mux.NewRouter()

	r.HandleFunc("/", handler)

	log.Fatal(
		http.ListenAndServe("localhost:8000",
			statusRewriter(r),
		))
}

[toravir]

Ah.. translate the status at the first writing itself.. yes that does the work. Earlier, I was only thinking of re-writing the status after the actual handler completes.

This knob isn't useful - unless in a corner case where one wants a behavior of using 308 ONLY for the case of URL cleaning is done. In my current project, i don't have any handlers returning 301 - so i can use the blanket middleware to do the job - since 301 is generated only when url is cleaned.

Thx. Will close the PR in a day.

[elithrar]

Yep! The middleware doesn’t “care”. I’ll add the middleware example to the docs to make it easier for the next person.

…

On Fri, Apr 5, 2019 at 7:34 AM Ravi Raju ***@***.***> wrote: Ah.. translate the status at the first writing itself.. yes that does the work. Earlier, I was only thinking of re-writing the status after the actual handler completes. This knob isn't useful - unless in a corner case where one wants a behavior of using 308 ONLY for the case of URL cleaning is done. In my current project, i don't have any handlers returning 301 - so i can use the blanket middleware to do the job - since 301 is generated only when url is cleaned. Thx. Will close the PR in a day. — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub <#465 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABIcJ-KJFwo3PmqIUHnaVLwLGae_KNuks5vd17qgaJpZM4cZnuX> .