Disable UI when web updater is disabled in config.php by PVince81 · Pull Request #331 · nextcloud/updater

PVince81 · 2021-02-17T08:38:28Z

For #281

kesselb

LGTM 👍

index.php

nickvergessen · 2021-02-17T09:31:42Z

Isn't that code also used in the phar?
You can check php_sapi_name() == 'cli' to see if it's ran via web or CLI

PVince81 · 2021-02-17T09:46:42Z

Isn't that code also used in the phar?
You can check php_sapi_name() == 'cli' to see if it's ran via web or CLI

I've checked box.json and it uses updater.php as entry point which itself is based on NC\Updater\Application.php.

So from my understanding this change is not needed, but I could still do it just to stay on the safe side.

PVince81 · 2021-02-17T09:53:26Z

I've now added a CLI check

kesselb · 2021-02-17T10:52:42Z

For some reason (#313) index.php and master/lib/Updater.php needs to contain the same code. At least to have a green ci.

kesselb · 2021-02-17T10:55:29Z

index.php

 		require_once $configFileName;
 		$this->configValues = $CONFIG;

+		if (php_sapi_name() !== 'cli' && $this->configValues['upgrade.disable-web'] ?? false) {


Suggested change

if (php_sapi_name() !== 'cli' && $this->configValues['upgrade.disable-web'] ?? false) {

if (PHP_SAPI !== 'cli' && ($this->configValues['upgrade.disable-web'] ?? false)) {

not sure... my PHP expertise level is inferior compared to @kesselb and @nickvergessen, so not sure which to pick.
battle of the experts ? 😄

It's your choice ;) I would prefer to wrap the second part because it's easier to understand.

ah of course. I meant the sapi thing

Some years ago I read an article comparing function call vs reading a constant in PHP. Constant was a bit faster and that's what I do since then: Use the constant if possible. That might be outdated nowadays.

https://github.com/nextcloud/server/blob/d4d33e3095fb5d0797ec7c3bb8c93c2d1c905ade/lib/base.php#L563

cough

PVince81 · 2021-02-17T11:48:43Z

I've aligned the classes now, let's see...

PVince81 · 2021-02-17T14:01:40Z

I've raised #332 for the CI issues as there seems to be multiple topics

Signed-off-by: Vincent Petry <vincent@nextcloud.com>

PVince81 · 2021-02-26T09:50:45Z

rebased for CI

szaimen · 2021-02-26T10:24:23Z

CI is green now

PVince81 · 2021-02-26T10:29:11Z

@kesselb can you merge ? thanks

MorrisJobke · 2021-03-16T21:33:28Z

Thank you @PVince81, @nickvergessen and @kesselb for digging through the updater code 👍

MorrisJobke · 2021-03-23T12:42:49Z

/backport to stable19

MorrisJobke · 2021-03-23T12:42:52Z

/backport to stable20

MorrisJobke · 2021-03-23T12:42:55Z

/backport to stable21

PVince81 requested review from ChristophWurst, kesselb and nickvergessen February 17, 2021 08:38

PVince81 self-assigned this Feb 17, 2021

PVince81 mentioned this pull request Feb 17, 2021

Ability to Disable Web Updater #281

Closed

kesselb approved these changes Feb 17, 2021

View reviewed changes

index.php Show resolved Hide resolved

PVince81 requested a review from kesselb February 17, 2021 10:11

kesselb reviewed Feb 17, 2021

View reviewed changes

kesselb approved these changes Feb 17, 2021

View reviewed changes

J0WI mentioned this pull request Feb 19, 2021

Updater takes you to dashboard nextcloud/docker#1287

Closed

PVince81 added 3 commits February 26, 2021 10:50

Disable UI when web updater is disabled in config.php

8929ea1

Signed-off-by: Vincent Petry <vincent@nextcloud.com>

Add CLI check in index.php

4a5035b

Signed-off-by: Vincent Petry <vincent@nextcloud.com>

Align updater class with index.php

b53b4e3

Signed-off-by: Vincent Petry <vincent@nextcloud.com>

PVince81 force-pushed the bugfix/281/block-updater-when-disabled branch from 550edca to b53b4e3 Compare February 26, 2021 09:50

nickvergessen approved these changes Feb 26, 2021

View reviewed changes

ChristophWurst merged commit e490062 into master Feb 26, 2021

delete-merged-branch bot deleted the bugfix/281/block-updater-when-disabled branch February 26, 2021 12:13

backportbot-nextcloud bot added the backport-request label Mar 23, 2021

backportbot-nextcloud bot mentioned this pull request Mar 23, 2021

[stable19] Disable UI when web updater is disabled in config.php #350

Merged

backportbot-nextcloud bot removed the backport-request label Mar 23, 2021

This was referenced Mar 23, 2021

[stable21] Disable UI when web updater is disabled in config.php #351

Merged

[stable20] Disable UI when web updater is disabled in config.php #352

Merged

joshtrichards mentioned this pull request Apr 22, 2024

Ensure CLI environment in PHAR execution mode #330

Closed

	if (php_sapi_name() !== 'cli' && $this->configValues['upgrade.disable-web'] ?? false) {
	if (PHP_SAPI !== 'cli' && ($this->configValues['upgrade.disable-web'] ?? false)) {

Conversation

PVince81 commented Feb 17, 2021

Uh oh!

kesselb left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nickvergessen commented Feb 17, 2021

Uh oh!

PVince81 commented Feb 17, 2021

Uh oh!

PVince81 commented Feb 17, 2021

Uh oh!

kesselb commented Feb 17, 2021

Uh oh!

kesselb Feb 17, 2021

Choose a reason for hiding this comment

Uh oh!

PVince81 Feb 17, 2021

Choose a reason for hiding this comment

Uh oh!

kesselb Feb 17, 2021

Choose a reason for hiding this comment

Uh oh!

PVince81 Feb 17, 2021

Choose a reason for hiding this comment

Uh oh!

kesselb Feb 17, 2021

Choose a reason for hiding this comment

Uh oh!

nickvergessen Feb 17, 2021

Choose a reason for hiding this comment

Uh oh!

PVince81 commented Feb 17, 2021

Uh oh!

PVince81 commented Feb 17, 2021

Uh oh!

PVince81 commented Feb 26, 2021

Uh oh!

szaimen commented Feb 26, 2021

Uh oh!

PVince81 commented Feb 26, 2021

Uh oh!

MorrisJobke commented Mar 16, 2021

Uh oh!

MorrisJobke commented Mar 23, 2021

Uh oh!

MorrisJobke commented Mar 23, 2021

Uh oh!

MorrisJobke commented Mar 23, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants