poolConnection.changeUser changing for the whole Pool.getConnection()

[TheVoid-0]

This may be intentional, but if it is, i'd need some help.
Basically what happens is that when you get a pool connection and then change it's user (i use this because i need to change the database) and a error occurs, like bad credentials for the new user, instead of only compromising the current connection that i've got, it compromises the whole pool, whenever i try to get another connection from the pool, the pool tries to use the same wrong credentials that i've passed to the connection.changeUser. This makes the pool unusable in my code because i can never get a connection again to be able to change user, in my understanding, changing the user on the connection should only affect that pooledConnection and not the whole pool.

I have one pool per server, and that server has many dbs, that's why i need to change the connection user from that specific pool, when some error occurs that makes that connection unusable, i use connection.destroy(), but this don't prevent the pool to keep trying to connect the new pool.getConnection() with the same wrong credentials i passed to the one i destroyed.

I would like to get some info if this behaviour is intentional, or if it really has this behaviour at all, maybe i'm fuc**ing things up somewhere... Thanks for helping in advance!

[TheVoid-0]

Here is a repo to try reproduce the issue . I've already fix my problem by releasing the connection instead of destroying it. But i think this shouldn't matter because i may want to actually destroy a connection that got access denied instead of releasing it.

[diovannaschell]

I'm also having this problem and would like to know if this is the expected behavior. In my current test, when you get two or more connections from the same pool and change the user using the connection.changeUser() in any of them, it changes for every connection on that pool, even the ones already in use, that IMO does not make much sense, if I wanted to change for every connection I would look for a method to do this on the pool itself, the connection should only affect itself. Is this really intended? Like @TheVoid-0 I also want to use multiple databases on the same pool and is currently unviable

[sidorares]

@diovannaschell @TheVoid-0 hopefully #1437 (comment) should help

[sidorares]

Oh maybe I'm reading this wrong @TheVoid-0 . So you thing changeUser is updating the pool config and next time connection is created it's using that config instead of the original one?

const pool = createPool({ user: 'A', db: 'B' });
const connA = await pool.getConnection();
await connA.changeUser({ user: 'B', db: 'C' });
const connB = await pool.getConnection(); // <- id expect this one to be connected as { user: 'A', db: 'B' }

[TheVoid-0]

Thanks for answering @sidorares . I couldn't get the draft of the comment you linked, can you explain how it is related? But answering your question: Yes you understood it correctly, my application indeed expect that every pooled connection can have its own configuration

const pool = createPool({ user: 'A' }); // My pools don't have a DB by default, cuz they're atached to the server
const connA = await pool.getConnection();
await connA.changeUser({ db: 'DB_A' }); // I assign a DB based on a user request
await connA.query('SHOW TABLES'); // shows tables from DB_A

const connB = await pool.getConnection(); // I'd expect that the connB will have no DB selected because default pool don't hav
connB.changeUser({ db: 'DB_B' }); // Now the problem here is that even though connA was already taken from the pool
// when i change de DB from the brand new connB it will also change from connA

await connA.query('SHOW TABLES'); // Now shows tables from DB_B

await connB.query('SHOW TABLES'); // Shows tables from DB_B

If this is the intended behavior, i fail to see why it should be like that, to change every connection at once, shouldn't the pool expose a method that does so?

pool.changeUser({db: new_DB}) // changes for every connection that owns this pool

I realize that normal situations would require one pool for only one DB, but I really can't do this, i have thousands of DBs in different servers, i can't have a pool for each, that'd be the same of opening a connection to everyone all the time, i know that changeUser requires the underlying connection to connect again, but with a pool for each server i can control how many connections i have with each, even though the performance will be the same as createConnection().

So can you help me in any way? Thanks in advance for spending your time through this issue @sidorares .

[sidorares]

I believe the issue is that connection's currentConfig in the case of pool is a reference ( not a copy ) to a pool config, and when ChangeUser command is sent it updates pool config as a result ( I thing intention was to change connection config only ). Relevant code:

node-mysql2/lib/commands/change_user.js

Lines 30 to 33 in ef28341

	this.currentConfig.user = this.user;
	this.currentConfig.password = this.password;
	this.currentConfig.database = this.database;
	this.currentConfig.charsetNumber = this.charsetNumber;

@TheVoid-0 do you think this is something you can contribute to fix ( happy to guide )? Even PR with new failing unit test would be a good start

[sidorares]

I realize that normal situations would require one pool for only one DB

I don't see where changing a db of a pool can be a problem, so totally normal from my view. As I mentioned in the comment we plan to automatically reset connections state as connection is released to the pool ( not implemented yet but should be done soon )

[TheVoid-0]

Hey there @sidorares , many thanks for answering me again. You were correct once more, i took a look on the code following from where you pointed and after changing the instantiation of the PoolConnection to use a new config object instead of the Pool config reference, the issue was resolved

// lib/pool.js

// Before
new PoolConnection(this, {config: this.config.connectionConfig})
// After
new PoolConnection(this, {config: {...this.config.connectionConfig}})

Now my question is, do you think this will suffice as a fix? Actually i used Object.assin({}, this.config.connectionConfig) but anyway, i'm willing to continue to investigate if you think there's more to it, and i hope this minor change will not have any impact on the rest of the code. I would apreciate immensely if you could guide me into submitting the complete fix to this repo, then i would be able to continue from where i stopped with my server pools

[sidorares]

I think it should suffice, maybe worth checking a bit more if we can expect objects / arrays potentially as config keys ( in that case better to do proper deep clone )

[sidorares]

another thing to think - if connection is updating its local config copy, what's expected behaviour when we add automatic connection reset on release ( and why do we need to update local config at all? )