Skip to content

Comments

Bypass upgrade page when occ controller is requested#25356

Merged
PVince81 merged 1 commit intomasterfrom
checkupgrade-bypass
Jul 5, 2016
Merged

Bypass upgrade page when occ controller is requested#25356
PVince81 merged 1 commit intomasterfrom
checkupgrade-bypass

Conversation

@VicDeo
Copy link
Member

@VicDeo VicDeo commented Jul 5, 2016 •
edited
Loading

Request to domain.tld/index.php/occ/upgrade respond with a pending upgrade page otherwise

To test:

  1. increase oc version in version.phpor any app version
  2. try to access domain.tld/index.php/occ/upgrade

Expected: Response is json with the respective command result

Actual: Response is Your ownCloud is going to be upgraded HTML page

@VicDeo VicDeo added this to the 9.2 milestone Jul 5, 2016
@mention-bot
Copy link

mention-bot commented Jul 5, 2016

@VicDeo, thanks for your PR! By analyzing the annotation information on this pull request, we identified @LukasReschke, @icewind1991 and @DeepDiver1975 to be potential reviewers

@VicDeo VicDeo mentioned this pull request Jul 5, 2016
Merged
@PVince81
Copy link
Contributor

PVince81 commented Jul 5, 2016

@VicDeo how come this worked on 9.1 but not 9.0 ? Did we miss a backport or is this piece of code different ?

@VicDeo VicDeo mentioned this pull request Jul 5, 2016
Merged
6 tasks
@PVince81
Copy link
Contributor

PVince81 commented Jul 5, 2016

Ah I see, was broken on master too.

PVince81
PVince81 reviewed Jul 5, 2016
View reviewed changes
lib/base.php

$request = \OC::$server->getRequest();
// Check if requested URL matches 'index.php/occ'
$isOccControllerRequested = preg_match('|/index\.php$|', $request->getScriptName()) === 1
Copy link
Contributor

@PVince81 PVince81 Jul 5, 2016

Choose a reason for hiding this comment

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

shouldn't occ be part of the regexp?

Copy link
Contributor

@PVince81 PVince81 Jul 5, 2016

Choose a reason for hiding this comment

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

never mind, I jsut saw "getScriptName" vs "getPathInfo"

@PVince81
Copy link
Contributor

PVince81 commented Jul 5, 2016

I was on a stable9-related branch then switched to this one. An update is due.

I opened http://localhost/owncloud/index.php/occ/upgrade in the web browser but am getting an error page "Internal Server Error"

{"reqId":"UvYeYjjwbzT38z8Ev62l","remoteAddr":"127.0.0.1","app":"index","message":"Exception: {\"Exception\":\"Symfony\\\\Component\\\\Routing\\\\Exception\\\\MethodNotAllowedException\",\"Message\":\"\",\"Code\":0,\"Trace\":\"#0 \\\/srv\\\/www\\\/htdocs\\\/owncloud\\\/lib\\\/private\\\/Route\\\/Router.php(256): Symfony\\\\Component\\\\Routing\\\\Matcher\\\\UrlMatcher->match('\\\/occ\\\/upgrade')\\n#1 \\\/srv\\\/www\\\/htdocs\\\/owncloud\\\/lib\\\/base.php(864): OC\\\\Route\\\\Router->match('\\\/occ\\\/upgrade')\\n#2 \\\/srv\\\/www\\\/htdocs\\\/owncloud\\\/index.php(39): OC::handleRequest()\\n#3 {main}\",\"File\":\"\\\/srv\\\/www\\\/htdocs\\\/owncloud\\\/3rdparty\\\/symfony\\\/routing\\\/Matcher\\\/UrlMatcher.php\",\"Line\":97}","level":3,"time":"2016-07-05T09:25:58+00:00","method":"GET","url":"\/owncloud\/index.php\/occ\/upgrade","user":"--"}

@VicDeo Which method to use for that URL ?

@PVince81
Copy link
Contributor

PVince81 commented Jul 5, 2016

Steps:

  1. Check out stable9 (fdb0d4a)
  2. Switch to this branch
  3. Open web browser and see pending update
  4. Open http://localhost/owncloud/index.php/occ/upgrade in web browser

Getting the error from above.

@VicDeo
Copy link
Member Author

VicDeo commented Jul 5, 2016

@PVince81 POST https://github.com/owncloud/core/blob/master/core/routes.php#L51

@PVince81
Copy link
Contributor

PVince81 commented Jul 5, 2016

Tested, works 👍

@PVince81
Copy link
Contributor

PVince81 commented Jul 5, 2016

Second review please @DeepDiver1975 @georgehrke

@georgehrke
Copy link
Contributor

georgehrke commented Jul 5, 2016

👍

@PVince81
Copy link
Contributor

PVince81 commented Jul 5, 2016

@VicDeo please submit the backport PR for stable9.
I already tested it when patching for owncloud/updater#355 and can confirm it works.

@PVince81 PVince81 merged commit e668cf1 into master Jul 5, 2016
@PVince81 PVince81 deleted the checkupgrade-bypass branch July 5, 2016 10:04
@VicDeo VicDeo mentioned this pull request Jul 5, 2016
Merged
@VicDeo
Copy link
Member Author

VicDeo commented Jul 5, 2016 •
edited
Loading

Stable9: #25363
Stable9.1: #25366

@PVince81
and I guess stable9.1 needs this too

@PVince81
Copy link
Contributor

PVince81 commented Jul 5, 2016

and I guess stable9.1 needs this too

Yes

@VicDeo VicDeo mentioned this pull request Jul 5, 2016
Merged
@MorrisJobke MorrisJobke mentioned this pull request Jul 7, 2016
Merged
@lock
Copy link

lock bot commented Aug 5, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Aug 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

4 - To release

Projects

None yet

Milestone

10.0

Development

Successfully merging this pull request may close these issues.

5 participants

@VicDeo @mention-bot @PVince81 @georgehrke @DeepDiver1975