net/http: optionally add headers on redirect

[zachgersh]

Fixes #19973

[gopherbot]

This PR (HEAD: f369c10) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/#/c/go/+/95895 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

[gopherbot]

Message from Brad Fitzpatrick:

Patch Set 1:

(1 comment)

---

Please don’t reply on this GitHub thread. Visit golang.org/cl/95895.
After addressing review feedback, remember to publish your drafts!

[gopherbot]

Message from Zachary Gershman:

Patch Set 1:

Patch Set 1:

(1 comment)

Is that clear enough? I actively struggled with the name so I welcome the feedback here. Originally I had the field as ForwardHeader(s) but that didn't feel right either.

---

Please don’t reply on this GitHub thread. Visit golang.org/cl/95895.
After addressing review feedback, remember to publish your drafts!

[gopherbot]

Message from Brad Fitzpatrick:

Patch Set 1:

Patch Set 1:

Patch Set 1:

(1 comment)

Is that clear enough? I actively struggled with the name so I welcome the feedback here. Originally I had the field as ForwardHeader(s) but that didn't feel right either.

Well, FooPolicy returning a bool is weird. A policy often has more than two values, and they're often not called true & false.

So naming it something that has a yes/no answer (Allow, Reject, Forward, Redirect) makes more sense, because they it reads well at call sites too.

But yeah, I think RedirectHeader is fine (especially since it'll still have docs). Or ShouldRedirectHeader, but there's no precedence for the name "Should" in existing Go API anywhere.

---

Please don’t reply on this GitHub thread. Visit golang.org/cl/95895.
After addressing review feedback, remember to publish your drafts!

[gopherbot]

Message from Kunpei Sakai:

Patch Set 1:

(1 comment)

---

Please don’t reply on this GitHub thread. Visit golang.org/cl/95895.
After addressing review feedback, remember to publish your drafts!

[gopherbot]

Message from Gerrit User 5065:

Patch Set 1:

Ping Zach.

---

Please don’t reply on this GitHub thread. Visit golang.org/cl/95895.
After addressing review feedback, remember to publish your drafts!

[gopherbot]

Message from Gerrit User 24545:

Patch Set 1:

Definitely going to finish this - sorry I’ve let it sit for so long, haven’t seen a burning need from anyone in the community for the feature (not really an excuse).

Will wrap this last week of October when I am back from vacation.

Thanks for the patience.

---

Please don’t reply on this GitHub thread. Visit golang.org/cl/95895.
After addressing review feedback, remember to publish your drafts!

[gopherbot]

Message from Agniva De Sarker:

Patch Set 1:

Zach, just checking in to see if you got a chance to look at this ?

---

Please don’t reply on this GitHub thread. Visit golang.org/cl/95895.
After addressing review feedback, remember to publish your drafts!

[gopherbot]

Message from Zachary Gershman:

Patch Set 1:

Patch Set 1:

Zach, just checking in to see if you got a chance to look at this ?

So sorry, I had not. I really apologize for putting this down so long. Going to get something done today.

---

Please don’t reply on this GitHub thread. Visit golang.org/cl/95895.
After addressing review feedback, remember to publish your drafts!

[gopherbot]

This PR (HEAD: eefe499) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/95895 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

[gopherbot]

Message from Agniva De Sarker:

Patch Set 2:

Patch Set 1:

Patch Set 1:

Zach, just checking in to see if you got a chance to look at this ?

So sorry, I had not. I really apologize for putting this down so long. Going to get something done today.

No worries. Please mark comments as "Done" once you have addressed them.

---

Please don’t reply on this GitHub thread. Visit golang.org/cl/95895.
After addressing review feedback, remember to publish your drafts!

[gopherbot]

Message from Zach Gershman:

Patch Set 2:

(2 comments)

Patch Set 1:

(1 comment)

All feedback resolved in patch set 2

---

Please don’t reply on this GitHub thread. Visit golang.org/cl/95895.
After addressing review feedback, remember to publish your drafts!

[gopherbot]

Message from Zach Gershman:

Patch Set 2:

Hey everyone - I realize the irony that I am not poking this fix after letting it languish for so long. Can someone take a quick review and run Trybot? Cheers!

---

Please don’t reply on this GitHub thread. Visit golang.org/cl/95895.
After addressing review feedback, remember to publish your drafts!

[gopherbot]

Message from Brad Fitzpatrick:

Patch Set 3: Patch Set 2 was rebased

---

Please don’t reply on this GitHub thread. Visit golang.org/cl/95895.
After addressing review feedback, remember to publish your drafts!

[gopherbot]

Message from Brad Fitzpatrick:

Patch Set 3: Run-TryBot+1

The docs say nothing about the handful of magic headers that avoid the user-provided policy function.

---

Please don’t reply on this GitHub thread. Visit golang.org/cl/95895.
After addressing review feedback, remember to publish your drafts!

[gopherbot]

Message from Gobot Gobot:

Patch Set 3:

TryBots beginning. Status page: https://farmer.golang.org/try?commit=2c209a39

---

Please don’t reply on this GitHub thread. Visit golang.org/cl/95895.
After addressing review feedback, remember to publish your drafts!

[gopherbot]

Message from Gobot Gobot:

Patch Set 3:

Build is still in progress...
This change failed on misc-vet-vetall:
See https://storage.googleapis.com/go-build-log/2c209a39/misc-vet-vetall_c13bbdb0.log

Other builds still in progress; subsequent failure notices suppressed until final report. Consult https://build.golang.org/ to see whether they are new failures. Keep in mind that TryBots currently test exactly your git commit, without rebasing. If your commit's git parent is old, the failure might've already been fixed.

---

Please don’t reply on this GitHub thread. Visit golang.org/cl/95895.
After addressing review feedback, remember to publish your drafts!

[gopherbot]

Message from Gobot Gobot:

Patch Set 3: TryBot-Result-1

11 of 19 TryBots failed:
Failed on misc-vet-vetall: https://storage.googleapis.com/go-build-log/2c209a39/misc-vet-vetall_c13bbdb0.log
Failed on freebsd-amd64-12_0: https://storage.googleapis.com/go-build-log/2c209a39/freebsd-amd64-12_0_2c61c620.log
Failed on linux-amd64: https://storage.googleapis.com/go-build-log/2c209a39/linux-amd64_81bbb00e.log
Failed on linux-386: https://storage.googleapis.com/go-build-log/2c209a39/linux-386_a0d4edd3.log
Failed on openbsd-amd64-64: https://storage.googleapis.com/go-build-log/2c209a39/openbsd-amd64-64_90044bde.log
Failed on windows-amd64-2016: https://storage.googleapis.com/go-build-log/2c209a39/windows-amd64-2016_daee7c19.log
Failed on nacl-amd64p32: https://storage.googleapis.com/go-build-log/2c209a39/nacl-amd64p32_c9b9ee21.log
Failed on linux-amd64-race: https://storage.googleapis.com/go-build-log/2c209a39/linux-amd64-race_90ccac95.log
Failed on nacl-386: https://storage.googleapis.com/go-build-log/2c209a39/nacl-386_095fe921.log
Failed on windows-386-2008: https://storage.googleapis.com/go-build-log/2c209a39/windows-386-2008_ec450911.log
Failed on js-wasm: https://storage.googleapis.com/go-build-log/2c209a39/js-wasm_5df657ed.log

Consult https://build.golang.org/ to see whether they are new failures. Keep in mind that TryBots currently test exactly your git commit, without rebasing. If your commit's git parent is old, the failure might've already been fixed.

---

Please don’t reply on this GitHub thread. Visit golang.org/cl/95895.
After addressing review feedback, remember to publish your drafts!

[gopherbot]

Message from Zach Gershman:

Patch Set 3:

Patch Set 3: Run-TryBot+1

The docs say nothing about the handful of magic headers that avoid the user-provided policy function.

Agreed - will straighten that out in the comment. Appreciate the feedback Brad.

---

Please don’t reply on this GitHub thread. Visit golang.org/cl/95895.
After addressing review feedback, remember to publish your drafts!

[gopherbot]

This PR (HEAD: dedd217) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/95895 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

[gopherbot]

Message from Agniva De Sarker:

Patch Set 4: Run-TryBot+1

---

Please don’t reply on this GitHub thread. Visit golang.org/cl/95895.
After addressing review feedback, remember to publish your drafts!

[gopherbot]

Message from Gobot Gobot:

Patch Set 4:

TryBots beginning. Status page: https://farmer.golang.org/try?commit=e3e7b054

---

Please don’t reply on this GitHub thread. Visit golang.org/cl/95895.
After addressing review feedback, remember to publish your drafts!

[gopherbot]

Message from Gobot Gobot:

Patch Set 4:

Build is still in progress...
This change failed on openbsd-amd64-64:
See https://storage.googleapis.com/go-build-log/e3e7b054/openbsd-amd64-64_a8b05182.log

Other builds still in progress; subsequent failure notices suppressed until final report. Consult https://build.golang.org/ to see whether they are new failures. Keep in mind that TryBots currently test exactly your git commit, without rebasing. If your commit's git parent is old, the failure might've already been fixed.

---

Please don’t reply on this GitHub thread. Visit golang.org/cl/95895.
After addressing review feedback, remember to publish your drafts!

[gopherbot]

Message from Gobot Gobot:

Patch Set 4: TryBot-Result-1

9 of 19 TryBots failed:
Failed on openbsd-amd64-64: https://storage.googleapis.com/go-build-log/e3e7b054/openbsd-amd64-64_a8b05182.log
Failed on freebsd-amd64-12_0: https://storage.googleapis.com/go-build-log/e3e7b054/freebsd-amd64-12_0_550f453d.log
Failed on linux-amd64: https://storage.googleapis.com/go-build-log/e3e7b054/linux-amd64_a2a89bd1.log
Failed on linux-386: https://storage.googleapis.com/go-build-log/e3e7b054/linux-386_026cb30f.log
Failed on nacl-amd64p32: https://storage.googleapis.com/go-build-log/e3e7b054/nacl-amd64p32_03354e37.log
Failed on nacl-386: https://storage.googleapis.com/go-build-log/e3e7b054/nacl-386_b1a76a71.log
Failed on linux-amd64-race: https://storage.googleapis.com/go-build-log/e3e7b054/linux-amd64-race_2757d40e.log
Failed on windows-amd64-2016: https://storage.googleapis.com/go-build-log/e3e7b054/windows-amd64-2016_408c0ba1.log
Failed on windows-386-2008: https://storage.googleapis.com/go-build-log/e3e7b054/windows-386-2008_8c882a2c.log

Consult https://build.golang.org/ to see whether they are new failures. Keep in mind that TryBots currently test exactly your git commit, without rebasing. If your commit's git parent is old, the failure might've already been fixed.

---

Please don’t reply on this GitHub thread. Visit golang.org/cl/95895.
After addressing review feedback, remember to publish your drafts!

[gopherbot]

Message from Agniva De Sarker:

Patch Set 4:

failures look real

---

Please don’t reply on this GitHub thread. Visit golang.org/cl/95895.
After addressing review feedback, remember to publish your drafts!

[gopherbot]

Message from Zach Gershman:

Patch Set 4:

Patch Set 4:

failures look real

That's interesting - are these lookup failures actually legit? Why didn't the other 10 builds fail?

---

Please don’t reply on this GitHub thread. Visit golang.org/cl/95895.
After addressing review feedback, remember to publish your drafts!

[zachgersh]

I've actually decided to close this. I feel like adding this additional knob doesn't seem like it will impact that many people and the current default behavior is probably fine as is.