Skip to content

Update UNC path handling#2591

Open
monkeyiq wants to merge 3 commits intosimplesamlphp:simplesamlphp-2.5from
monkeyiq:2025/feb/unc-path-and-resolvePath
Open

Update UNC path handling#2591
monkeyiq wants to merge 3 commits intosimplesamlphp:simplesamlphp-2.5from
monkeyiq:2025/feb/unc-path-and-resolvePath

Conversation

@monkeyiq
Copy link
Contributor

@monkeyiq monkeyiq commented Feb 5, 2026

This

\\some-pc\some-share\some.key

Should result in

var_dump($ret);
string(29) "//some-pc/some-share/some.key"

Which according to
#2586 should be able to be loaded.

I only have a partial php win env so it needs verification against a real SSP install.

This

```
\\some-pc\some-share\some.key
```

Should result in

```
var_dump($ret);
string(29) "//some-pc/some-share/some.key"
```

Which according to
simplesamlphp#2586
should be able to be loaded.

I only have a partial php win env so it needs verification against a
real SSP install.
@tvdijen
Copy link
Member

tvdijen commented Feb 5, 2026

If this works, we should capture this case in a unit test

@monkeyiq
Copy link
Contributor Author

monkeyiq commented Feb 5, 2026

I don't know if folks are running the httpd build for win most or something else these days. From my perspective apache makes it easier if that is commonish. I guess if I installed httpd at some stage and maybe set it up to rsync the code over to that box it would perhaps help with win testing.

These for absolute path on Win
@monkeyiq
Copy link
Contributor Author

I now have a simple test case for this on windows. I have used a hack based on DIRECTORY_SEPARATOR to know if we are running on Win, perhaps there is a DEFINE or something that is more direct to use here instead.


/**
*/
public function testUNCpaths(): void
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not right. If the separator is \\ then the UNC-path should also use backslashes?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the UNC path should, I was looking at first at the comment from the original issue which mentions that php 8.2 seems to allow //server/path as well and work #2586 (comment)

If we are looking to make the returned path use '\' then we could change the rebuild code at


and see what else falls over with that and try to get those ripple effects under control again.

@monkeyiq
Copy link
Contributor Author

monkeyiq commented Feb 13, 2026

I guess I should look at installing apache on the win box for testing too. I assume nobody else on the core team is looking to test / run things on a win machine.

@tvdijen
Copy link
Member

tvdijen commented Feb 13, 2026

[..] I assume nobody else on the core team is looking to test / run things on a win machine.

I don't have anything available that we can use right now. I intend to replace and improve my home lab, but with RAM prices going through the roof it'll have to wait.. Almost a thousand euro's for 64 GB :'(

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.

2 participants