prevent delivering broken ics data via proxycontroller

Signed-off-by: Georg Ehrke <developer@georgehrke.com>
This commit is contained in:
Georg Ehrke 2018-11-23 17:44:49 +01:00
parent b3343d2ba2
commit e72267c480
No known key found for this signature in database
GPG Key ID: 9D98FD9380A1CB43
5 changed files with 30 additions and 121 deletions

View File

@ -147,7 +147,6 @@ appstore:
$(copy_command) --parents -r \
"appinfo" \
"controller" \
"http" \
"img" \
"l10n" \
"templates" \

View File

@ -25,15 +25,15 @@ use GuzzleHttp\Exception\RequestException;
use GuzzleHttp\Exception\ClientException;
use GuzzleHttp\Exception\ConnectException;
use OCA\Calendar\Http\StreamResponse;
use OCP\AppFramework\Http;
use OCP\AppFramework\Http\JSONResponse;
use OCP\AppFramework\Http\DataDisplayResponse;
use OCP\AppFramework\Controller;
use OCP\Http\Client\IClientService;
use OCP\IL10N;
use OCP\ILogger;
use OCP\IRequest;
use Sabre\VObject\Reader;
class ProxyController extends Controller {
@ -72,7 +72,7 @@ class ProxyController extends Controller {
* @NoAdminRequired
*
* @param $url
* @return StreamResponse|JSONResponse
* @return DataDisplayResponse|JSONResponse
*/
public function proxy($url) {
$client = $this->client->newClient();
@ -86,7 +86,6 @@ class ProxyController extends Controller {
// try to find a chain of 301s
do {
$clientResponse = $client->get($queryUrl, [
'stream' => true,
'allow_redirects' => $allow_redirects,
]);
@ -118,7 +117,20 @@ class ProxyController extends Controller {
'new_url' => $queryUrl,
], Http::STATUS_BAD_REQUEST);
} elseif ($done) {
$response = new StreamResponse($clientResponse->getBody());
$icsData = $clientResponse->getBody();
try {
Reader::read($icsData, Reader::OPTION_FORGIVING);
} catch(\Exception $ex) {
$response = new JSONResponse([
'message' => $this->l10n->t('The remote server did not give us access to the calendar (HTTP 404 error)'),
'proxy_code' => 404
], Http::STATUS_UNPROCESSABLE_ENTITY);
return $response;
}
$response = new DataDisplayResponse($icsData, 200);
$response->setHeaders([
'Content-Type' => 'text/calendar',
]);

View File

@ -1,50 +0,0 @@
<?php
/**
* ownCloud - Calendar App
*
* @author Georg Ehrke
* @copyright 2016 Georg Ehrke <oc.list@georgehrke.com>
*
* This code is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License, version 3,
* as published by the Free Software Foundation.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License, version 3,
* along with this program. If not, see <http://www.gnu.org/licenses/>
*
*/
namespace OCA\Calendar\Http;
use OCP\AppFramework\Http\ICallbackResponse;
use OCP\AppFramework\Http\IOutput;
use OCP\AppFramework\Http\Response;
class StreamResponse extends Response implements ICallbackResponse {
/**
* @var resource
*/
protected $stream;
/**
* @param resource $stream
*/
public function __construct ($stream) {
$this->stream = $stream;
}
/**
* @param IOutput $output a small wrapper that handles output
*/
public function callback (IOutput $output) {
fpassthru($this->stream);
}
}

View File

@ -21,6 +21,7 @@
*/
namespace OCA\Calendar\Controller;
use OCP\AppFramework\Http\DataDisplayResponse;
use PHPUnit\Framework\TestCase;
use GuzzleHttp\Exception\RequestException;
use GuzzleHttp\Exception\ClientException;
@ -112,7 +113,6 @@ class ProxyControllerTest extends TestCase {
$this->newClient->expects($this->once())
->method('get')
->with($testUrl, [
'stream' => true,
'allow_redirects' => false,
])
->will($this->returnValue($this->response0));
@ -120,11 +120,16 @@ class ProxyControllerTest extends TestCase {
->method('getStatusCode')
->with()
->will($this->returnValue(200));
$this->response0->expects($this->once())
->method('getBody')
->with()
->will($this->returnValue("BEGIN:VCALENDAR\r\nVERSION:2.0\r\nPRODID:-//Sabre//Sabre VObject 4.1.6//EN\r\nCALSCALE:GREGORIAN\r\nEND:VCALENDAR\r\n"));
$actual = $this->controller->proxy($testUrl);
$this->assertInstanceOf('OCA\Calendar\Http\StreamResponse', $actual);
$this->assertInstanceOf(DataDisplayResponse::class, $actual);
$this->assertEquals('text/calendar', $actual->getHeaders()['Content-Type']);
$this->assertEquals("BEGIN:VCALENDAR\r\nVERSION:2.0\r\nPRODID:-//Sabre//Sabre VObject 4.1.6//EN\r\nCALSCALE:GREGORIAN\r\nEND:VCALENDAR\r\n", $actual->getData());
}
public function testProxyClientException() {
@ -136,7 +141,6 @@ class ProxyControllerTest extends TestCase {
$this->newClient->expects($this->once())
->method('get')
->with($testUrl, [
'stream' => true,
'allow_redirects' => false,
])
->will($this->throwException(new ClientException('Exception Message foo bar 42',
@ -171,7 +175,6 @@ class ProxyControllerTest extends TestCase {
$this->newClient->expects($this->once())
->method('get')
->with($testUrl, [
'stream' => true,
'allow_redirects' => false,
])
->will($this->throwException(new ConnectException('Exception Message foo bar 42',
@ -203,7 +206,6 @@ class ProxyControllerTest extends TestCase {
$this->newClient->expects($this->once())
->method('get')
->with($testUrl, [
'stream' => true,
'allow_redirects' => false,
])
->will($this->throwException(new RequestException('Exception Message foo bar 42',
@ -235,7 +237,6 @@ class ProxyControllerTest extends TestCase {
$this->newClient->expects($this->once())
->method('get')
->with($testUrl, [
'stream' => true,
'allow_redirects' => false,
])
->will($this->throwException(new RequestException('Exception Message foo bar 42',
@ -267,7 +268,6 @@ class ProxyControllerTest extends TestCase {
$this->newClient->expects($this->at(0))
->method('get')
->with($testUrl, [
'stream' => true,
'allow_redirects' => false,
])
->will($this->returnValue($this->response0));
@ -282,7 +282,6 @@ class ProxyControllerTest extends TestCase {
$this->newClient->expects($this->at(1))
->method('get')
->with('http://def.abc/foobar?456', [
'stream' => true,
'allow_redirects' => false,
])
->will($this->returnValue($this->response0));
@ -309,7 +308,6 @@ class ProxyControllerTest extends TestCase {
$this->newClient->expects($this->at(0))
->method('get')
->with($testUrl, [
'stream' => true,
'allow_redirects' => false,
])
->will($this->returnValue($this->response0));
@ -320,7 +318,6 @@ class ProxyControllerTest extends TestCase {
$this->newClient->expects($this->at(1))
->method('get')
->with('http://abc.def/foobar?123' , [
'stream' => true,
'allow_redirects' => [
'max' => 5,
],
@ -330,11 +327,16 @@ class ProxyControllerTest extends TestCase {
->method('getStatusCode')
->with()
->will($this->returnValue(200));
$this->response1->expects($this->once())
->method('getBody')
->with()
->will($this->returnValue("BEGIN:VCALENDAR\r\nVERSION:2.0\r\nPRODID:-//Sabre//Sabre VObject 4.1.6//EN\r\nCALSCALE:GREGORIAN\r\nEND:VCALENDAR\r\n"));
$actual = $this->controller->proxy($testUrl);
$this->assertInstanceOf('OCA\Calendar\Http\StreamResponse', $actual);
$this->assertInstanceOf(DataDisplayResponse::class, $actual);
$this->assertEquals('text/calendar', $actual->getHeaders()['Content-Type']);
$this->assertEquals("BEGIN:VCALENDAR\r\nVERSION:2.0\r\nPRODID:-//Sabre//Sabre VObject 4.1.6//EN\r\nCALSCALE:GREGORIAN\r\nEND:VCALENDAR\r\n", $actual->getData());
}
public function testProxyMultipleRedirects() {
@ -346,21 +348,18 @@ class ProxyControllerTest extends TestCase {
$this->newClient->expects($this->at(0))
->method('get')
->with($testUrl, [
'stream' => true,
'allow_redirects' => false,
])
->will($this->returnValue($this->response0));
$this->newClient->expects($this->at(1))
->method('get')
->with('http://def.abc/foobar?456' , [
'stream' => true,
'allow_redirects' => false,
])
->will($this->returnValue($this->response1));
$this->newClient->expects($this->at(2))
->method('get')
->with('http://xyz.abc/foobar?789' , [
'stream' => true,
'allow_redirects' => false,
])
->will($this->returnValue($this->response2));
@ -403,14 +402,12 @@ class ProxyControllerTest extends TestCase {
$this->newClient->expects($this->at(0))
->method('get')
->with($testUrl, [
'stream' => true,
'allow_redirects' => false,
])
->will($this->returnValue($this->response0));
$this->newClient->expects($this->at(1))
->method('get')
->with('http://def.abc/foobar?456' , [
'stream' => true,
'allow_redirects' => false,
])
->will($this->returnValue($this->response1));
@ -445,42 +442,36 @@ class ProxyControllerTest extends TestCase {
$this->newClient->expects($this->at(0))
->method('get')
->with($testUrl, [
'stream' => true,
'allow_redirects' => false,
])
->will($this->returnValue($this->response0));
$this->newClient->expects($this->at(1))
->method('get')
->with('http://def.abc/foobar?456-0', [
'stream' => true,
'allow_redirects' => false,
])
->will($this->returnValue($this->response1));
$this->newClient->expects($this->at(2))
->method('get')
->with('http://def.abc/foobar?456-1', [
'stream' => true,
'allow_redirects' => false,
])
->will($this->returnValue($this->response2));
$this->newClient->expects($this->at(3))
->method('get')
->with('http://def.abc/foobar?456-2', [
'stream' => true,
'allow_redirects' => false,
])
->will($this->returnValue($this->response3));
$this->newClient->expects($this->at(4))
->method('get')
->with('http://def.abc/foobar?456-3', [
'stream' => true,
'allow_redirects' => false,
])
->will($this->returnValue($this->response4));
$this->newClient->expects($this->at(5))
->method('get')
->with('http://def.abc/foobar?456-4', [
'stream' => true,
'allow_redirects' => false,
])
->will($this->returnValue($this->response5));

View File

@ -1,43 +0,0 @@
<?php
/**
* Calendar App
*
* @author Georg Ehrke
* @copyright 2016 Georg Ehrke <oc.list@georgehrke.com>
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU AFFERO GENERAL PUBLIC LICENSE
* License as published by the Free Software Foundation; either
* version 3 of the License, or any later version.
*
* This library is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU AFFERO GENERAL PUBLIC LICENSE for more details.
*
* You should have received a copy of the GNU Affero General Public
* License along with this library. If not, see <http://www.gnu.org/licenses/>.
*
*/
namespace OCA\Calendar\Http;
use PHPUnit\Framework\TestCase;
class StreamResponseTest extends TestCase {
public function setUp() {
}
public function testCallback() {
$stream = fopen('data://text/plain,test_data', 'r');
$this->expectOutputString('test_data');
$ioutput = $this->getMockBuilder('OCP\AppFramework\Http\IOutput')
->disableOriginalConstructor()
->getMock();
$streamResponse = new StreamResponse($stream);
$streamResponse->callback($ioutput);
}
}