Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(setup-checks): Ensure URL with webroot works #47883

Merged
merged 2 commits into from
Sep 13, 2024
Merged

Conversation

susnux
Copy link
Contributor

@susnux susnux commented Sep 10, 2024

Summary

We basically mock the way URLGenerator::getAbsoluteURL works, so we must make sure that the URL might already contain the webroot. Because baseURL and cliURL also contain the webroot we need to remove the webroot from the URL first.

Checklist

@susnux susnux added this to the Nextcloud 31 milestone Sep 10, 2024
@susnux
Copy link
Contributor Author

susnux commented Sep 10, 2024

/backport to stable30

@kesselb

This comment was marked as resolved.

@susnux

This comment was marked as resolved.

@kesselb

This comment was marked as resolved.

@susnux

This comment was marked as resolved.

@kesselb

This comment was marked as resolved.

@susnux

This comment was marked as resolved.

@kesselb
Copy link
Contributor

kesselb commented Sep 11, 2024

Too many forward slashes for me 🙈

@kesselb
Copy link
Contributor

kesselb commented Sep 11, 2024

/backport to stable29

Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

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

Psalm found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.

Copy link
Contributor

@come-nc come-nc left a comment

Choose a reason for hiding this comment

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

I think fixing the wrong type in the phpdoc should fix psalm errors.
Nice job 👍

@come-nc
Copy link
Contributor

come-nc commented Sep 13, 2024

phpunit tests also need to be adapted to change from getSystemValue to getSystemValueString

if ($removeWebroot) {
$segments = parse_url($url);
$port = isset($segments['port']) ? (':' . $segments['port']) : '';
return $segments['scheme'] . '://' . $segments['host'] . $port;

Check notice

Code scanning / Psalm

PossiblyUndefinedArrayOffset Note

Possibly undefined array key $segments['scheme'] on array{fragment?: string, host?: string, pass?: string, path?: string, port?: int, query?: string, scheme?: string, user?: string}|false
if ($removeWebroot) {
$segments = parse_url($url);
$port = isset($segments['port']) ? (':' . $segments['port']) : '';
return $segments['scheme'] . '://' . $segments['host'] . $port;

Check notice

Code scanning / Psalm

PossiblyUndefinedArrayOffset Note

Possibly undefined array key $segments['host'] on array{fragment?: string, host?: string, pass?: string, path?: string, port?: int, query?: string, scheme?: string, user?: string}|false
susnux and others added 2 commits September 13, 2024 13:06
We basically mock the way `URLGenerator::getAbsoluteURL` works,
so we must make sure that the URL might already contain the webroot.
Because `baseURL` and `cliURL` also contain the webroot we need to remove
the webroot from the URL first.

Co-authored-by: Ferdinand Thiessen <opensource@fthiessen.de>
Co-authored-by: Daniel <mail@danielkesselberg.de>
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
… path

Co-authored-by: Ferdinand Thiessen <opensource@fthiessen.de>
Co-authored-by: Côme Chilliet <91878298+come-nc@users.noreply.github.com>
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants