Maybe totally unrelated maybe not.
06 May 2021
PHP 7.4.19 Released!
The PHP development team announces the immediate availability of PHP 7.4.19. This release reverts a bug related to PDO_pgsql that was introduced in PHP 7.4.18.

The PHP development team announces the immediate availability of PHP 8.0.6. This release reverts a bug related to PDO_pgsql that was introduced in PHP 8.0.5.

PHP 8.0 users that use PDO_pgsql are encouraged to upgrade to this version.

Maybe something wrong with template style parameter json in the installation SQL script for PostrgeSQL. Will check later today or tomorrow morning if I can reproduce and if so, find the reason and fix it (if it’s not the PDO issue mentioned above).

@richard67 I think you are confused. This issue is about sessions and the fact pgsql doesn't store the session_id as a string, and loadColumn returns resources and not the value of the session_id.

The "look and feel" of the admin template is forked to a different issue #33741 :)

I see. Well I just had in mind that the last SQL stuff we did was for the repaint, so that might have lead me the wrong way.

@peteruoi Unfortunately this doesn't look like the root problem, as PHP Bug #80892 was about PDO::PARAM_INT is treated the same as PDO::PARAM_STR

No experience with Postgres but it works well for me on my local computer :D

I use docker containers for development.

I have tested with PHP 7.4.18 and PHP 8.0.5 and both seem to have this issue when used with postgresql 13.2 (Debian 13.2-1.pgdg100+1)

I note your postgresql version is 11.11

so I tried and it worked ok - I could logout with no issues (postgresql 11.11 (Debian 11.11-1.pgdg90+1))

so I tried and it worked ok - I could logout with no issues (postgresql 12.6 (Debian 12.6-1.pgdg100+1))

Thus I have to conclude that this appears to be a postgresql 13 issue only.

I see nothing in the open issues tracker for version 13 that might be related https://wiki.postgresql.org/wiki/PostgreSQL_13_Open_Items

On installation I did get this logged to my docker console, I doubt its related but logging here incase this means something else to someone.

postgres_1   | 2021-05-11 09:15:06.849 UTC [79] WARNING:  nonstandard use of \\ in a string literal at character 193
postgres_1   | 2021-05-11 09:15:06.849 UTC [79] HINT:  Use the escape string syntax for backslashes, e.g., E'\\'.
postgres_1   | 2021-05-11 09:15:06.849 UTC [79] WARNING:  nonstandard use of \\ in a string literal at character 1189
postgres_1   | 2021-05-11 09:15:06.849 UTC [79] HINT:  Use the escape string syntax for backslashes, e.g., E'\\'.
postgres_1   | 2021-05-11 09:15:06.849 UTC [79] WARNING:  nonstandard use of \\ in a string literal at character 1974
postgres_1   | 2021-05-11 09:15:06.849 UTC [79] HINT:  Use the escape string syntax for backslashes, e.g., E'\\'.
postgres_1   | 2021-05-11 09:15:06.849 UTC [79] WARNING:  nonstandard use of \\ in a string literal at character 3255
postgres_1   | 2021-05-11 09:15:06.849 UTC [79] HINT:  Use the escape string syntax for backslashes, e.g., E'\\'.
postgres_1   | 2021-05-11 09:15:06.849 UTC [79] WARNING:  nonstandard use of \\ in a string literal at character 4132
postgres_1   | 2021-05-11 09:15:06.849 UTC [79] HINT:  Use the escape string syntax for backslashes, e.g., E'\\'.
postgres_1   | 2021-05-11 09:15:06.849 UTC [79] WARNING:  nonstandard use of \\ in a string literal at character 5165
postgres_1   | 2021-05-11 09:15:06.849 UTC [79] HINT:  Use the escape string syntax for backslashes, e.g., E'\\'.
postgres_1   | 2021-05-11 09:15:06.849 UTC [79] WARNING:  nonstandard use of \\ in a string literal at character 5932
postgres_1   | 2021-05-11 09:15:06.849 UTC [79] HINT:  Use the escape string syntax for backslashes, e.g., E'\\'.
postgres_1   | 2021-05-11 09:15:06.849 UTC [79] WARNING:  nonstandard use of \\ in a string literal at character 6817
postgres_1   | 2021-05-11 09:15:06.849 UTC [79] HINT:  Use the escape string syntax for backslashes, e.g., E'\\'.
postgres_1   | 2021-05-11 09:15:06.849 UTC [79] WARNING:  nonstandard use of \\ in a string literal at character 7895
postgres_1   | 2021-05-11 09:15:06.849 UTC [79] HINT:  Use the escape string syntax for backslashes, e.g., E'\\'.
postgres_1   | 2021-05-11 09:15:06.849 UTC [79] WARNING:  nonstandard use of \\ in a string literal at character 8730
postgres_1   | 2021-05-11 09:15:06.849 UTC [79] HINT:  Use the escape string syntax for backslashes, e.g., E'\\'.
postgres_1   | 2021-05-11 09:15:06.849 UTC [79] WARNING:  nonstandard use of \\ in a string literal at character 9808
postgres_1   | 2021-05-11 09:15:06.849 UTC [79] HINT:  Use the escape string syntax for backslashes, e.g., E'\\'.
postgres_1   | 2021-05-11 09:15:06.849 UTC [79] WARNING:  nonstandard use of \\ in a string literal at character 10647
postgres_1   | 2021-05-11 09:15:06.849 UTC [79] HINT:  Use the escape string syntax for backslashes, e.g., E'\\'.
postgres_1   | 2021-05-11 09:15:06.849 UTC [79] WARNING:  nonstandard use of \\ in a string literal at character 11727
postgres_1   | 2021-05-11 09:15:06.849 UTC [79] HINT:  Use the escape string syntax for backslashes, e.g., E'\\'.
postgres_1   | 2021-05-11 09:15:06.849 UTC [79] WARNING:  nonstandard use of \\ in a string literal at character 12541
postgres_1   | 2021-05-11 09:15:06.849 UTC [79] HINT:  Use the escape string syntax for backslashes, e.g., E'\\'.
postgres_1   | 2021-05-11 09:15:06.849 UTC [79] WARNING:  nonstandard use of \\ in a string literal at character 13557
postgres_1   | 2021-05-11 09:15:06.849 UTC [79] HINT:  Use the escape string syntax for backslashes, e.g., E'\\'.
postgres_1   | 2021-05-11 09:15:06.849 UTC [79] WARNING:  nonstandard use of \\ in a string literal at character 14286
postgres_1   | 2021-05-11 09:15:06.849 UTC [79] HINT:  Use the escape string syntax for backslashes, e.g., E'\\'.
postgres_1   | 2021-05-11 09:15:06.849 UTC [79] WARNING:  nonstandard use of \\ in a string literal at character 15446
postgres_1   | 2021-05-11 09:15:06.849 UTC [79] HINT:  Use the escape string syntax for backslashes, e.g., E'\\'.
postgres_1   | 2021-05-11 09:15:06.849 UTC [79] WARNING:  nonstandard use of \\ in a string literal at character 16243
postgres_1   | 2021-05-11 09:15:06.849 UTC [79] HINT:  Use the escape string syntax for backslashes, e.g., E'\\'.
postgres_1   | 2021-05-11 09:15:06.849 UTC [79] WARNING:  nonstandard use of \\ in a string literal at character 17284
postgres_1   | 2021-05-11 09:15:06.849 UTC [79] HINT:  Use the escape string syntax for backslashes, e.g., E'\\'.
postgres_1   | 2021-05-11 09:15:06.850 UTC [79] WARNING:  nonstandard use of \\ in a string literal at character 18116
postgres_1   | 2021-05-11 09:15:06.850 UTC [79] HINT:  Use the escape string syntax for backslashes, e.g., E'\\'.
postgres_1   | 2021-05-11 09:15:06.850 UTC [79] WARNING:  nonstandard use of \\ in a string literal at character 18279
postgres_1   | 2021-05-11 09:15:06.850 UTC [79] HINT:  Use the escape string syntax for backslashes, e.g., E'\\'.
postgres_1   | 2021-05-11 09:15:06.850 UTC [79] WARNING:  nonstandard use of \\ in a string literal at character 18538
postgres_1   | 2021-05-11 09:15:06.850 UTC [79] HINT:  Use the escape string syntax for backslashes, e.g., E'\\'.
postgres_1   | 2021-05-11 09:15:06.850 UTC [79] WARNING:  nonstandard use of \\ in a string literal at character 18693
postgres_1   | 2021-05-11 09:15:06.850 UTC [79] HINT:  Use the escape string syntax for backslashes, e.g., E'\\'.
postgres_1   | 2021-05-11 09:15:06.850 UTC [79] WARNING:  nonstandard use of \\ in a string literal at character 19457
postgres_1   | 2021-05-11 09:15:06.850 UTC [79] HINT:  Use the escape string syntax for backslashes, e.g., E'\\'.
postgres_1   | 2021-05-11 09:15:06.850 UTC [79] WARNING:  nonstandard use of \\ in a string literal at character 20499
postgres_1   | 2021-05-11 09:15:06.850 UTC [79] HINT:  Use the escape string syntax for backslashes, e.g., E'\\'.

@PhilETaylor The "WARNING: nonstandard use of \ in a string literal" is something we ignore since long time because it doesn't do harm. Not that I'm happy with that, but I got used to it.

No worries :-) I dont think its in any way connected with the btexta conversion to a string when using loadColumn :)

Also works for me on 13.2 on Windows

Strange :) well @alikon and I can both replicate it so I know Im not going mad :)

Did you test with other databases before in the same browser sessions, so maybe some stuff remaining in the session cookie from previous tests with MySQL or MariaDB which causes that?

Nope - completely new platform built by docker and new browser session each time.

I debugged it before right down to the root issue - the root issue is that running loadColumn to get the session_id from the sessions table returns resources and not strings as session ids, and therefore they cannot be passed to PHP methods expecting the session_id to be a string.

This is 100% a Postgres issue and not a session or browser issue.

Look like I can re-procedure it now. I change Session Handle to Files instead of Database and could see the issue now.

Look like I can re-procedure it now. I change Session Handle to Files instead of Database and could see the issue now.

Same for me with PostgreSQL 11.7.

@PhilETaylor The following change in the SessionManager.php here https://github.com/joomla/joomla-cms/blob/4.0-dev/libraries/src/Session/SessionManager.php#L49-L52 works for me:

public function destroySession($sessionId): bool
{
	if (is_resource($sessionId) && get_resource_type($sessionId) === 'stream')
	{
		$sessionId = stream_get_contents($sessionId);
	}

	return $this->sessionHandler->destroy($sessionId);
}

But I have no idea if that is right, not specifying any type for that parameter (since type "mixed"or a union type "string | resource" would only be available with PHP 8, as far as I understand the PHP docs).

What do you think?

Would that be a way to go, or should we add more checks for the type inside that function, i.e. check if it's a string, when not using a type specified for the function parameter?

@alikon Do you know why we use "bytea" for the session_id column in PostgreSQL, and not a normal string?

i've made a a quick test with your snippets and i'm able to logout from back & front now
iirc bytea was used for performance reason

@alikon No idea if my fix is good but it seems to work. I'm just not sure when the old session data in database shall be deleted when the option to collect session metadata is enabled and the session handler is file.

That code is certainly one way to go, and the way that I would've done, which leads me to believe it's probably a quick hack, and not the correct course of action ha ha

I know nothing of pgsql , but surely the Joomla! Database layer method loadColumn should never be returning resources?

I would say that we should not implement that kind of hack code into our codebase. We need to find a better way. @nibra Do you have any idea to address this issue?

anyway bytea is already used on j3...

since #19708

I guess the real question now then is, is this a problem already in Joomla! 3 that nobody other than myself has ever come across?

If that is the case then I cannot see this being a release blocker!

Although it is something that should still be addressed as soon as possible.

So i have tested Joomla 3, and it handles the session destroying differently as it has less features than Joomla 4.

However, interestingly, the same "bug" in the Joomla database layer exists, where loadColum (correctly?) returns binary data as a resource when selecting from a bytea column

If I run this code in Joomla 3, It returns an array of resources and not an array of session_ids as strings.

$query = $this->db->getQuery(true)
			->select($this->db->quoteName('session_id'))
			->from($this->db->quoteName('#__session'))
			->where($this->db->quoteName('userid') . ' = '. $user['id']);
		$sessionIds = $this->db->setQuery($query)->loadColumn();
		// $sessionIds is an array of resources... 

Joomla 3 seems to always handle the returned session_id as binary correctly using $db->quoteBinary($id) in the destroy method of the Joomla database session storage layer - but for redis and the other storages there seems to be no correct handling of this as binary data, because there is no destroy logic in the methods, they just return true.

Therefore the first REAL problem is the new type hints on method params that are string in SessionManger.php (Just like the error message clearly states). Type hinting session_ids as string is plainly wrong as pgsql will return binary/resource for the session_id and Joomla

So I removed that type hint:

public function destroySession($sessionId): bool

But then... onto the next issue. DatabaseHandler.php:175 doesn't quoteBinary the $session_id, ok so I change that too (but now we are in joomla-fromawork/session again... The param hint says that session_id should be an integer /facepalm... )

	/**
	 * Destroy a session
	 *
	 * @param   integer  $session_id  The session ID being destroyed
	 *
	 * @return  boolean  True on success, false otherwise
	 *
	 * @since   2.0.0-beta
	 */
	public function destroy($session_id)
	{
		try
		{
			$query = $this->db->getQuery(true)
				->delete($this->db->quoteName('#__session'))
				->where($this->db->quoteName('session_id') . ' = ' . $this->db->quoteBinary($session_id));

			// Remove a session from the database.
			$this->db->setQuery($query)->execute();

			return true;
		}
		catch (\Exception $e)
		{
			return false;
		}
	}

So another bug fixed there... but then... quoteBinary method in the PgsqlDriver.php tries to pass the resource to bin2hex... so you get another error... so then Im back to adding @richard67 's code at this level...

	/**
	 * Quotes a binary string to database requirements for use in database queries.
	 *
	 * @param   string  $data  A binary string to quote.
	 *
	 * @return  string  The binary quoted input string.
	 *
	 * @since   1.7.0
	 */
	public function quoteBinary($data)
	{
		if (is_resource($data) && get_resource_type($data) === 'stream')
		{
			$data = stream_get_contents($data);
		}

		return "decode('" . bin2hex($data) . "', 'hex')";
	}

But all this is tacky I think...

What this really needs is for someone who really understands pgsql to correctly rewrite joomla/session framework package to that session_ids are always treated correctly.. unless I missed something.

try this #33787 - quick simple and effective, and compatible with existing type hints :)

just looked at it
shouldnt be better don't touch lib
something like here on plugin https://github.com/joomla/joomla-cms/blob/4.0-dev/plugins/user/joomla/joomla.php#L373

well that would code would have to loop over the session ids and cast them all to string, a pointless loop for 99% of the cases.

anyway better to discuss on code
please test #33787

or as an alternative #33817

I prefer doing it in a central place like in the PR #33787 so that we do not have to worry about casting data each time we call the method.

As it is in real life, everything has its advantages and disadvantages.

The advantage of Nicola's way is we don't have to change the signature of the public method destroySession, so it's still strict with parameter type.

The disadvantage is that we have to do that at every place where we want to call that function with a parameter coming from a loadColumn(), e.g. here:

The perfect way would be to raise the minimum PHP requirement to 8.0 and use a union type for the function parameter and do the stream reading in destroySession. ?

The advantage of Nicola's way is we don't have to change the signature of the public method destroySession, so it's still strict with parameter type.

Actually, you do not have to change the signature of destroySession method. In SessionManager https://github.com/joomla/joomla-cms/blob/4.0-dev/libraries/src/Session/SessionManager.php#L63 , we do the check to make sure $sessionId is a string before calling destroySession. I prefer doing this in a central place instead of doing every where.

@joomdonation I see ... see #33819 .