aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDavid King <imkingdavid@gmail.com>2012-11-19 12:37:20 -0500
committerDavid King <imkingdavid@gmail.com>2012-11-19 12:37:20 -0500
commitf8614bfc84ba9b9cc814b8f78e343005620f18f8 (patch)
tree2bc656eecad6052aa04134c4506ab10876d6fe63
parent30043502814cd42d824dc1d6bcb25bebc60adbed (diff)
downloadforums-f8614bfc84ba9b9cc814b8f78e343005620f18f8.tar
forums-f8614bfc84ba9b9cc814b8f78e343005620f18f8.tar.gz
forums-f8614bfc84ba9b9cc814b8f78e343005620f18f8.tar.bz2
forums-f8614bfc84ba9b9cc814b8f78e343005620f18f8.tar.xz
forums-f8614bfc84ba9b9cc814b8f78e343005620f18f8.zip
[feature/controller] Check for proper status codes from controllers
PHPBB3-10864
-rw-r--r--phpBB/app.php7
-rw-r--r--phpBB/includes/event/kernel_exception_subscriber.php9
-rw-r--r--tests/functional/extension_controller_test.php13
-rw-r--r--tests/functional/fixtures/ext/foo/bar/config/routing.yml4
-rw-r--r--tests/functional/fixtures/ext/foo/bar/controller/controller.php5
5 files changed, 29 insertions, 9 deletions
diff --git a/phpBB/app.php b/phpBB/app.php
index c085f966c8..d93208d585 100644
--- a/phpBB/app.php
+++ b/phpBB/app.php
@@ -24,12 +24,7 @@ $user->session_begin();
$auth->acl($user->data);
$user->setup('app');
-// Until we fix the issue with relative paths, we have to fake path info to
-// allow urls like app.php?controller=foo/bar
-$controller = $request->variable('controller', '');
-$uri = '/' . $controller;
-$symfony_request = phpbb_create_symfony_request($uri, $request);
-
+$symfony_request = phpbb_create_symfony_request($request);
$http_kernel = $phpbb_container->get('http_kernel');
$response = $http_kernel->handle($symfony_request);
$response->send();
diff --git a/phpBB/includes/event/kernel_exception_subscriber.php b/phpBB/includes/event/kernel_exception_subscriber.php
index e2668d4560..e843df2e68 100644
--- a/phpBB/includes/event/kernel_exception_subscriber.php
+++ b/phpBB/includes/event/kernel_exception_subscriber.php
@@ -18,6 +18,7 @@ if (!defined('IN_PHPBB'))
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
use Symfony\Component\HttpKernel\KernelEvents;
use Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent;
+use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
use Symfony\Component\HttpFoundation\Response;
class phpbb_event_kernel_exception_subscriber implements EventSubscriberInterface
@@ -56,9 +57,11 @@ class phpbb_event_kernel_exception_subscriber implements EventSubscriberInterfac
{
page_header($this->user->lang('INFORMATION'));
+ $exception = $event->getException();
+
$this->template->assign_vars(array(
'MESSAGE_TITLE' => $this->user->lang('INFORMATION'),
- 'MESSAGE_TEXT' => $event->getException()->getMessage(),
+ 'MESSAGE_TEXT' => $exception->getMessage(),
));
$this->template->set_filenames(array(
@@ -67,7 +70,9 @@ class phpbb_event_kernel_exception_subscriber implements EventSubscriberInterfac
page_footer(true, false, false);
- $response = new Response($this->template->assign_display('body'), 404);
+
+ $status_code = $exception instanceof NotFoundHttpException ? $exception->getStatusCode() : 500;
+ $response = new Response($this->template->assign_display('body'), $status_code);
$event->setResponse($response);
}
diff --git a/tests/functional/extension_controller_test.php b/tests/functional/extension_controller_test.php
index 482fe9dfd3..c7b585354e 100644
--- a/tests/functional/extension_controller_test.php
+++ b/tests/functional/extension_controller_test.php
@@ -116,11 +116,20 @@ class phpbb_functional_extension_controller_test extends phpbb_functional_test_c
{
$this->phpbb_extension_manager->enable('foo/bar');
$crawler = $this->request('GET', 'app.php?controller=foo/baz');
- $this->assertEquals(404, $this->client->getResponse()->getStatus());
+ $this->assertEquals(500, $this->client->getResponse()->getStatus());
$this->assertContains('Missing value for argument #1: test in class phpbb_ext_foo_bar_controller:baz', $crawler->filter('body')->text());
$this->phpbb_extension_manager->purge('foo/bar');
}
+ public function test_exception_thrown_status_code()
+ {
+ $this->phpbb_extension_manager->enable('foo/bar');
+ $crawler = $this->request('GET', 'app.php?controller=foo/exception');
+ $this->assertEquals(500, $this->client->getResponse()->getStatus());
+ $this->assertContains('Exception thrown from foo/exception route', $crawler->filter('body')->text());
+ $this->phpbb_extension_manager->purge('foo/bar');
+ }
+
/**
* Check the error produced by extension at ./ext/does/not/exist.
*
@@ -133,6 +142,8 @@ class phpbb_functional_extension_controller_test extends phpbb_functional_test_c
public function test_error_ext_disabled_or_404()
{
$crawler = $this->request('GET', 'app.php?controller=does/not/exist');
+ // This is 500 response because the exception is thrown from within Symfony
+ // and does not provide a exception code, so we assign it 500 by default
$this->assertEquals(404, $this->client->getResponse()->getStatus());
$this->assertContains('No route found for "GET /does/not/exist"', $crawler->filter('body')->text());
}
diff --git a/tests/functional/fixtures/ext/foo/bar/config/routing.yml b/tests/functional/fixtures/ext/foo/bar/config/routing.yml
index 123f5f1b73..7eb604f746 100644
--- a/tests/functional/fixtures/ext/foo/bar/config/routing.yml
+++ b/tests/functional/fixtures/ext/foo/bar/config/routing.yml
@@ -9,3 +9,7 @@ foo_baz_controller:
foo_template_controller:
pattern: /foo/template
defaults: { _controller: foo_bar.controller:template }
+
+foo_exception_controller:
+ pattern: /foo/foo_exception
+ defaults: { _controller: foo_bar.controller:exception }
diff --git a/tests/functional/fixtures/ext/foo/bar/controller/controller.php b/tests/functional/fixtures/ext/foo/bar/controller/controller.php
index 50ea5d034b..5a91b5f681 100644
--- a/tests/functional/fixtures/ext/foo/bar/controller/controller.php
+++ b/tests/functional/fixtures/ext/foo/bar/controller/controller.php
@@ -27,4 +27,9 @@ class phpbb_ext_foo_bar_controller
return $this->helper->render('foo_bar_body.html');
}
+
+ public function exception()
+ {
+ throw new phpbb_controller_exception('Exception thrown from foo/exception route');
+ }
}