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

fix(redis): Remove empty object body from GET requests to Redis API #3251

Merged
merged 1 commit into from
Mar 11, 2025

Conversation

sbosio
Copy link
Contributor

@sbosio sbosio commented Mar 11, 2025

Description

There's a bug in the request function from the shared Redis API lib file used for most of redis:... commands, where the default value for the request body is an empty object ({}). That makes all GET requests using that function to send that empty body when, according to standards, GET requests shouldn't send a body.

Here we change the default to a null value, so no body gets sent with GET requests and add an empty object body to all non-GET requests that would've inherited the previous default to avoid behavior changes.

Testing

Checkout this branch, ensure you're running Node 20.x and run yarn && yarn build. For the tests you will need access to a test app with a Redis database installed. If you don't have one at hand let me know and I can add you to one of my test apps.

There will be no behavior change on execution of the commands, only the headers sent with the request will change.

  • Run the following command through your usual Heroku CLI:
DEBUG=http,http:headers heroku redis:info <redis-addon-name> -a <test-app-name>

You should see on the Redis API request debug output (GET https://api.heroku.com/redis/v0/databases/...) that the following headers are set:

accept: 'application/json'
content-length: '2'
content-type: 'application/json'
  • Compare against the same command, but run through this branch's version:
DEBUG=http,http:headers ./bin/run redis:info <redis-addon-name> -a <test-app-name>

You should see on the Redis API request debug output (GET https://api.heroku.com/redis/v0/databases/...) that no content-type and content-length headers are included and the accept header is now:

accept: 'application/vnd.heroku+json; version=3'
  • Repeat for other commands that make GET requests like redis:wait and redis:credentials. The same behavior should be observed.

SOC2 Compliance

GUS Work Item: W-18015751

@sbosio sbosio requested a review from a team as a code owner March 11, 2025 21:28
@troycoll
Copy link
Contributor

Tested all redis: commands against the environment that was giving us problems, 🟢 🟢 🟢

@sbosio sbosio merged commit 0938ee7 into main Mar 11, 2025
8 checks passed
@sbosio sbosio deleted the sbosio/fix-redis-commands branch March 11, 2025 21:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants