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.

Sign up for GitHub

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

Jump to bottom

http.send caching should incorporate all params in key #1980

Closed
tsandall opened this issue Jan 3, 2020 · 0 comments · Fixed by #2067
Closed

http.send caching should incorporate all params in key #1980

tsandall opened this issue Jan 3, 2020 · 0 comments · Fixed by #2067
Assignees
@patrick-east
Labels
bug

Comments

@tsandall
Copy link
Member

tsandall commented Jan 3, 2020

The http.send function caches/memoizes responses using the method and url as the key. The problem is that for non-restful APIs this will result in incorrect answers. The http.send function should default to performing a comparison on all function parameters. If this becomes too expensive we could add an optional cache key parameter in the future.
@tsandall tsandall changed the title http.send caching should incorporate headers/body http.send caching should incorporate all params in key Jan 3, 2020
@tsandall tsandall added the bug label Jan 3, 2020
@patrick-east patrick-east self-assigned this Feb 5, 2020
@patrick-east patrick-east mentioned this issue Feb 7, 2020
Merged
patrick-east added a commit to patrick-east/opa that referenced this issue Feb 19, 2020
@patrick-east
topdown: Make http.send() caching use full request
c6a587f 
Previously it just used a string key with the method and url. This
causes problems if headers or body contents affect the response, and
users were seeing cached responses returned for different requests.

The new version takes into account the entire parameters object that
is provided to the builtin function.

Fixes: open-policy-agent#1980
Signed-off-by: Patrick East <east.patrick@gmail.com>
tsandall pushed a commit that referenced this issue Feb 20, 2020
@patrick-east @tsandall
topdown: Make http.send() caching use full request
9f20f22 
Previously it just used a string key with the method and url. This
causes problems if headers or body contents affect the response, and
users were seeing cached responses returned for different requests.

The new version takes into account the entire parameters object that
is provided to the builtin function.

Fixes: #1980
Signed-off-by: Patrick East <east.patrick@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@patrick-east patrick-east

Labels
bug
Projects
Open Policy Agent
Archived in project
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

topdown: Make http.send() caching use full request patrick-east/opa
2 participants
@tsandall @patrick-east