Skip to content

Commit

Permalink
Adjust integration tests for validation plugin
Browse files Browse the repository at this point in the history
  • Loading branch information
@mrow4a
mrow4a committed Aug 10, 2017
1 parent 685af44 commit 975b2f4
Show file tree
Hide file tree
Showing 6 changed files with 109 additions and 82 deletions.
37 changes: 25 additions & 12 deletions apps/dav/lib/Connector/Sabre/ValidateRequestPlugin.php
View file
Expand Up @@ -21,16 +21,23 @@

namespace OCA\DAV\Connector\Sabre;

use OCP\IConfig;
use Sabre\DAV\Exception\ServiceUnavailable;
use Sabre\DAV\ServerPlugin;

/**
* Class ValidateRequestPlugin is a plugin which examines correctness of
* the webdav request, checking if accessing a service e.g. "dav"
* the request has proper headers/structure.
*
* @see https://github.com/owncloud/core/issues/28200
* @package OCA\DAV\Connector\Sabre
*/
class ValidateRequestPlugin extends ServerPlugin {

/**
* Reference to main server object
*
* @var Server
* @var \Sabre\DAV\Server
*/
private $server;

Expand All @@ -41,6 +48,17 @@ class ValidateRequestPlugin extends ServerPlugin {
*/
private $service;

/**
* This plugin ensures that all request directed to specific
* services (type $service as decided by resolveService($service) in remote.php)
* contain correct headers and their structure is correct
*
* Currently supported:
* 'webdav'
* 'dav'
*
* @var string $service
*/
public function __construct($service) {
$this->service = $service;
}
Expand All @@ -58,7 +76,7 @@ public function __construct($service) {
*/
public function initialize(\Sabre\DAV\Server $server) {
$this->server = $server;
$this->server->on('beforeMethod', [$this, 'checkValidity'], 1);
$this->server->on('beforeMethod:PUT', [$this, 'checkValidityPut'], 100);
}

/**
Expand All @@ -68,18 +86,13 @@ public function initialize(\Sabre\DAV\Server $server) {
* @throws ServiceUnavailable
* @return bool
*/
public function checkValidity() {
public function checkValidityPut() {
$request = $this->server->httpRequest;
$method = $request->getMethod();

// Verify if optional OC headers are routed in the proper endpoint
if ($method === 'PUT'
&& $request->hasHeader('OC-Chunk-Offset')
&& ($this->service != 'dav')) {
throw new ServiceUnavailable('Specified OC-Chunk-Offset header is allowed only in dav endpoint');
} else if ($method === 'PUT'
&& ($request->hasHeader('HTTP_OC_CHUNKED') || $request->hasHeader('Oc-Chunked'))
&& ($this->service != 'webdav')) {
if (($request->hasHeader('HTTP_OC_CHUNKED') || $request->hasHeader('Oc-Chunked'))
&& ($this->service !== 'webdav')) {
// Headers not allowed in new dav endpoint
throw new ServiceUnavailable('Specified header (HTTP_OC_CHUNKED/OC-Chunked header) is allowed only in webdav endpoint');
}

Expand Down
18 changes: 2 additions & 16 deletions apps/dav/tests/unit/Connector/Sabre/ValidateRequestPluginTest.php
View file
Expand Up @@ -43,20 +43,6 @@ public function setUp() {
$this->server = new \Sabre\DAV\Server();
}

/**
* @expectedException \Sabre\DAV\Exception\ServiceUnavailable
* @expectedExceptionMessage Specified OC-Chunk-Offset header is allowed only in dav endpoint
*/
public function testNewChunkingInOldEndpoint() {
$headers['OC-Chunk-Offset'] = 0;
$this->plugin = new ValidateRequestPlugin('webdav');

$this->server->httpRequest = new \Sabre\HTTP\Request('PUT', 'http://server/root_url/remote.php/webdav/testchunkold.txt', $headers);
$this->plugin->initialize($this->server);

$this->plugin->checkValidity();
}

/**
* @expectedException \Sabre\DAV\Exception\ServiceUnavailable
* @expectedExceptionMessage Specified header (HTTP_OC_CHUNKED/OC-Chunked header) is allowed only in webdav endpoint
Expand All @@ -68,7 +54,7 @@ public function testOldChunkingInNewEndpoint1() {
$this->server->httpRequest = new \Sabre\HTTP\Request('PUT', 'http://server/root_url/remote.php/dav/files/user/testnew.txt', $headers);
$this->plugin->initialize($this->server);

$this->plugin->checkValidity();
$this->plugin->checkValidityPut();
}

/**
Expand All @@ -82,7 +68,7 @@ public function testOldChunkingInNewEndpoint2() {
$this->server->httpRequest = new \Sabre\HTTP\Request('PUT', 'http://server/root_url/remote.php/dav/files/user/testnew.txt', $headers);
$this->plugin->initialize($this->server);

$this->plugin->checkValidity();
$this->plugin->checkValidityPut();
}

}
22 changes: 13 additions & 9 deletions tests/integration/features/bootstrap/Checksums.php
View file
Expand Up @@ -159,14 +159,18 @@ public function theOcChecksumHeaderShouldNotBeThere()
* @param string $checksum
*/
public function userUploadsChunkFileOfWithToWithChecksum($user, $num, $total, $data, $destination, $checksum) {
$num -= 1;
$data = \GuzzleHttp\Stream\Stream::factory($data);
$file = $destination . '-chunking-42-' . $total . '-' . $num;
$this->response = $this->makeDavRequest($user,
'PUT',
$file,
['OC-Checksum' => $checksum, 'OC-Chunked' => '1'],
$data,
"files");
try {
$num -= 1;
$data = \GuzzleHttp\Stream\Stream::factory($data);
$file = $destination . '-chunking-42-' . $total . '-' . $num;
$this->response = $this->makeDavRequest($user,
'PUT',
$file,
['OC-Checksum' => $checksum, 'OC-Chunked' => '1'],
$data,
"files");
} catch (\GuzzleHttp\Exception\RequestException $ex) {
$this->response = $ex->getResponse();
}
}
}
57 changes: 30 additions & 27 deletions tests/integration/features/bootstrap/WebDav.php
View file
Expand Up @@ -680,19 +680,6 @@ public function uploadWithAllMechanisms($user, $source, $destination, $overwrite
}
}
if ($dav === 'new') {
// old chunking style applied to new endpoint 🙈
if (!$overwriteMode) {
$suffix = '-' . $dav . 'dav-oldchunking';
}
try {
// FIXME: prepending new dav path because the chunking utility functions are messed up
$this->userUploadsAFileToWithChunks($user, $source, '/files/' . $user . '/' . ltrim($destination, '/') . $suffix, 'old');
$responses[] = $this->response;
} catch (ServerException $e) {
$responses[] = $e->getResponse();
}

// new chunking style applied to new endpoint
if (!$overwriteMode) {
$suffix = '-' . $dav . 'dav-newchunking';
}
Expand Down Expand Up @@ -834,29 +821,41 @@ public function userCreatedAFolder($user, $destination){
*/
public function userUploadsChunkedFile($user, $num, $total, $data, $destination)
{
$num -= 1;
$data = \GuzzleHttp\Stream\Stream::factory($data);
$file = $destination . '-chunking-42-' . $total . '-' . $num;
$this->makeDavRequest($user, 'PUT', $file, ['OC-Chunked' => '1'], $data, "uploads");
try {
$num -= 1;
$data = \GuzzleHttp\Stream\Stream::factory($data);
$file = $destination . '-chunking-42-' . $total . '-' . $num;
$this->makeDavRequest($user, 'PUT', $file, ['OC-Chunked' => '1'], $data, "uploads");
} catch (\GuzzleHttp\Exception\RequestException $ex) {
$this->response = $ex->getResponse();
}
}

/**
* @Given user :user creates a new chunking upload with id :id
*/
public function userCreatesANewChunkingUploadWithId($user, $id)
{
$destination = '/uploads/'.$user.'/'.$id;
$this->makeDavRequest($user, 'MKCOL', $destination, [], null, "uploads");
try {
$destination = '/uploads/'.$user.'/'.$id;
$this->makeDavRequest($user, 'MKCOL', $destination, [], null, "uploads");
} catch (\GuzzleHttp\Exception\RequestException $ex) {
$this->response = $ex->getResponse();
}
}

/**
* @Given user :user uploads new chunk file :num with :data to id :id
*/
public function userUploadsNewChunkFileOfWithToId($user, $num, $data, $id)
{
$data = \GuzzleHttp\Stream\Stream::factory($data);
$destination = '/uploads/'. $user .'/'. $id .'/' . $num;
$this->makeDavRequest($user, 'PUT', $destination, [], $data, "uploads");
try {
$data = \GuzzleHttp\Stream\Stream::factory($data);
$destination = '/uploads/'. $user .'/'. $id .'/' . $num;
$this->makeDavRequest($user, 'PUT', $destination, [], $data, "uploads");
} catch (\GuzzleHttp\Exception\RequestException $ex) {
$this->response = $ex->getResponse();
}
}

/**
Expand Down Expand Up @@ -885,11 +884,15 @@ public function userUploadsNewChunkFileOfWithToIdWithChecksum($user, $num, $data
*/
public function userMovesNewChunkFileWithIdToMychunkedfile($user, $id, $dest)
{
$source = '/uploads/' . $user . '/' . $id . '/.file';
$destination = $this->baseUrlWithoutOCSAppendix() . $this->getDavFilesPath($user) . $dest;
$this->makeDavRequest($user, 'MOVE', $source, [
'Destination' => $destination
], null, "uploads");
try {
$source = '/uploads/' . $user . '/' . $id . '/.file';
$destination = $this->baseUrlWithoutOCSAppendix() . $this->getDavFilesPath($user) . $dest;
$this->makeDavRequest($user, 'MOVE', $source, [
'Destination' => $destination
], null, "uploads");
} catch (\GuzzleHttp\Exception\RequestException $ex) {
$this->response = $ex->getResponse();
}
}


Expand Down
25 changes: 7 additions & 18 deletions tests/integration/features/checksums.feature
View file
Expand Up @@ -115,24 +115,6 @@ Feature: checksums
And user "user0" downloads the file "/myChecksumFileCopy.txt"
Then The header checksum should match "SHA1:3ee962b839762adb0ad8ba6023a4690be478de6f"

Scenario: Uploading a chunked file with checksum should return the checksum in the propfind using new dav path
Given using new dav path
And user "user0" exists
And user "user0" uploads chunk file "1" of "3" with "AAAAA" to "/myChecksumFile.txt" with checksum "MD5:45a72715acdd5019c5be30bdbb75233e"
And user "user0" uploads chunk file "2" of "3" with "BBBBB" to "/myChecksumFile.txt" with checksum "MD5:45a72715acdd5019c5be30bdbb75233e"
And user "user0" uploads chunk file "3" of "3" with "CCCCC" to "/myChecksumFile.txt" with checksum "MD5:45a72715acdd5019c5be30bdbb75233e"
When user "user0" request the checksum of "/myChecksumFile.txt" via propfind
Then The webdav checksum should match "SHA1:acfa6b1565f9710d4d497c6035d5c069bd35a8e8 MD5:45a72715acdd5019c5be30bdbb75233e ADLER32:1ecd03df"

Scenario: Uploading a chunked file with checksum should return the checksum in the download header using new dav path
Given using new dav path
And user "user0" exists
And user "user0" uploads chunk file "1" of "3" with "AAAAA" to "/myChecksumFile.txt" with checksum "MD5:45a72715acdd5019c5be30bdbb75233e"
And user "user0" uploads chunk file "2" of "3" with "BBBBB" to "/myChecksumFile.txt" with checksum "MD5:45a72715acdd5019c5be30bdbb75233e"
And user "user0" uploads chunk file "3" of "3" with "CCCCC" to "/myChecksumFile.txt" with checksum "MD5:45a72715acdd5019c5be30bdbb75233e"
When user "user0" downloads the file "/myChecksumFile.txt"
Then The header checksum should match "SHA1:acfa6b1565f9710d4d497c6035d5c069bd35a8e8"

@local_storage
Scenario: Downloading a file from local storage has correct checksum using new dav path
Given using new dav path
Expand Down Expand Up @@ -183,3 +165,10 @@ Feature: checksums
Then The following headers should be set
| OC-Checksum | SHA1:ce5582148c6f0c1282335b87df5ed4be4b781399 |

## Validation Plugin or Old Endpoint Specific

Scenario: Uploading a old method chunked file with checksum should fail using new dav path
Given using new dav path
And user "user0" exists
When user "user0" uploads chunk file "1" of "3" with "AAAAA" to "/myChecksumFile.txt" with checksum "MD5:45a72715acdd5019c5be30bdbb75233e"
Then the HTTP status code should be "503"
32 changes: 32 additions & 0 deletions tests/integration/features/webdav-related-new-endpoint.feature
View file
Expand Up @@ -548,3 +548,35 @@ Feature: webdav-related-new-endpoint
And as "user1" the folder "/folderB/ONE/TWO" exists
And User "user1" checks id of file "/folderB/ONE"

## Validation Plugin or Old Endpoint Specific

Scenario: New chunked upload MKDIR using old dav path should fail
Given using old dav path
And user "user0" exists
When user "user0" creates a new chunking upload with id "chunking-42"
Then the HTTP status code should be "409"

Scenario: New chunked upload PUT using old dav path should fail
Given using new dav path
And user "user0" exists
And user "user0" creates a new chunking upload with id "chunking-42"
When using old dav path
And user "user0" uploads new chunk file "1" with "AAAAA" to id "chunking-42"
Then the HTTP status code should be "404"

Scenario: New chunked upload MOVE using old dav path should fail
Given using new dav path
And user "user0" exists
And user "user0" creates a new chunking upload with id "chunking-42"
And user "user0" uploads new chunk file "2" with "BBBBB" to id "chunking-42"
And user "user0" uploads new chunk file "3" with "CCCCC" to id "chunking-42"
And user "user0" uploads new chunk file "1" with "AAAAA" to id "chunking-42"
When using old dav path
And user "user0" moves new chunk file with id "chunking-42" to "/myChunkedFile.txt"
Then the HTTP status code should be "404"

Scenario: Upload to new dav path using old way should fail
Given using new dav path
And user "user0" exists
When user "user0" uploads chunk file "1" of "3" with "AAAAA" to "/myChunkedFile.txt"
Then the HTTP status code should be "503"

0 comments on commit 975b2f4

Please sign in to comment.