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

Provide way to customize cacheKey #81

Open
chrsmith opened this Issue Dec 31, 2017 · 3 comments

Comments

Projects
None yet
2 participants
@chrsmith

chrsmith commented Dec 31, 2017

The httpcache.cacheKey function only relies on the request's Method and URL properties. This means that requests that differ in request headers are perceived as identical. This causes problems if, for example, you are using a cache across different HTTP clients that each provide a different value in the Authorization header.

As a proposed solution, a new field could be added to the httpcache.Transport type that allows clients to provide a custom key generation function function, CacheKeyFn.

type CacheKeyFn func (*http.Request) string 

type Transport struct {
	// The RoundTripper interface actually used to make requests
	// If nil, http.DefaultTransport is used
	Transport http.RoundTripper
	Cache     Cache

	// CacheKey generates a unique key for the incoming request, which is then
	// used to look up any cached responses in the provided Cache.
	// If nil, DefaultCacheKey is used.
	CacheKey    CacheKeyFn

	// If true, responses returned from the cache will be given an extra header, X-From-Cache
	MarkCachedResponses bool
}

If that sounds reasonable, I'd be happy to submit a PR with the change.

Also, for anybody who runs into the same problem, a potential workaround is to insert another http.RoundTripper that modifies the request to add a URL fragment. This obviously won't work in some situations, but does allow you to change the cache key without impacting the response from the server (in some situations).

@dmitshur

This comment has been minimized.

Show comment
Hide comment
@dmitshur

dmitshur Jan 4, 2018

Collaborator

This causes problems if, for example, you are using a cache across different HTTP clients that each provide a different value in the Authorization header.

The project README states that:

It is only suitable for use as a 'private' cache (i.e. for a web-browser or an API-client and not for a shared proxy).

Just checking, are you already aware of that?

Collaborator

dmitshur commented Jan 4, 2018

This causes problems if, for example, you are using a cache across different HTTP clients that each provide a different value in the Authorization header.

The project README states that:

It is only suitable for use as a 'private' cache (i.e. for a web-browser or an API-client and not for a shared proxy).

Just checking, are you already aware of that?

@chrsmith

This comment has been minimized.

Show comment
Hide comment
@chrsmith

chrsmith Jan 4, 2018

I had read that to be honest, but didn't quite understand what "API-client" meant. Thinking about it a bit more just now, I'm exactly trying to use httpcache for exactly the scenario the README says it isn't intended for. (Sorry about that.)

For more context, I'm trying to use it as the transport to handle conditional requests the golang GitHub client. The docs there point users towards using httpcache for that scenario, and it works great... provided you don't change the Authorization header between requests when using the same cache.

So if this isn't a scenario isn't intended to supported "won't fix" is a fine resolution. Especially if there is a history of existing thinking around the "API-client" scenarios. (And intentionally not supporting them.)

chrsmith commented Jan 4, 2018

I had read that to be honest, but didn't quite understand what "API-client" meant. Thinking about it a bit more just now, I'm exactly trying to use httpcache for exactly the scenario the README says it isn't intended for. (Sorry about that.)

For more context, I'm trying to use it as the transport to handle conditional requests the golang GitHub client. The docs there point users towards using httpcache for that scenario, and it works great... provided you don't change the Authorization header between requests when using the same cache.

So if this isn't a scenario isn't intended to supported "won't fix" is a fine resolution. Especially if there is a history of existing thinking around the "API-client" scenarios. (And intentionally not supporting them.)

@dmitshur

This comment has been minimized.

Show comment
Hide comment
@dmitshur

dmitshur Jan 5, 2018

Collaborator

So if this isn't a scenario isn't intended to supported "won't fix" is a fine resolution. Especially if there is a history of existing thinking around the "API-client" scenarios. (And intentionally not supporting them.)

I'm not the project owner, so I can't make decisions about project scope. I suspect that it'd be quite an undertaking to support private caches; primarily to validate that there are no issues that would cause secrets to be leaked to wrong clients.

For more context, I'm trying to use it as the transport to handle conditional requests the golang GitHub client. The docs there point users towards using httpcache for that scenario, and it works great... provided you don't change the Authorization header between requests when using the same cache.

The traditional way I'd recommend doing that is to have a unique *github.Client (with a unique underlying *http.Client) per user. In case that helps.

Collaborator

dmitshur commented Jan 5, 2018

So if this isn't a scenario isn't intended to supported "won't fix" is a fine resolution. Especially if there is a history of existing thinking around the "API-client" scenarios. (And intentionally not supporting them.)

I'm not the project owner, so I can't make decisions about project scope. I suspect that it'd be quite an undertaking to support private caches; primarily to validate that there are no issues that would cause secrets to be leaked to wrong clients.

For more context, I'm trying to use it as the transport to handle conditional requests the golang GitHub client. The docs there point users towards using httpcache for that scenario, and it works great... provided you don't change the Authorization header between requests when using the same cache.

The traditional way I'd recommend doing that is to have a unique *github.Client (with a unique underlying *http.Client) per user. In case that helps.

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