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

net/http: Client should cache 301/308 redirect #29776

Open
yonderblue opened this issue Jan 17, 2019 · 7 comments
Open

net/http: Client should cache 301/308 redirect #29776

yonderblue opened this issue Jan 17, 2019 · 7 comments
Labels
NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Milestone

Comments

@yonderblue
Copy link

What version of Go are you using (go version)?

go1.11.1

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

linux amd64, android arm

What did you do?

Used http.Client for multiple calls against a url that returns 301 or 308.

What did you expect to see?

Second invocation to use a cached response from the first.

What did you see instead?

The first call before the Location is followed is always made.

Is there any reason this choice was made or has the logic just never been put in?

@agnivade
Copy link
Contributor

/cc @bradfitz

@bradfitz
Copy link
Contributor

Never considered either way.

@bradfitz bradfitz added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Jan 17, 2019
@bradfitz bradfitz added this to the Go1.13 milestone Jan 17, 2019
@rsc
Copy link
Contributor

rsc commented May 1, 2019

The http.Client does no caching of responses at all. It's not just 301/308.

What is the context here? Is there a situation where this is causing problems? Also, if you are making the same request repeatedly, why is it problematic to be repeating the redirected call but not problematic to be repeating the final call?

@rsc rsc modified the milestones: Go1.13, Go1.14 May 1, 2019
@rsc rsc modified the milestones: Go1.14, Backlog Oct 9, 2019
@odeke-em
Copy link
Member

I believe @Gaillard filed this issue since 301: Moved Permanently and 308: Permanent Redirect both have permanency in them, thus we can assume that those responses will never change, so perhaps we can consider these as cacheable.

@odeke-em
Copy link
Member

The issue though is that what happens if we have uncapped/unlimited response caching and many responses are cached; what if a server responds with refresh deadline/tokens in the 301/308 responses?

@odeke-em
Copy link
Member

@neild @bradfitz what do y'all think about the points that I raised here #29776 (comment)? Or perhaps we can use some sort of bounded cache or perhaps an LRU of fixed size containing domain+code whose keys when being evicted pop from the map[domain+code]->response

@seankhliao
Copy link
Member

I don't think the defaults for net/http should have any caching at all, my expectation is for requests to correspond directly to requests sent on the wire.
Adding caching makes the behavior less predictable and subverts user control over the requests they send.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Projects
None yet
Development

No branches or pull requests

7 participants
@bradfitz @rsc @yonderblue @agnivade @odeke-em @seankhliao and others