ERR invalid password when the password contains a "+"

[grisha87]

Environment

$ node --version
v12.11.1

$ npm --version
6.11.3

$ npm view ioredis version
4.14.1

Steps to reproduce

Consider the following test:

const connStrDb0 = "redis://[CENSORED]:6380?password=9R6ocWzNBdUK7sern0BDCC72b3rceZmqyrY+BVhD1wU=&tls=true";

let redis0;

describe("Checking possibility to use the same Redis with different DBs", () => {

  beforeEach(() => {
    redis0 = new Redis(connStrDb0);
  });

  afterEach(() => {
    redis0.disconnect();
  })

  it("should be possible to set the same key on different connections", async () => {
    await redis0.set("foo", "bar0");

    expect(await redis0.get("foo")).toEqual("bar0");
  });

And run it using DEBUG=ioredis:* jest index.test.js

Expected result

The test should pass :)

Actual result

ioredis has an issue and sends the password with the "+" changed to a space bar - logs attached

  ioredis:redis status[CENSORED:6380]: connecting -> connect +264ms
  ioredis:redis write command[CENSORED:6380]: 0 -> auth([ '9R6ocWzNBdUK7sern0BDCC72b3rceZmqyrY BVhD1wU=' ]) +1ms
  ioredis:redis write command[CENSORED:6380]: 0 -> info([]) +1ms

[tuananh]

related issue: nodejs/node#21841

looks like url.parse will always expand the + char, similar to how the querystring parse also does that.

[grisha87]

Thanks for checking the report @tuananh . Do you have any plans on picking this one up? The password which is used here has been generated by one of the major cloud providers so I think that fixing this one would be beneficial.

[tuananh]

@grisha87 agree this should be fixed but I don't know where should I start either. @luin any pointer?

[luin]

I just added two test cases to address this issue: 66920f1.

The correct usage for passing passwords via URL is to place it in the password part (that's user:pass@host). For your case, I think the correct way is:

redis://:9R6ocWzNBdUK7sern0BDCC72b3rceZmqyrY+BVhD1wU@[CENSORED]:6380?&tls=true

It's supported to pass passwords via queries, but passwords need to be encoded first:

redis://[CENSORED]:6380?password=9R6ocWzNBdUK7sern0BDCC72b3rceZmqyrY%2BBVhD1wU=&tls=true

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed after 7 days if no further activity occurs, but feel free to re-open a closed issue if needed.

[grisha87]

Thanks for the hint then!