Skip to content

Commit 2b7e5f8

Browse files
nickvergessennickvergessen
committed
Merge spliteUserRemote with fixRemoteUrlInShareWith
1 parent d38a378 commit 2b7e5f8

3 files changed

Lines changed: 75 additions & 74 deletions

File tree

‎lib/private/share/helper.php‎

Lines changed: 39 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -218,33 +218,25 @@ public static function calculateExpireDate($defaultExpireSettings, $creationTime
218218
}
219219

220220
/**
221-
* Extracts the necessary remote name from a given link
221+
* Strips away a potential file names and trailing slashes:
222+
* - http://localhost
223+
* - http://localhost/
224+
* - http://localhost/index.php
225+
* - http://localhost/index.php/s/{shareToken}
222226
*
223-
* Strips away a potential file name, to allow
224-
* - user
225-
* - user@localhost
226-
* - user@http://localhost
227-
* - user@http://localhost/
228-
* - user@http://localhost/index.php
229-
* - user@http://localhost/index.php/s/{shareToken}
227+
* all return: http://localhost
230228
*
231229
* @param string $shareWith
232230
* @return string
233231
*/
234-
public static function fixRemoteURLInShareWith($shareWith) {
235-
if (strpos($shareWith, '@')) {
236-
list($user, $remote) = explode('@', $shareWith, 2);
237-
238-
$remote = str_replace('\\', '/', $remote);
239-
if ($fileNamePosition = strpos($remote, '/index.php')) {
240-
$remote = substr($remote, 0, $fileNamePosition);
241-
}
242-
$remote = rtrim($remote, '/');
243-
244-
$shareWith = $user . '@' . $remote;
232+
protected static function fixRemoteURL($remote) {
233+
$remote = str_replace('\\', '/', $remote);
234+
if ($fileNamePosition = strpos($remote, '/index.php')) {
235+
$remote = substr($remote, 0, $fileNamePosition);
245236
}
237+
$remote = rtrim($remote, '/');
246238

247-
return rtrim($shareWith, '/');
239+
return $remote;
248240
}
249241

250242
/**
@@ -255,10 +247,36 @@ public static function fixRemoteURLInShareWith($shareWith) {
255247
* @throws InvalidFederatedCloudIdException
256248
*/
257249
public static function splitUserRemote($id) {
258-
$pos = strrpos($id, '@');
250+
if (strpos($id, '@') === false) {
251+
throw new InvalidFederatedCloudIdException('invalid Federated Cloud ID');
252+
}
253+
254+
// Find the first character that is not allowed in user names
255+
$id = str_replace('\\', '/', $id);
256+
$posSlash = strpos($id, '/');
257+
$posColon = strpos($id, ':');
258+
259+
if ($posSlash === false && $posColon === false) {
260+
$invalidPos = strlen($id);
261+
} else if ($posSlash === false) {
262+
$invalidPos = $posColon;
263+
} else if ($posColon === false) {
264+
$invalidPos = $posSlash;
265+
} else {
266+
$invalidPos = min($posSlash, $posColon);
267+
}
268+
269+
// Find the last @ before $invalidPos
270+
$pos = $lastAtPos = 0;
271+
while ($lastAtPos !== false && $lastAtPos <= $invalidPos) {
272+
$pos = $lastAtPos;
273+
$lastAtPos = strpos($id, '@', $pos + 1);
274+
}
275+
259276
if ($pos !== false) {
260277
$user = substr($id, 0, $pos);
261278
$remote = substr($id, $pos + 1);
279+
$remote = self::fixRemoteURL($remote);
262280
if (!empty($user) && !empty($remote)) {
263281
return array($user, $remote);
264282
}

‎lib/private/share/share.php‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -749,7 +749,8 @@ public static function shareItem($itemType, $itemSource, $shareType, $shareWith,
749749
$token = \OC::$server->getSecureRandom()->getMediumStrengthGenerator()->generate(self::TOKEN_LENGTH, \OCP\Security\ISecureRandom::CHAR_LOWER . \OCP\Security\ISecureRandom::CHAR_UPPER .
750750
\OCP\Security\ISecureRandom::CHAR_DIGITS);
751751

752-
$shareWith = Helper::fixRemoteURLInShareWith($shareWith);
752+
list($user, $remote) = Helper::splitUserRemote($shareWith);
753+
$shareWith = $user . '@' . $remote;
753754
$shareId = self::put($itemType, $itemSource, $shareType, $shareWith, $uidOwner, $permissions, null, $token, $itemSourceName);
754755

755756
$send = false;
@@ -1300,8 +1301,8 @@ protected static function unshareItem(array $item, $newParent = null) {
13001301
$hookParams['deletedShares'] = $deletedShares;
13011302
\OC_Hook::emit('OCP\Share', 'post_unshare', $hookParams);
13021303
if ((int)$item['share_type'] === \OCP\Share::SHARE_TYPE_REMOTE && \OC::$server->getUserSession()->getUser()) {
1303-
$urlParts = explode('@', $item['share_with'], 2);
1304-
self::sendRemoteUnshare($urlParts[1], $item['id'], $item['token']);
1304+
list(, $remote) = Helper::splitUserRemote($item['share_with']);
1305+
self::sendRemoteUnshare($remote, $item['id'], $item['token']);
13051306
}
13061307
}
13071308

@@ -2430,7 +2431,7 @@ private static function sendRemoteShare($token, $shareWith, $name, $remote_id, $
24302431
list($user, $remote) = Helper::splitUserRemote($shareWith);
24312432

24322433
if ($user && $remote) {
2433-
$url = rtrim($remote, '/') . self::BASE_PATH_TO_SHARE_API . '?format=' . self::RESPONSE_FORMAT;
2434+
$url = $remote . self::BASE_PATH_TO_SHARE_API . '?format=' . self::RESPONSE_FORMAT;
24342435

24352436
$local = \OC::$server->getURLGenerator()->getAbsoluteURL('/');
24362437

‎tests/lib/share/helper.php‎

Lines changed: 31 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -50,74 +50,65 @@ public function testCalculateExpireDate($defaultExpireSettings, $creationTime, $
5050
$this->assertSame($expected, $result);
5151
}
5252

53-
public function fixRemoteURLInShareWithData() {
54-
$userPrefix = ['test@', 'na/me@'];
53+
public function dataTestSplitUserRemote() {
54+
$userPrefix = ['user@name', 'username'];
5555
$protocols = ['', 'http://', 'https://'];
5656
$remotes = [
5757
'localhost',
58-
'test:foobar@localhost',
5958
'local.host',
6059
'dev.local.host',
6160
'dev.local.host/path',
61+
'dev.local.host/at@inpath',
6262
'127.0.0.1',
6363
'::1',
6464
'::192.0.2.128',
65+
'::192.0.2.128/at@inpath',
6566
];
6667

67-
$testCases = [
68-
['test', 'test'],
69-
['na/me', 'na/me'],
70-
['na/me/', 'na/me'],
71-
['na/index.php', 'na/index.php'],
72-
['http://localhost', 'http://localhost'],
73-
['http://localhost/', 'http://localhost'],
74-
['http://localhost/index.php', 'http://localhost/index.php'],
75-
['http://localhost/index.php/s/token', 'http://localhost/index.php/s/token'],
76-
['http://test:foobar@localhost', 'http://test:foobar@localhost'],
77-
['http://test:foobar@localhost/', 'http://test:foobar@localhost'],
78-
['http://test:foobar@localhost/index.php', 'http://test:foobar@localhost'],
79-
['http://test:foobar@localhost/index.php/s/token', 'http://test:foobar@localhost'],
80-
];
81-
68+
$testCases = [];
8269
foreach ($userPrefix as $user) {
8370
foreach ($remotes as $remote) {
8471
foreach ($protocols as $protocol) {
85-
$baseUrl = $user . $protocol . $remote;
72+
$baseUrl = $user . '@' . $protocol . $remote;
8673

87-
$testCases[] = [$baseUrl, $baseUrl];
88-
$testCases[] = [$baseUrl . '/', $baseUrl];
89-
$testCases[] = [$baseUrl . '/index.php', $baseUrl];
90-
$testCases[] = [$baseUrl . '/index.php/s/token', $baseUrl];
74+
$testCases[] = [$baseUrl, $user, $protocol . $remote];
75+
$testCases[] = [$baseUrl . '/', $user, $protocol . $remote];
76+
$testCases[] = [$baseUrl . '/index.php', $user, $protocol . $remote];
77+
$testCases[] = [$baseUrl . '/index.php/s/token', $user, $protocol . $remote];
9178
}
9279
}
9380
}
9481
return $testCases;
9582
}
9683

9784
/**
98-
* @dataProvider fixRemoteURLInShareWithData
99-
*/
100-
public function testFixRemoteURLInShareWith($remote, $expected) {
101-
$this->assertSame($expected, \OC\Share\Helper::fixRemoteURLInShareWith($remote));
102-
}
103-
104-
/**
105-
* @dataProvider dataTestSplitUserRemoteSuccess
85+
* @dataProvider dataTestSplitUserRemote
10686
*
107-
* @param string $id
87+
* @param string $remote
10888
* @param string $expectedUser
109-
* @param string $expectedRemote
89+
* @param string $expectedUrl
11090
*/
111-
public function testSplitUserRemoteSuccess($id, $expectedUser, $expectedRemote) {
112-
list($user, $remote) = \OC\Share\Helper::splitUserRemote($id);
113-
$this->assertSame($expectedUser, $user);
114-
$this->assertSame($expectedRemote, $remote);
91+
public function testSplitUserRemote($remote, $expectedUser, $expectedUrl) {
92+
list($remoteUser, $remoteUrl) = \OC\Share\Helper::splitUserRemote($remote);
93+
$this->assertSame($expectedUser, $remoteUser);
94+
$this->assertSame($expectedUrl, $remoteUrl);
11595
}
11696

117-
public function dataTestSplitUserRemoteSuccess() {
97+
public function dataTestSplitUserRemoteError() {
11898
return array(
119-
array('user@server', 'user', 'server'),
120-
array('user@name@server', 'user@name', 'server')
99+
// Invalid path
100+
array('user@'),
101+
102+
// Invalid user
103+
array('@server'),
104+
array('us/er@server'),
105+
array('us:er@server'),
106+
107+
// Invalid splitting
108+
array('user'),
109+
array(''),
110+
array('us/erserver'),
111+
array('us:erserver'),
121112
);
122113
}
123114

@@ -130,13 +121,4 @@ public function dataTestSplitUserRemoteSuccess() {
130121
public function testSplitUserRemoteError($id) {
131122
\OC\Share\Helper::splitUserRemote($id);
132123
}
133-
134-
public function dataTestSplitUserRemoteError() {
135-
return array(
136-
array('user@'),
137-
array('@server'),
138-
array('user'),
139-
array(''),
140-
);
141-
}
142124
}

0 commit comments

Comments
 (0)