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

use https by default if no protocol is given. #1975

Merged
merged 1 commit into from
Nov 2, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions apps/federatedfilesharing/lib/AddressHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,22 @@ public function removeProtocolFromUrl($url) {
return $url;
}

/**
* check if the url contain the protocol (http or https)
*
* @param string $url
* @return bool
*/
public function urlContainProtocol($url) {
if (strpos($url, 'https://') === 0 ||
strpos($url, 'http://') === 0) {

return true;
}

return false;
}

/**
* Strips away a potential file names and trailing slashes:
* - http://localhost
Expand Down
59 changes: 27 additions & 32 deletions apps/federatedfilesharing/lib/Notifications.php
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,6 @@ public function sendRemoteShare($token, $shareWith, $name, $remote_id, $owner, $
list($user, $remote) = $this->addressHandler->splitUserRemote($shareWith);

if ($user && $remote) {
$url = $remote;
$local = $this->addressHandler->generateRemoteURL();

$fields = array(
Expand All @@ -99,8 +98,7 @@ public function sendRemoteShare($token, $shareWith, $name, $remote_id, $owner, $
'remote' => $local,
);

$url = $this->addressHandler->removeProtocolFromUrl($url);
$result = $this->tryHttpPostToShareEndpoint($url, '', $fields);
$result = $this->tryHttpPostToShareEndpoint($remote, '', $fields);
$status = json_decode($result['result'], true);

if ($result['success'] && ($status['ocs']['meta']['statuscode'] === 100 || $status['ocs']['meta']['statuscode'] === 200)) {
Expand Down Expand Up @@ -135,8 +133,7 @@ public function requestReShare($token, $id, $shareId, $remote, $shareWith, $perm
'remoteId' => $shareId
);

$url = $this->addressHandler->removeProtocolFromUrl($remote);
$result = $this->tryHttpPostToShareEndpoint(rtrim($url, '/'), '/' . $id . '/reshare', $fields);
$result = $this->tryHttpPostToShareEndpoint(rtrim($remote, '/'), '/' . $id . '/reshare', $fields);
$status = json_decode($result['result'], true);

$httpRequestSuccessful = $result['success'];
Expand Down Expand Up @@ -193,7 +190,7 @@ public function sendPermissionChange($remote, $remoteId, $token, $permissions) {

/**
* forward accept reShare to remote server
*
*
* @param string $remote
* @param int $remoteId
* @param string $token
Expand Down Expand Up @@ -231,8 +228,7 @@ public function sendUpdateToRemote($remote, $remoteId, $token, $action, $data =
$fields[$key] = $value;
}

$url = $this->addressHandler->removeProtocolFromUrl($remote);
$result = $this->tryHttpPostToShareEndpoint(rtrim($url, '/'), '/' . $remoteId . '/' . $action, $fields);
$result = $this->tryHttpPostToShareEndpoint(rtrim($remote, '/'), '/' . $remoteId . '/' . $action, $fields);
$status = json_decode($result['result'], true);

if ($result['success'] &&
Expand Down Expand Up @@ -270,7 +266,8 @@ protected function getTimestamp() {
}

/**
* try http post first with https and then with http as a fallback
* try http post with the given protocol, if no protocol is given we pick
* the secure one (https)
*
* @param string $remoteDomain
* @param string $urlSuffix
Expand All @@ -280,33 +277,31 @@ protected function getTimestamp() {
*/
protected function tryHttpPostToShareEndpoint($remoteDomain, $urlSuffix, array $fields) {
$client = $this->httpClientService->newClient();
$protocol = 'https://';

if ($this->addressHandler->urlContainProtocol($remoteDomain) === false) {
$remoteDomain = 'https://' . $remoteDomain;
}

$result = [
'success' => false,
'result' => '',
];
$try = 0;

while ($result['success'] === false && $try < 2) {
$endpoint = $this->discoveryManager->getShareEndpoint($protocol . $remoteDomain);
try {
$response = $client->post($protocol . $remoteDomain . $endpoint . $urlSuffix . '?format=' . self::RESPONSE_FORMAT, [
'body' => $fields,
'timeout' => 10,
'connect_timeout' => 10,
]);
$result['result'] = $response->getBody();
$result['success'] = true;
break;
} catch (\Exception $e) {
// if flat re-sharing is not supported by the remote server
// we re-throw the exception and fall back to the old behaviour.
// (flat re-shares has been introduced in Nextcloud 9.1)
if ($e->getCode() === Http::STATUS_INTERNAL_SERVER_ERROR) {
throw $e;
}
$try++;
$protocol = 'http://';

$endpoint = $this->discoveryManager->getShareEndpoint($remoteDomain);
try {
$response = $client->post($remoteDomain . $endpoint . $urlSuffix . '?format=' . self::RESPONSE_FORMAT, [
'body' => $fields,
'timeout' => 10,
'connect_timeout' => 10,
]);
$result['result'] = $response->getBody();
$result['success'] = true;
} catch (\Exception $e) {
// if flat re-sharing is not supported by the remote server
// we re-throw the exception and fall back to the old behaviour.
// (flat re-shares has been introduced in Nextcloud 9.1)
if ($e->getCode() === Http::STATUS_INTERNAL_SERVER_ERROR) {
throw $e;
}
}

Expand Down
20 changes: 20 additions & 0 deletions apps/federatedfilesharing/tests/AddressHandlerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,26 @@ public function dataTestRemoveProtocolFromUrl() {
];
}

/**
* @dataProvider dataTestUrlContainProtocol
*
* @param string $url
* @param bool $expectedResult
*/
public function testUrlContainProtocol($url, $expectedResult) {
$result = $this->addressHandler->urlContainProtocol($url);
$this->assertSame($expectedResult, $result);
}

public function dataTestUrlContainProtocol() {
return [
['http://nextcloud.com', true],
['https://nextcloud.com', true],
['nextcloud.com', false],
['httpserver.com', false],
];
}

/**
* @dataProvider dataTestFixRemoteUrl
*
Expand Down
9 changes: 3 additions & 6 deletions apps/federatedfilesharing/tests/NotificationsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ public function setUp() {

/**
* get instance of Notifications class
*
*
* @param array $mockedMethods methods which should be mocked
* @return Notifications | \PHPUnit_Framework_MockObject_MockObject
*/
Expand All @@ -81,7 +81,7 @@ private function getInstance(array $mockedMethods = []) {
]
)->setMethods($mockedMethods)->getMock();
}

return $instance;
}

Expand All @@ -94,7 +94,7 @@ private function getInstance(array $mockedMethods = []) {
* @param bool $expected
*/
public function testSendUpdateToRemote($try, $httpRequestResult, $expected) {
$remote = 'remote';
$remote = 'http://remote';
$id = 42;
$timestamp = 63576;
$token = 'token';
Expand All @@ -106,9 +106,6 @@ public function testSendUpdateToRemote($try, $httpRequestResult, $expected) {
->with($remote, '/'.$id.'/unshare', ['token' => $token, 'data1Key' => 'data1Value'])
->willReturn($httpRequestResult);

$this->addressHandler->expects($this->once())->method('removeProtocolFromUrl')
->with($remote)->willReturn($remote);

// only add background job on first try
if ($try === 0 && $expected === false) {
$this->jobList->expects($this->once())->method('add')
Expand Down