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(JSRecourceLocator): Add missing slash after server root #44408

Merged
merged 1 commit into from Mar 22, 2024

Conversation

susnux
Copy link
Contributor

@susnux susnux commented Mar 22, 2024

Summary

The OC::$SERVERROOT is always returned without a trailing slash, so we need to add a slash between server root and apps directory.

Checklist

The `OC::$SERVERROOT` is always returned without a trailing slash, so we need to add a slash between server root and apps directory.

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
@susnux susnux added bug 3. to review Waiting for reviews regression labels Mar 22, 2024
@susnux susnux added this to the Nextcloud 29 milestone Mar 22, 2024
@susnux susnux requested review from a team, Altahrim, artonge and sorbaugh and removed request for a team March 22, 2024 13:53
@susnux
Copy link
Contributor Author

susnux commented Mar 22, 2024

/backport to stable28

@susnux susnux merged commit d4216bd into master Mar 22, 2024
167 checks passed
@susnux susnux deleted the fix/jsresource-locator-app-root branch March 22, 2024 15:30
@Altahrim Altahrim mentioned this pull request Mar 25, 2024
@thorsten-schwartz
Copy link

Hi all,
so what does this mean "added to backportbot"? It is neither in the daily builds nor in the stable brunch for nc 28. I think this is a major bug. No one can open a file with it's mobile.

So maybe someone can explain when it will be fixed in nc 28.

Thanks Thorsten

@susnux
Copy link
Contributor Author

susnux commented Apr 8, 2024

It is neither in the daily builds nor in the stable brunch for nc 28.

What do you mean with daily builds? This is already merged in the stable28 branch and will be released with 28.0.5, see the PR on stable28: #44413

@thorsten-schwartz
Copy link

Thank you for your quick reply. The release procedure was not quite clear to me here.
I thought bugs are fixed asap in the daily builds. From my point of view, this is a major bug. No user with a mobile and Nextcloud client can currently open files.

That's what I mean by daily builds:

https://download.nextcloud.com/server/

Greetings Thorsten

@susnux
Copy link
Contributor Author

susnux commented Apr 8, 2024

I thought bugs are fixed asap in the daily builds.

It is fixed in the daily builds, I checked nextcloud-28-daily-2024-04-07 and it is included.

@thorsten-schwartz
Copy link

Maybe I cannot update from daily....

I changed in config.php to
'updater.release.channel' => 'daily',

php ./updater/updater.phar
Nextcloud Updater - version: v28.0.2rc2-1-gba2e50f dirty

Current version is 28.0.4.

Update to Nextcloud daily available. (channel: "daily")
Following file will be downloaded automatically: https://download.nextcloud.com/server/daily/latest-stable28.zip
Open changelog ↗

Steps that will be executed:
[✔] Check for expected files
[✔] Check for write permissions
[✔] Create backup
[✔] Downloading
[✔] Verify integrity
[✔] Extracting
[✔] Enable maintenance mode
[✔] Replace entry points
[✔] Delete old files
[✔] Move new files in place
[✔] Done

Continue update? [y/N] y

Info: Pressing Ctrl-C will finish the currently running step and then stops the updater.

[✔] Check for expected files
[✔] Check for write permissions
[✔] Create backup
[✔] Downloading
[✔] Verify integrity
[✔] Extracting
[✔] Enable maintenance mode
[✔] Replace entry points
[✔] Delete old files
[✔] Move new files in place
[✔] Done

Update of code successful.

Should the "occ upgrade" command be executed? [Y/n]
Nextcloud is already latest version

Keep maintenance mode active? [y/N]
Maintenance mode already disabled

Maintenance mode is disabled

....
same behaviour with failure. What am I doing wrong?

Thanks Thorsten

@joshtrichards
Copy link
Member

joshtrichards commented Apr 8, 2024

Oh I see the confusion.

Technically your installation's code has been updated to the daily. You can verify by checking for a change in lib/private/Template/JSResourceLocator.php in your environment. Should match the one here in the PR.

The daily builds of stable releases (maintenance releases) will never trigger the needUpgrade() version check used by occ upgrade if the starting point is the latest trailing version (e.g. v28.0.4) because the version string in the daily build isn't bumped. It'll always look like the same version once deployed... Fortunately it's basically irrelevant - there are rarely db migrations or required app upgrades for daily maintenance releases. Only the code update (the piece handled by the updater.phar) is important if there aren't any db changes.

If you're curious, you can see the README for the Updater which has a bit more on the details associated with the Updater and the associated occ upgrade piece: https://github.com/nextcloud/updater?tab=readme-ov-file#how-it-works

EDIT: I guess at some point we could start bumping the patch level for daily stable builds:

server/version.php

Lines 29 to 33 in f89dabb

// We only can count up. The 4. digit is only for the internal patch level to trigger DB upgrades
// between betas, final and RCs. This is _not_ the public version number. Reset minor/patch level
// when updating major/minor version number.
$OC_Version = [28, 0, 4, 1];

This would only be important if we had a maintenance release with a db migration (which is highly unusual) and if we wanted to make sure it worked for the daily's too.

@thorsten-schwartz
Copy link

thorsten-schwartz commented Apr 11, 2024 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: open_basedir restriction in effect
5 participants