Remove sid-link value from posts

[Dragosvr92]

The sid value is only relevant to the user access. The user doesnt care about it, but needs it to access the site.

Many dont bother to just copy the link without the sid and they post it entirely... When a link is detected to have a ?sid= value in it, the phpbb engine should just ignore it and post the link without it. But, i think i recall hearing the sid will be embedded into a cookie, as it should... Its still present in 3.1.5 though.

[DavidIQ]

But, i think i recall hearing the sid will be embedded into a cookie, as it should... Its still present in 3.1.5 though.

Will be? Has been since version 2 of phpBB, maybe even before that. The reason the SID query string still exists is because of people blocking cookies and incorrect site cookie settings.

I do find the idea of removing the SID as something that might be beneficial though. Could probably just be done through the new parser, if not already being done.

[Dragosvr92]

Is it really...? I think it isnt removed completely, from all url types. It seems that only the MCP and ACP links still keep the sids on my board.
Im not blocking cookies and They are configured properly. Its still poping up. I was referring to this btw, viewtopic.php?f=84&t=42577&start=10#p266831


Ive sent a ticket. https://tracker.phpbb.com/browse/PHPBB3-14105

[DavidIQ]

There has always been a cookie, otherwise you couldn't stay logged in. If there is an SID being appended to ACP and MCP sessions then that is either a bug or intentional.

[Dragosvr92]

I know there was. But it always gave me the impression that not the entire functionality is embedded into a cookie.

[JoshyPHP]

There's something in message_parser::validate_url() that seems to remove sid= from local URLs. I can't explain why it calls append_sid() afterward though.

Code: Select all

// Is this a link to somewhere inside this board? If so then remove the session id from the url
if (strpos($url, generate_board_url()) !== false && strpos($url, 'sid=') !== false)
{
	$url = preg_replace('/(&|\?)sid=[0-9a-f]{32}&/', '\1', $url);
	$url = preg_replace('/(&|\?)sid=[0-9a-f]{32}$/', '', $url);
	$url = append_sid($url);
}

message_parser::validate_url() is not used in 3.2. I've sent a PR that runs the equivalent code in 3.2. However, generate_board_url() adds a mystery dependency to request and I think this code is bloat. Removing generate_board_url() and just removing sid= from every URL makes it a fair bit less crummy, although I think the whole thing is more trouble than it's worth.

[Nicofuma]

Honestly I don't see any good reason to keep the sid from any url.

[JoshyPHP]

Honestly I don't see any good reason to keep the sid from any url.

Cool, let's shred this diff then. I removed the calls to generate_board_url() and append_sid().

https://github.com/phpbb/phpbb/pull/3847

[Paul]

There's something in message_parser::validate_url() that seems to remove sid= from local URLs. I can't explain why it calls append_sid() afterward though.

Code: Select all

// Is this a link to somewhere inside this board? If so then remove the session id from the url
if (strpos($url, generate_board_url()) !== false && strpos($url, 'sid=') !== false)
{
	$url = preg_replace('/(&|\?)sid=[0-9a-f]{32}&/', '\1', $url);
	$url = preg_replace('/(&|\?)sid=[0-9a-f]{32}$/', '', $url);
	$url = append_sid($url);
}

message_parser::validate_url() is not used in 3.2. I've sent a PR that runs the equivalent code in 3.2. However, generate_board_url() adds a mystery dependency to request and I think this code is bloat. Removing generate_board_url() and just removing sid= from every URL makes it a fair bit less crummy, although I think the whole thing is more trouble than it's worth.

It calls append_sid so if a SID is needed (Client doesn't support cookies, board configured wrong), it adds the SID for the current session, and not from the user who posted.

[JoshyPHP]

It calls append_sid so if a SID is needed (Client doesn't support cookies, board configured wrong), it adds the SID for the current session, and not from the user who posted.

But do you know why it calls append_sid()? Why would it remove the SID of a link and replace it with the SID of the user posting/editing the text?