From 06158693c7b846518abfe9d72491fc7376e457f3 Mon Sep 17 00:00:00 2001 From: David King Date: Fri, 19 Oct 2012 19:54:19 -0400 Subject: [PATCH 01/67] [feature/controller] Implement a front controller PHPBB3-10864 --- phpBB/.htaccess | 11 +- phpBB/app.php | 29 ++++ phpBB/common.php | 9 +- phpBB/composer.json | 1 + phpBB/composer.lock | 62 +++++++- phpBB/config/routing.yml | 9 ++ phpBB/config/services.yml | 63 ++++++++ phpBB/includes/controller/helper.php | 142 ++++++++++++++++++ phpBB/includes/controller/provider.php | 94 ++++++++++++ phpBB/includes/controller/resolver.php | 123 +++++++++++++++ .../includes/controller/route_collection.php | 36 +++++ phpBB/includes/functions.php | 35 ++++- phpBB/includes/template/template.php | 19 ++- phpBB/index.php | 40 +---- phpBB/language/en/app.php | 59 ++++++++ phpBB/web.config | 12 ++ tests/controller/config/routing.yml | 3 + tests/controller/config/services.yml | 3 + tests/controller/controller_test.php | 75 +++++++++ tests/controller/ext/foo/config/routing.yml | 3 + tests/controller/ext/foo/config/services.yml | 3 + tests/controller/ext/foo/controller.php | 23 +++ tests/controller/includes/controller/foo.php | 23 +++ 23 files changed, 818 insertions(+), 59 deletions(-) create mode 100644 phpBB/app.php create mode 100644 phpBB/config/routing.yml create mode 100644 phpBB/includes/controller/helper.php create mode 100644 phpBB/includes/controller/provider.php create mode 100644 phpBB/includes/controller/resolver.php create mode 100644 phpBB/includes/controller/route_collection.php create mode 100644 phpBB/language/en/app.php create mode 100644 tests/controller/config/routing.yml create mode 100644 tests/controller/config/services.yml create mode 100644 tests/controller/controller_test.php create mode 100644 tests/controller/ext/foo/config/routing.yml create mode 100644 tests/controller/ext/foo/config/services.yml create mode 100644 tests/controller/ext/foo/controller.php create mode 100644 tests/controller/includes/controller/foo.php diff --git a/phpBB/.htaccess b/phpBB/.htaccess index 474f9774c2..61bd4249e3 100644 --- a/phpBB/.htaccess +++ b/phpBB/.htaccess @@ -1,12 +1,17 @@ + +RewriteEngine on + # # Uncomment the statement below if you want to make use of # HTTP authentication and it does not already work. # This could be required if you are for example using PHP via Apache CGI. # -# -#RewriteEngine on #RewriteRule .* - [E=HTTP_AUTHORIZATION:%{HTTP:Authorization},L] -# + +RewriteCond %{REQUEST_FILENAME} !-f +RewriteCond %{REQUEST_FILENAME} !-d +RewriteRule ^(.*)$ app.php [QSA,L] + Order Allow,Deny diff --git a/phpBB/app.php b/phpBB/app.php new file mode 100644 index 0000000000..9ff9069104 --- /dev/null +++ b/phpBB/app.php @@ -0,0 +1,29 @@ +session_begin(); +$auth->acl($user->data); +$user->setup('app'); + +$http_kernel = $phpbb_container->get('http_kernel'); +$response = $http_kernel->handle($symfony_request); +$response->send(); +$http_kernel->terminate($symfony_request, $response); diff --git a/phpBB/common.php b/phpBB/common.php index c4237dfcf5..986f0a8839 100644 --- a/phpBB/common.php +++ b/phpBB/common.php @@ -8,9 +8,7 @@ * Minimum Requirement: PHP 5.3.3 */ -use Symfony\Component\Config\FileLocator; -use Symfony\Component\DependencyInjection\ContainerBuilder; -use Symfony\Component\DependencyInjection\Loader\YamlFileLoader; +use Symfony\Component\HttpFoundation\Request; /** */ @@ -106,6 +104,11 @@ $phpbb_class_loader_ext->set_cache($phpbb_container->get('cache.driver')); // set up caching $cache = $phpbb_container->get('cache'); +// Instantiate the Symfony Request object +// This must be done before phpbb_request +// because otherwise globals are disabled +$symfony_request = Request::createFromGlobals(); + // Instantiate some basic classes $phpbb_dispatcher = $phpbb_container->get('dispatcher'); $request = $phpbb_container->get('request'); diff --git a/phpBB/composer.json b/phpBB/composer.json index 380bdc367c..5a03e68f73 100644 --- a/phpBB/composer.json +++ b/phpBB/composer.json @@ -5,6 +5,7 @@ "symfony/dependency-injection": "2.1.*", "symfony/event-dispatcher": "2.1.*", "symfony/http-kernel": "2.1.*", + "symfony/routing": "2.1.*", "symfony/yaml": "2.1.*" }, "require-dev": { diff --git a/phpBB/composer.lock b/phpBB/composer.lock index 69e4a2b4b8..62ece6d505 100644 --- a/phpBB/composer.lock +++ b/phpBB/composer.lock @@ -1,5 +1,5 @@ { - "hash": "407cc89f4bb0e409146c863dee51b0ae", + "hash": "efb4768ba71d7cd2c84baa0610d84067", "packages": [ { "name": "symfony/config", @@ -272,6 +272,64 @@ "description": "Symfony HttpKernel Component", "homepage": "http://symfony.com" }, + { + "name": "symfony/routing", + "version": "v2.1.3", + "target-dir": "Symfony/Component/Routing", + "source": { + "type": "git", + "url": "https://github.com/symfony/Routing", + "reference": "v2.1.3" + }, + "dist": { + "type": "zip", + "url": "https://github.com/symfony/Routing/zipball/v2.1.3", + "reference": "v2.1.3", + "shasum": "" + }, + "require": { + "php": ">=5.3.3" + }, + "require-dev": { + "symfony/config": "2.1.*", + "symfony/yaml": "2.1.*", + "symfony/http-kernel": "2.1.*", + "doctrine/common": ">=2.2,<2.4-dev" + }, + "suggest": { + "symfony/config": "2.1.*", + "symfony/yaml": "2.1.*", + "doctrine/common": ">=2.2,<2.4-dev" + }, + "time": "2012-10-26 02:26:42", + "type": "library", + "extra": { + "branch-alias": { + "dev-master": "2.1-dev" + } + }, + "installation-source": "dist", + "autoload": { + "psr-0": { + "Symfony\\Component\\Routing": "" + } + }, + "license": [ + "MIT" + ], + "authors": [ + { + "name": "Fabien Potencier", + "email": "fabien@symfony.com" + }, + { + "name": "Symfony Community", + "homepage": "http://symfony.com/contributors" + } + ], + "description": "Symfony Routing Component", + "homepage": "http://symfony.com" + }, { "name": "symfony/yaml", "version": "v2.1.3", @@ -331,7 +389,7 @@ }, "dist": { "type": "zip", - "url": "https://github.com/fabpot/Goutte/zipball/f2940f9c7c1f409159f5e9f512e575946c5cff48", + "url": "https://github.com/fabpot/Goutte/archive/f2940f9c7c1f409159f5e9f512e575946c5cff48.zip", "reference": "f2940f9c7c1f409159f5e9f512e575946c5cff48", "shasum": "" }, diff --git a/phpBB/config/routing.yml b/phpBB/config/routing.yml new file mode 100644 index 0000000000..f6f728fa47 --- /dev/null +++ b/phpBB/config/routing.yml @@ -0,0 +1,9 @@ +# Structure: +# +# foo_controller: +# pattern: /foo +# defaults: { _controller: foo_sevice:method } +# +# The above will be accessed via app.php/foo and it will instantiate the +# "foo_service" service and call the "method" method. +# diff --git a/phpBB/config/services.yml b/phpBB/config/services.yml index 20aa0546d5..650869d1e6 100644 --- a/phpBB/config/services.yml +++ b/phpBB/config/services.yml @@ -44,6 +44,27 @@ services: - @cache.driver - %tables.config% + controller.helper: + class: phpbb_controller_helper + arguments: + - @service_container + + controller.resolver: + class: phpbb_controller_resolver + arguments: + - @user + - @service_container + - @ext.finder + + controller.route_collection: + class: phpbb_controller_route_collection + arguments: + - @ext.finder + - @controller.provider + + controller.provider: + class: phpbb_controller_provider + cron.task_collection: class: phpbb_di_service_collection arguments: @@ -92,10 +113,46 @@ services: - %core.root_path% - .%core.php_ext% - @cache.driver +<<<<<<< HEAD +======= + + ext.finder: + class: phpbb_extension_finder + arguments: + - @ext.manager + - %core.root_path% + - @cache.driver + - .%core.php_ext% + - _ext_finder + + http_kernel: + class: Symfony\Component\HttpKernel\HttpKernel + arguments: + - @dispatcher + - @controller.resolver + + kernel_event_subscriber: + class: phpbb_event_kernel_subscriber + arguments: + - @template + - @user + tags: + - { name: kernel.event_subscriber } +>>>>>>> 719171f... [feature/controller] Implement a front controller request: class: phpbb_request + request.context: + class: Symfony\Component\Routing\RequestContext + + router_listener: + class: Symfony\Component\HttpKernel\EventListener\RouterListener + arguments: + - @url_matcher + tags: + - { name: kernel.event_subscriber } + style: class: phpbb_style arguments: @@ -132,5 +189,11 @@ services: template_context: class: phpbb_template_context + url_matcher: + class: Symfony\Component\Routing\Matcher\UrlMatcher + arguments: + - @controller.route_collection + - @request.context + user: class: phpbb_user diff --git a/phpBB/includes/controller/helper.php b/phpBB/includes/controller/helper.php new file mode 100644 index 0000000000..1998ea6733 --- /dev/null +++ b/phpBB/includes/controller/helper.php @@ -0,0 +1,142 @@ +container = $container; + + $this->template = $this->container->get('template'); + $this->phpbb_root_path = $this->container->getParameter('core.root_path'); + $this->php_ext = $this->container->getParameter('core.php_ext'); + } + + /** + * Automate setting up the page and creating the response object. + * + * @param string $handle The template handle to render + * @param string $page_title The title of the page to output + * @param int $status_code The status code to be sent to the page header + * @return Response object containing rendered page + */ + public function render($template_file, $page_title = '', $status_code = 200) + { + if (!function_exists('page_header')) + { + include("{$this->phpbb_root_path}includes/functions.{$this->php_ext}"); + } + + page_header($page_title); + + $this->template->set_filenames(array( + 'body' => $template_file, + )); + + page_footer(true, false, false); + + return new Response($this->template->return_display('body'), $status_code); + } + + /** + * Easily generate a URL + * + * @param array $url_parts Each array element is a 'folder' + * i.e. array('my', 'ext') maps to ./app.php/my/ext + * @param mixed $query The Query string, passed directly into the second + * argument of append_sid() + * @return string A URL that has already been run through append_sid() + */ + public function url(array $url_parts, $query = '') + { + return append_sid($this->phpbb_root_path . $this->url_base . implode('/', $url_parts), $query); + } + + /** + * Set base to prepend to urls generated by url() + * This allows extensions to have a certain 'directory' under which + * all their pages are served, but not have to type it every time + * + * @param array $url_parts Each array element is a 'folder' + * i.e. array('my', 'ext') maps to ./app.php/my/ext + * @return null + */ + public function set_url_base(array $url_parts) + { + $this->url_base = !empty($url_parts) ? implode('/', $url_parts) . '/' : ''; + } + + /** + * Output an error, effectively the same thing as trigger_error + * + * @param string $code The error code (e.g. 404, 500, 503, etc.) + * @param string $message The error message + * @return Response A Reponse instance + */ + public function error($code = 500, $message = '') + { + $this->template->assign_vars(array( + 'MESSAGE_TEXT' => $message, + 'MESSAGE_TITLE' => $this->container->get('user')->lang('INFORMATION'), + )); + + return $this->render('message_body.html', $this->container->get('user')->lang('INFORMATION'), $code); + } +} diff --git a/phpBB/includes/controller/provider.php b/phpBB/includes/controller/provider.php new file mode 100644 index 0000000000..25deedb5d1 --- /dev/null +++ b/phpBB/includes/controller/provider.php @@ -0,0 +1,94 @@ +set_paths($routing_paths); + } + + /** + * Locate paths containing routing files + * This sets an internal property but does not return the paths. + * + * @return The current instance of this object for method chaining + */ + public function get_paths(phpbb_extension_finder $finder) + { + // We hardcode the path to the core config directory + // because the finder cannot find it + $this->set_paths(array_merge(array('config'), array_map('dirname', array_keys($finder + ->directory('config') + ->prefix('routing') + ->suffix('yml') + ->find() + )))); + + return $this; + } + + /** + * Set the $routing_paths property with a given list of paths + * + * @return The current instance of this object for method chaining + */ + public function set_paths(array $paths) + { + $this->routing_paths = $paths; + + return $this; + } + + /** + * Get a list of controllers and return it + * + * @param string $base_path Base path to prepend to file paths + * @return array Array of controllers and their route information + */ + public function find($base_path = '') + { + $routes = new RouteCollection; + foreach ($this->routing_paths as $path) + { + $loader = new YamlFileLoader(new FileLocator($base_path . $path)); + $routes->addCollection($loader->load('routing.yml')); + } + + return $routes; + } +} diff --git a/phpBB/includes/controller/resolver.php b/phpBB/includes/controller/resolver.php new file mode 100644 index 0000000000..bf9ce3330d --- /dev/null +++ b/phpBB/includes/controller/resolver.php @@ -0,0 +1,123 @@ +user = $user; + $this->container = $container; + } + + /** + * Load a controller callable + * + * @param Symfony\Component\HttpFoundation\Request $request Symfony Request object + * @return bool|Callable Callable or false + * @throws RuntimeException + */ + public function getController(Request $request) + { + $controller = $request->attributes->get('_controller'); + + if (!$controller) + { + throw new RuntimeException($this->user->lang['CONTROLLER_NOT_SPECIFIED']); + } + + // Require a method name along with the service name + if (stripos($controller, ':') === false) + { + throw new RuntimeException($this->user->lang['CONTROLLER_METHOD_NOT_SPECIFIED']); + } + + list($service, $method) = explode(':', $controller); + + if (!$this->container->has($service)) + { + throw new RuntimeException($this->user->lang('CONTROLLER_SERVICE_UNDEFINED', $service)); + } + + $controller_object = $this->container->get($service); + + return array($controller_object, $method); + } + + /** + * Dependencies should be specified in the service definition and can be + * then accessed in __construct(). Arguments are sent through the URL path + * and should match the parameters of the method you are using as your + * controller. + * + * @param Symfony\Component\HttpFoundation\Request $request Symfony Request object + * @param string $controller Controller class name + * @return bool False + */ + public function getArguments(Request $request, $controller) + { + // At this point, $controller contains the object and method name + list($object, $method) = $controller; + $mirror = new ReflectionMethod($object, $method); + + $arguments = array(); + $parameters = $mirror->getParameters(); + $attributes = $request->attributes->all(); + foreach ($parameters as $param) + { + if (array_key_exists($param->name, $attributes)) + { + $arguments[] = $attributes[$param->name]; + } + else if ($param->isDefaultValueAvailable()) + { + $arguments[] = $param->getDefaultValue(); + } + else + { + throw new RuntimeException($user->lang('CONTROLLER_ARGUMENT_VALUE_MISSING', $param->getPosition() + 1, get_class($object) . ':' . $method, $param->name)); + } + } + + return $arguments; + } +} diff --git a/phpBB/includes/controller/route_collection.php b/phpBB/includes/controller/route_collection.php new file mode 100644 index 0000000000..e6c7d3b543 --- /dev/null +++ b/phpBB/includes/controller/route_collection.php @@ -0,0 +1,36 @@ +addCollection($provider->get_paths($finder)->find()); + } +} diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php index 3a5b100515..fb05b74cd3 100644 --- a/phpBB/includes/functions.php +++ b/phpBB/includes/functions.php @@ -2335,7 +2335,7 @@ function phpbb_on_page($template, $user, $base_url, $num_items, $per_page, $star function append_sid($url, $params = false, $is_amp = true, $session_id = false) { global $_SID, $_EXTRA_URL, $phpbb_hook; - global $phpbb_dispatcher; + global $phpbb_dispatcher, $phpbb_root_path, $config, $symfony_request; if ($params === '' || (is_array($params) && empty($params))) { @@ -2343,6 +2343,20 @@ function append_sid($url, $params = false, $is_amp = true, $session_id = false) $params = false; } + // Make sure we have a Symfony Request object; tests do not have one + // unless they need it. + if ($symfony_request) + { + // Correct the path when we are accessing it through a controller + // This simply rewrites the value given by $phpbb_root_path to the + // script_path in config. + $path_info = $symfony_request->getPathInfo(); + if (!empty($path_info) && $path_info != '/') + { + $url = $config['script_path'] . '/' . substr($url, strlen($phpbb_root_path)); + } + } + $append_sid_overwrite = false; /** @@ -5039,7 +5053,7 @@ function page_header($page_title = '', $display_online_list = true, $item_id = 0 // Determine board url - we may need it later $board_url = generate_board_url() . '/'; - $web_path = (defined('PHPBB_USE_BOARD_URL_PATH') && PHPBB_USE_BOARD_URL_PATH) ? $board_url : $phpbb_root_path; + $web_path = (defined('PHPBB_USE_BOARD_URL_PATH') && PHPBB_USE_BOARD_URL_PATH) ? $board_url : $config['script_path'] . '/'; // Send a proper content-language to the output $user_lang = $user->lang['USER_LANG']; @@ -5216,8 +5230,12 @@ function page_header($page_title = '', $display_online_list = true, $item_id = 0 /** * Generate page footer +* +* @param bool $run_cron Whether or not to run the cron +* @param bool $display_template Whether or not to display the template +* @param bool $exit_handler Whether or not to run the exit_handler() */ -function page_footer($run_cron = true) +function page_footer($run_cron = true, $display_template = true, $exit_handler = true) { global $db, $config, $template, $user, $auth, $cache, $starttime, $phpbb_root_path, $phpEx; global $request, $phpbb_dispatcher; @@ -5312,10 +5330,17 @@ function page_footer($run_cron = true) } } - $template->display('body'); + if ($display_template) + { + $template->display('body'); + } garbage_collection(); - exit_handler(); + + if ($exit_handler) + { + exit_handler(); + } } /** diff --git a/phpBB/includes/template/template.php b/phpBB/includes/template/template.php index 8a7dc6b2f3..a48e8459d5 100644 --- a/phpBB/includes/template/template.php +++ b/phpBB/includes/template/template.php @@ -224,15 +224,9 @@ class phpbb_template */ public function assign_display($handle, $template_var = '', $return_content = true) { - ob_start(); - $result = $this->display($handle); - $contents = ob_get_clean(); - if ($result === false) - { - return false; - } + $contents = $this->return_display($handle); - if ($return_content) + if ($return_content === true || empty($template_var) || $contents === false) { return $contents; } @@ -242,6 +236,15 @@ class phpbb_template return true; } + public function return_display($handle) + { + ob_start(); + $result = $this->display($handle); + $contents = ob_get_clean(); + + return $result === false ? $result : $contents; + } + /** * Obtains a template renderer for a template identified by specified * handle. The template renderer can display the template later. diff --git a/phpBB/index.php b/phpBB/index.php index 66e1b2114b..845d0f0c02 100644 --- a/phpBB/index.php +++ b/phpBB/index.php @@ -17,48 +17,12 @@ define('IN_PHPBB', true); $phpbb_root_path = (defined('PHPBB_ROOT_PATH')) ? PHPBB_ROOT_PATH : './'; $phpEx = substr(strrchr(__FILE__, '.'), 1); include($phpbb_root_path . 'common.' . $phpEx); +include($phpbb_root_path . 'includes/functions_display.' . $phpEx); // Start session management $user->session_begin(); $auth->acl($user->data); -$user->setup(); - -// Handle the display of extension front pages -if ($ext = $request->variable('ext', '')) -{ - $class = 'phpbb_ext_' . str_replace('/', '_', $ext) . '_controller'; - - if (!$phpbb_extension_manager->available($ext)) - { - send_status_line(404, 'Not Found'); - trigger_error($user->lang('EXTENSION_DOES_NOT_EXIST', $ext)); - } - else if (!$phpbb_extension_manager->enabled($ext)) - { - send_status_line(404, 'Not Found'); - trigger_error($user->lang('EXTENSION_DISABLED', $ext)); - } - else if (!class_exists($class)) - { - send_status_line(404, 'Not Found'); - trigger_error($user->lang('EXTENSION_CONTROLLER_MISSING', $ext)); - } - - $controller = new $class; - - if (!($controller instanceof phpbb_extension_controller_interface)) - { - send_status_line(500, 'Internal Server Error'); - trigger_error($user->lang('EXTENSION_CLASS_WRONG_TYPE', $class)); - } - - $controller->handle(); - exit_handler(); -} - -include($phpbb_root_path . 'includes/functions_display.' . $phpEx); - -$user->add_lang('viewforum'); +$user->setup('viewforum'); display_forums('', $config['load_moderators']); diff --git a/phpBB/language/en/app.php b/phpBB/language/en/app.php new file mode 100644 index 0000000000..db7f3164d0 --- /dev/null +++ b/phpBB/language/en/app.php @@ -0,0 +1,59 @@ + 'Missing value for argument #%1$s: %3$s in class %2$s', + 'CONTROLLER_NOT_SPECIFIED' => 'No controller has been specified.', + 'CONTROLLER_NOT_FOUND' => 'The requested page could not be found.', + 'CONTROLLER_METHOD_NOT_SPECIFIED' => 'No method was specified for the controller.', + 'CONTROLLER_SERVICE_NOT_GIVEN' => 'The controller "%s" must have a service specified in ./config/routing.yml.', + 'CONTROLLER_SERVICE_UNDEFINED' => 'The service for controller "%s" is not defined in ./config/services.yml.', + 'CONTROLLER_RETURN_TYPE_INVALID' => 'The controller object %s must return a Symfony\Component\HttpFoundation\Response object.', + + // Event Listener/Subscriber error messages + 'NO_EVENT_ATTRIBUTE' => 'Service "%1$s" must define the "event" attribute on "kernel.event_listener" tags.', + 'SUBSCRIBER_WRONG_TYPE' => 'Service "%1$s" must implement interface "%2$s".', + + // Core error controller messages + 'PAGE_NOT_FOUND_ERROR' => 'The page you have requested does not exist.', + 'NOT_AUTHORISED_ERROR' => 'You do not have permission to access this page.', + 'NOT_AUTHENTICATED_ERROR' => 'You must log in to access this page.', + 'INTERNAL_SERVER_ERROR_ERROR' => 'An unknown error occured.', +)); diff --git a/phpBB/web.config b/phpBB/web.config index a73c328626..e31a48b991 100644 --- a/phpBB/web.config +++ b/phpBB/web.config @@ -12,6 +12,18 @@ + + + + + + + + + + + + diff --git a/tests/controller/config/routing.yml b/tests/controller/config/routing.yml new file mode 100644 index 0000000000..175b11f130 --- /dev/null +++ b/tests/controller/config/routing.yml @@ -0,0 +1,3 @@ +core_controller: + pattern: /core_foo + defaults: { _controller: core_foo.controller:bar } diff --git a/tests/controller/config/services.yml b/tests/controller/config/services.yml new file mode 100644 index 0000000000..f1bd047489 --- /dev/null +++ b/tests/controller/config/services.yml @@ -0,0 +1,3 @@ +services: + core_foo.controller: + class: phpbb_controller_foo diff --git a/tests/controller/controller_test.php b/tests/controller/controller_test.php new file mode 100644 index 0000000000..38f58c80a0 --- /dev/null +++ b/tests/controller/controller_test.php @@ -0,0 +1,75 @@ +extension_manager = new phpbb_mock_extension_manager( + dirname(__FILE__) . '/', + array( + 'foo' => array( + 'ext_name' => 'foo', + 'ext_active' => '1', + 'ext_path' => 'ext/foo/', + ), + )); + } + + public function test_provider() + { + $provider = new phpbb_controller_provider; + $routes = $provider + ->get_paths($this->extension_manager->get_finder()) + ->find('./tests/controller/'); + + // This will need to be updated if any new routes are defined + $this->assertEquals(2, count($routes)); + } + + public function test_controller_resolver() + { + $container = new ContainerBuilder(); + // For some reason, I cannot get it to load more than one services + // file at a time, even when givein multiple paths + // So instead, I am looping through all of the paths + foreach (array(__DIR__.'/config', __DIR__.'/ext/foo/config') as $path) + { + $loader = new YamlFileLoader($container, new FileLocator($path)); + $loader->load('services.yml'); + } + + // Autoloading classes within the tests folder does not work + // so I'll include them manually + if (!class_exists('phpbb_ext_foo_controller')) + { + include(__DIR__.'/ext/foo/controller.php'); + } + if (!class_exists('phpbb_controller_foo')) + { + include(__DIR__.'/includes/controller/foo.php'); + } + + $resolver = new phpbb_controller_resolver(new phpbb_user, $container); + $symfony_request = new Request(array(), array(), array('_controller' => 'foo.controller:handle')); + + $this->assertEquals($resolver->getController($symfony_request), array(new phpbb_ext_foo_controller, 'handle')); + + $symfony_request = new Request(array(), array(), array('_controller' => 'core_foo.controller:bar')); + + $this->assertEquals($resolver->getController($symfony_request), array(new phpbb_controller_foo, 'bar')); + } +} diff --git a/tests/controller/ext/foo/config/routing.yml b/tests/controller/ext/foo/config/routing.yml new file mode 100644 index 0000000000..4799fec37d --- /dev/null +++ b/tests/controller/ext/foo/config/routing.yml @@ -0,0 +1,3 @@ +controller1: + pattern: /foo + defaults: { _controller: foo.controller:handle } diff --git a/tests/controller/ext/foo/config/services.yml b/tests/controller/ext/foo/config/services.yml new file mode 100644 index 0000000000..ce0e18c610 --- /dev/null +++ b/tests/controller/ext/foo/config/services.yml @@ -0,0 +1,3 @@ +services: + foo.controller: + class: phpbb_ext_foo_controller diff --git a/tests/controller/ext/foo/controller.php b/tests/controller/ext/foo/controller.php new file mode 100644 index 0000000000..72b8560c20 --- /dev/null +++ b/tests/controller/ext/foo/controller.php @@ -0,0 +1,23 @@ + Date: Sat, 20 Oct 2012 17:06:04 -0400 Subject: [PATCH 02/67] [feature/controller] Transfer kernel-related stuff from container PR PHPBB3-10864 --- phpBB/includes/event/kernel_compiler_pass.php | 72 ++++++++++++++ phpBB/includes/event/kernel_subscriber.php | 94 +++++++++++++++++++ 2 files changed, 166 insertions(+) create mode 100644 phpBB/includes/event/kernel_compiler_pass.php create mode 100644 phpBB/includes/event/kernel_subscriber.php diff --git a/phpBB/includes/event/kernel_compiler_pass.php b/phpBB/includes/event/kernel_compiler_pass.php new file mode 100644 index 0000000000..18b6661cd4 --- /dev/null +++ b/phpBB/includes/event/kernel_compiler_pass.php @@ -0,0 +1,72 @@ +getDefinition('dispatcher'); + $user = $container->get('user'); + + foreach ($container->findTaggedServiceIds('kernel.event_listener') as $id => $events) + { + foreach ($events as $event) + { + $priority = isset($event['priority']) ? $event['priority'] : 0; + + if (!isset($event['event'])) + { + throw new InvalidArgumentException($user->lang('NO_EVENT_ATTRIBUTE', $id)); + } + + if (!isset($event['method'])) + { + $event['method'] = 'on'.preg_replace(array( + '/(?<=\b)[a-z]/ie', + '/[^a-z0-9]/i' + ), array('strtoupper("\\0")', ''), $event['event']); + } + + $definition->addMethodCall('addListenerService', array($event['event'], array($id, $event['method']), $priority)); + } + } + + foreach ($container->findTaggedServiceIds('kernel.event_subscriber') as $id => $attributes) + { + // We must assume that the class value has been correctly filled, even if the service is created by a factory + $class = $container->getDefinition($id)->getClass(); + + $refClass = new ReflectionClass($class); + $interface = 'Symfony\Component\EventDispatcher\EventSubscriberInterface'; + if (!$refClass->implementsInterface($interface)) + { + throw new InvalidArgumentException($user->lang('SUBSCRIBER_WRONG_TYPE', $id, $interface)); + } + + $definition->addMethodCall('addSubscriberService', array($id, $class)); + } + } +} diff --git a/phpBB/includes/event/kernel_subscriber.php b/phpBB/includes/event/kernel_subscriber.php new file mode 100644 index 0000000000..9737d9bc23 --- /dev/null +++ b/phpBB/includes/event/kernel_subscriber.php @@ -0,0 +1,94 @@ +template = $template; + $this->user = $user; + } + + /** + * This listener is run when the KernelEvents::TERMINATE event is triggered + * This comes after a Response has been sent to the server; this is + * primarily cleanup stuff. + * + * @param PostResponseEvent $event + * @return null + */ + public function on_kernel_terminate(PostResponseEvent $event) + { + exit_handler(); + } + + /** + * This listener is run when the KernelEvents::EXCEPTION event is triggered + * + * @param GetResponseForExceptionEvent $event + * @return null + */ + public function on_kernel_exception(GetResponseForExceptionEvent $event) + { + page_header($this->user->lang('INFORMATION')); + + $this->template->assign_vars(array( + 'MESSAGE_TITLE' => $this->user->lang('INFORMATION'), + 'MESSAGE_TEXT' => $event->getException()->getMessage(), + )); + + $this->template->set_filenames(array( + 'body' => 'message_body.html', + )); + + page_footer(true, false, false); + + $event->setResponse(new Response($this->template->return_display('body'), 404)); + } + + public static function getSubscribedEvents() + { + return array( + KernelEvents::TERMINATE => 'on_kernel_terminate', + KernelEvents::EXCEPTION => 'on_kernel_exception', + ); + } +} From 5c86a1660d436cf16448ae6c978237833e3e86b7 Mon Sep 17 00:00:00 2001 From: David King Date: Sat, 20 Oct 2012 17:16:14 -0400 Subject: [PATCH 03/67] [feature/controller] Don't allow a kernel listener to be added with no method PHPBB3-10864 --- phpBB/includes/event/kernel_compiler_pass.php | 5 +---- phpBB/language/en/app.php | 1 + 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/phpBB/includes/event/kernel_compiler_pass.php b/phpBB/includes/event/kernel_compiler_pass.php index 18b6661cd4..9a650bc404 100644 --- a/phpBB/includes/event/kernel_compiler_pass.php +++ b/phpBB/includes/event/kernel_compiler_pass.php @@ -44,10 +44,7 @@ class phpbb_event_kernel_compiler_pass implements CompilerPassInterface if (!isset($event['method'])) { - $event['method'] = 'on'.preg_replace(array( - '/(?<=\b)[a-z]/ie', - '/[^a-z0-9]/i' - ), array('strtoupper("\\0")', ''), $event['event']); + throw new InvalidArgumentException($user->lang('NO_METHOD_ATTRIBUTE', $id)); } $definition->addMethodCall('addListenerService', array($event['event'], array($id, $event['method']), $priority)); diff --git a/phpBB/language/en/app.php b/phpBB/language/en/app.php index db7f3164d0..2cbeaa2cba 100644 --- a/phpBB/language/en/app.php +++ b/phpBB/language/en/app.php @@ -49,6 +49,7 @@ $lang = array_merge($lang, array( // Event Listener/Subscriber error messages 'NO_EVENT_ATTRIBUTE' => 'Service "%1$s" must define the "event" attribute on "kernel.event_listener" tags.', + 'NO_METHOD_ATTRIBUTE' => 'Service "%1$s" must define the "method" attribute on "kernel.event_listener" tags.', 'SUBSCRIBER_WRONG_TYPE' => 'Service "%1$s" must implement interface "%2$s".', // Core error controller messages From 97957d679250c642969a09b002f4125889a5f4fa Mon Sep 17 00:00:00 2001 From: David King Date: Sun, 21 Oct 2012 16:37:03 -0400 Subject: [PATCH 04/67] [feature/controller-new] Rename kernel compiler pass class PHPBB3-10864 --- phpBB/common.php | 1 + .../{event/kernel_compiler_pass.php => di/pass/kernel.php} | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) rename phpBB/includes/{event/kernel_compiler_pass.php => di/pass/kernel.php} (96%) diff --git a/phpBB/common.php b/phpBB/common.php index 986f0a8839..b0bc281b45 100644 --- a/phpBB/common.php +++ b/phpBB/common.php @@ -93,6 +93,7 @@ $phpbb_container = phpbb_create_dumped_container_unless_debug( ), array( new phpbb_di_pass_collection_pass(), + new phpbb_di_pass_kernel(), ), $phpbb_root_path, $phpEx diff --git a/phpBB/includes/event/kernel_compiler_pass.php b/phpBB/includes/di/pass/kernel.php similarity index 96% rename from phpBB/includes/event/kernel_compiler_pass.php rename to phpBB/includes/di/pass/kernel.php index 9a650bc404..7b190a6236 100644 --- a/phpBB/includes/event/kernel_compiler_pass.php +++ b/phpBB/includes/di/pass/kernel.php @@ -18,7 +18,7 @@ if (!defined('IN_PHPBB')) use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface; -class phpbb_event_kernel_compiler_pass implements CompilerPassInterface +class phpbb_di_pass_kernel implements CompilerPassInterface { /** * Modify the container before it is passed to the rest of the code From 4e1f17a87dc6fefa30becc594a7f41e7f6293cad Mon Sep 17 00:00:00 2001 From: David King Date: Sun, 11 Nov 2012 14:17:31 -0500 Subject: [PATCH 05/67] [feature/controller] includes/functions.php is included by default PHPBB3-10864 --- phpBB/includes/controller/helper.php | 5 ----- 1 file changed, 5 deletions(-) diff --git a/phpBB/includes/controller/helper.php b/phpBB/includes/controller/helper.php index 1998ea6733..f3127fd1fc 100644 --- a/phpBB/includes/controller/helper.php +++ b/phpBB/includes/controller/helper.php @@ -79,11 +79,6 @@ class phpbb_controller_helper */ public function render($template_file, $page_title = '', $status_code = 200) { - if (!function_exists('page_header')) - { - include("{$this->phpbb_root_path}includes/functions.{$this->php_ext}"); - } - page_header($page_title); $this->template->set_filenames(array( From cba0be96a9d21d6927ab998b9a880c469b64fb24 Mon Sep 17 00:00:00 2001 From: David King Date: Sun, 11 Nov 2012 14:35:59 -0500 Subject: [PATCH 06/67] [feature/controller] Fix 403 Forbidden error --- phpBB/.htaccess | 2 ++ 1 file changed, 2 insertions(+) diff --git a/phpBB/.htaccess b/phpBB/.htaccess index 61bd4249e3..68021177f2 100644 --- a/phpBB/.htaccess +++ b/phpBB/.htaccess @@ -1,3 +1,5 @@ +Options +FollowSymLinks + RewriteEngine on From dd1f8a0e554cc7c7fd975089cfd01acc88c656a6 Mon Sep 17 00:00:00 2001 From: David King Date: Sun, 11 Nov 2012 19:41:45 -0500 Subject: [PATCH 07/67] [feature/controller-new] Call pass to subscribe the Kernel event listener PHPBB3-10864 --- phpBB/download/file.php | 1 + phpBB/install/database_update.php | 1 + 2 files changed, 2 insertions(+) diff --git a/phpBB/download/file.php b/phpBB/download/file.php index 85f5d11504..eed713b891 100644 --- a/phpBB/download/file.php +++ b/phpBB/download/file.php @@ -64,6 +64,7 @@ if (isset($_GET['avatar'])) ), array( new phpbb_di_pass_collection_pass(), + new phpbb_di_pass_kernel(), ), $phpbb_root_path, $phpEx diff --git a/phpBB/install/database_update.php b/phpBB/install/database_update.php index 297802c210..b8f5021dfd 100644 --- a/phpBB/install/database_update.php +++ b/phpBB/install/database_update.php @@ -113,6 +113,7 @@ $phpbb_container = phpbb_create_dumped_container_unless_debug( ), array( new phpbb_di_pass_collection_pass(), + new phpbb_di_pass_kernel(), ), $phpbb_root_path, $phpEx From 6e647cf6e142da3ddf304275803e708501daeb82 Mon Sep 17 00:00:00 2001 From: David King Date: Mon, 12 Nov 2012 15:48:53 -0500 Subject: [PATCH 08/67] [feature/controller] Resolve trivial conflict I missed during rebase PHPBB3-10864 --- phpBB/config/services.yml | 3 --- 1 file changed, 3 deletions(-) diff --git a/phpBB/config/services.yml b/phpBB/config/services.yml index 650869d1e6..54d347debe 100644 --- a/phpBB/config/services.yml +++ b/phpBB/config/services.yml @@ -113,8 +113,6 @@ services: - %core.root_path% - .%core.php_ext% - @cache.driver -<<<<<<< HEAD -======= ext.finder: class: phpbb_extension_finder @@ -138,7 +136,6 @@ services: - @user tags: - { name: kernel.event_subscriber } ->>>>>>> 719171f... [feature/controller] Implement a front controller request: class: phpbb_request From 7687f069611b35f81c6d0fc87e46b9bb0821d616 Mon Sep 17 00:00:00 2001 From: David King Date: Tue, 13 Nov 2012 09:26:31 -0500 Subject: [PATCH 09/67] [feature/controller] Inject dependencies instead of container PHPBB3-10864 --- phpBB/includes/controller/helper.php | 44 ++++++++++++++-------------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/phpBB/includes/controller/helper.php b/phpBB/includes/controller/helper.php index f3127fd1fc..0fc3ba0c67 100644 --- a/phpBB/includes/controller/helper.php +++ b/phpBB/includes/controller/helper.php @@ -15,9 +15,7 @@ if (!defined('IN_PHPBB')) exit; } -use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; -use Symfony\Component\DependencyInjection\ContainerBuilder; /** * Controller helper class, contains methods that do things for controllers @@ -25,12 +23,6 @@ use Symfony\Component\DependencyInjection\ContainerBuilder; */ class phpbb_controller_helper { - /** - * Container - * @var ContainerBuilder - */ - protected $container; - /** * Template object * @var phpbb_template @@ -38,13 +30,19 @@ class phpbb_controller_helper protected $template; /** - * phpBB Root Path - * @var string + * User object + * @var phpbb_user */ - protected $phpbb_root_path; + protected $user; /** - * PHP Extension + * phpBB root path + * @var string + */ + protected $root_path; + + /** + * PHP extension * @var string */ protected $php_ext; @@ -58,15 +56,17 @@ class phpbb_controller_helper /** * Constructor * - * @param ContainerBuilder $container DI Container + * @param phpbb_template $template Template object + * @param phpbb_user $user User object + * @param string $root_path phpBB root path + * @param string $php_ext PHP extension */ - public function __construct(ContainerBuilder $container) + public function __construct(phpbb_template $template, phpbb_user $user, $root_path, $php_ext) { - $this->container = $container; - - $this->template = $this->container->get('template'); - $this->phpbb_root_path = $this->container->getParameter('core.root_path'); - $this->php_ext = $this->container->getParameter('core.php_ext'); + $this->template = $template; + $this->user = $user; + $this->root_path = $root_path; + $this->php_ext = $php_ext; } /** @@ -101,7 +101,7 @@ class phpbb_controller_helper */ public function url(array $url_parts, $query = '') { - return append_sid($this->phpbb_root_path . $this->url_base . implode('/', $url_parts), $query); + return append_sid($this->root_path . $this->url_base . implode('/', $url_parts), $query); } /** @@ -129,9 +129,9 @@ class phpbb_controller_helper { $this->template->assign_vars(array( 'MESSAGE_TEXT' => $message, - 'MESSAGE_TITLE' => $this->container->get('user')->lang('INFORMATION'), + 'MESSAGE_TITLE' => $this->user->lang('INFORMATION'), )); - return $this->render('message_body.html', $this->container->get('user')->lang('INFORMATION'), $code); + return $this->render('message_body.html', $this->user->lang('INFORMATION'), $code); } } From 4b6d538b062a56f55ba221ac8437b4bfc712a475 Mon Sep 17 00:00:00 2001 From: David King Date: Tue, 13 Nov 2012 09:28:56 -0500 Subject: [PATCH 10/67] [feature/controller] Rename kernel pass class properly PHPBB3-10864 --- phpBB/common.php | 2 +- phpBB/download/file.php | 2 +- phpBB/includes/di/pass/{kernel.php => kernel_pass.php} | 2 +- phpBB/install/database_update.php | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) rename phpBB/includes/di/pass/{kernel.php => kernel_pass.php} (96%) diff --git a/phpBB/common.php b/phpBB/common.php index b0bc281b45..a5ffcea8e4 100644 --- a/phpBB/common.php +++ b/phpBB/common.php @@ -93,7 +93,7 @@ $phpbb_container = phpbb_create_dumped_container_unless_debug( ), array( new phpbb_di_pass_collection_pass(), - new phpbb_di_pass_kernel(), + new phpbb_di_pass_kernel_pass(), ), $phpbb_root_path, $phpEx diff --git a/phpBB/download/file.php b/phpBB/download/file.php index eed713b891..73b9e0a9f2 100644 --- a/phpBB/download/file.php +++ b/phpBB/download/file.php @@ -64,7 +64,7 @@ if (isset($_GET['avatar'])) ), array( new phpbb_di_pass_collection_pass(), - new phpbb_di_pass_kernel(), + new phpbb_di_pass_kernel_pass(), ), $phpbb_root_path, $phpEx diff --git a/phpBB/includes/di/pass/kernel.php b/phpBB/includes/di/pass/kernel_pass.php similarity index 96% rename from phpBB/includes/di/pass/kernel.php rename to phpBB/includes/di/pass/kernel_pass.php index 7b190a6236..d186ff2767 100644 --- a/phpBB/includes/di/pass/kernel.php +++ b/phpBB/includes/di/pass/kernel_pass.php @@ -18,7 +18,7 @@ if (!defined('IN_PHPBB')) use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface; -class phpbb_di_pass_kernel implements CompilerPassInterface +class phpbb_di_pass_kernel_pass implements CompilerPassInterface { /** * Modify the container before it is passed to the rest of the code diff --git a/phpBB/install/database_update.php b/phpBB/install/database_update.php index b8f5021dfd..377e38c423 100644 --- a/phpBB/install/database_update.php +++ b/phpBB/install/database_update.php @@ -113,7 +113,7 @@ $phpbb_container = phpbb_create_dumped_container_unless_debug( ), array( new phpbb_di_pass_collection_pass(), - new phpbb_di_pass_kernel(), + new phpbb_di_pass_kernel_pass(), ), $phpbb_root_path, $phpEx From ac29c7e9d982648ed64e0ef73bbebd67567c2d89 Mon Sep 17 00:00:00 2001 From: David King Date: Tue, 13 Nov 2012 09:43:53 -0500 Subject: [PATCH 11/67] [feature/controller] Rework assign_display(), use exceptions instead of return PHPBB3-10864 --- phpBB/includes/template/template.php | 29 +++++++++++++++++++++------- 1 file changed, 22 insertions(+), 7 deletions(-) diff --git a/phpBB/includes/template/template.php b/phpBB/includes/template/template.php index a48e8459d5..a6ae44969b 100644 --- a/phpBB/includes/template/template.php +++ b/phpBB/includes/template/template.php @@ -219,30 +219,45 @@ class phpbb_template * * @param string $handle Handle to operate on * @param string $template_var Template variable to assign compiled handle to - * @param bool $return_content If true return compiled handle, otherwise assign to $template_var - * @return bool|string false on failure, otherwise if $return_content is true return string of the compiled handle, otherwise return true + * @param bool $return_contents If true return compiled handle, otherwise assign to $template_var + * @return bool|string If $return_content is true return string of the compiled handle, otherwise return true + * @throws RuntimeException */ - public function assign_display($handle, $template_var = '', $return_content = true) + public function assign_display($handle, $template_var = '', $return_contents = true) { $contents = $this->return_display($handle); - if ($return_content === true || empty($template_var) || $contents === false) + if (!$template_var) { - return $contents; + throw new RuntimeException($this->user->lang('TEMPLATE_CANNOT_BE_ASSIGNED') } $this->assign_var($template_var, $contents); - return true; + // If !$return_content evaluates to true, true will be returned + // Otherwise, the value of $contents will be returned + return !$return_contents ?: $contents; } + /** + * Return the compiled template code as a string + * + * @param string $handle Handle to operate on + * @return string Compiled template code; can be output directly to page + * @throws RuntimeException + */ public function return_display($handle) { ob_start(); $result = $this->display($handle); $contents = ob_get_clean(); - return $result === false ? $result : $contents; + if ($result === false) + { + throw new RuntimeException($user->lang('TEMPLATE_HANDLE_NOT_FOUND')); + } + + return $contents; } /** From 79bcbd3691a91e213a860dbcd1b11330752bd076 Mon Sep 17 00:00:00 2001 From: David King Date: Tue, 13 Nov 2012 09:48:12 -0500 Subject: [PATCH 12/67] [feature/controller] Add _controller attribute to Request after instantiation PHPBB3-10864 --- tests/controller/controller_test.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/controller/controller_test.php b/tests/controller/controller_test.php index 38f58c80a0..b73019a6a0 100644 --- a/tests/controller/controller_test.php +++ b/tests/controller/controller_test.php @@ -64,11 +64,13 @@ class phpbb_controller_test extends phpbb_test_case } $resolver = new phpbb_controller_resolver(new phpbb_user, $container); - $symfony_request = new Request(array(), array(), array('_controller' => 'foo.controller:handle')); + $symfony_request = new Request(); + $symfony_request->attributes->set('_controller', 'foo.controller:handle'); $this->assertEquals($resolver->getController($symfony_request), array(new phpbb_ext_foo_controller, 'handle')); - $symfony_request = new Request(array(), array(), array('_controller' => 'core_foo.controller:bar')); + $symfony_request = new Request(); + $symfony_request->attributes->set('_controller', 'core_foo.controller:bar'); $this->assertEquals($resolver->getController($symfony_request), array(new phpbb_controller_foo, 'bar')); } From 1c5a82c4110cfbc328281d832bb38387df95c39d Mon Sep 17 00:00:00 2001 From: David King Date: Tue, 13 Nov 2012 09:48:42 -0500 Subject: [PATCH 13/67] [feature/controller] Remove empty __construct() method PHPBB3-10864 --- tests/controller/ext/foo/controller.php | 7 ------- 1 file changed, 7 deletions(-) diff --git a/tests/controller/ext/foo/controller.php b/tests/controller/ext/foo/controller.php index 72b8560c20..cfc5c20622 100644 --- a/tests/controller/ext/foo/controller.php +++ b/tests/controller/ext/foo/controller.php @@ -4,13 +4,6 @@ use Symfony\Component\HttpFoundation\Response; class phpbb_ext_foo_controller { - /** - * Constructor - */ - public function __construct() - { - } - /** * Handle method * From b5255d4ea4f9e5e1d8c2783555891a0d5a63aca2 Mon Sep 17 00:00:00 2001 From: David King Date: Tue, 13 Nov 2012 09:51:23 -0500 Subject: [PATCH 14/67] [feature/controller] Fix syntax error in template code PHPBB3-10864 --- phpBB/includes/template/template.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpBB/includes/template/template.php b/phpBB/includes/template/template.php index a6ae44969b..75bbbe2ef3 100644 --- a/phpBB/includes/template/template.php +++ b/phpBB/includes/template/template.php @@ -229,7 +229,7 @@ class phpbb_template if (!$template_var) { - throw new RuntimeException($this->user->lang('TEMPLATE_CANNOT_BE_ASSIGNED') + throw new RuntimeException($this->user->lang('TEMPLATE_CANNOT_BE_ASSIGNED')); } $this->assign_var($template_var, $contents); From ef46af8298e18c64b662375adb7eaa40342900a8 Mon Sep 17 00:00:00 2001 From: David King Date: Tue, 13 Nov 2012 09:57:51 -0500 Subject: [PATCH 15/67] [feature/controller] Don't attempt to assign if no variable is give PHPBB3-10864 --- phpBB/includes/template/template.php | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/phpBB/includes/template/template.php b/phpBB/includes/template/template.php index 75bbbe2ef3..11d226d705 100644 --- a/phpBB/includes/template/template.php +++ b/phpBB/includes/template/template.php @@ -227,12 +227,15 @@ class phpbb_template { $contents = $this->return_display($handle); - if (!$template_var) + if (!$template_var && !$return_contents) { throw new RuntimeException($this->user->lang('TEMPLATE_CANNOT_BE_ASSIGNED')); } - $this->assign_var($template_var, $contents); + if ($template_var) + { + $this->assign_var($template_var, $contents); + } // If !$return_content evaluates to true, true will be returned // Otherwise, the value of $contents will be returned From 6900456e903a01c3df8d717de711480d347d287c Mon Sep 17 00:00:00 2001 From: David King Date: Tue, 13 Nov 2012 10:41:11 -0500 Subject: [PATCH 16/67] [feature/controller] Revert changes to template class As per IRC, assign_display() effectively does what return_display() was added to do, so no change was needed. PHPBB3-10864 --- phpBB/includes/template/template.php | 1139 +++++++++++++------------- 1 file changed, 559 insertions(+), 580 deletions(-) diff --git a/phpBB/includes/template/template.php b/phpBB/includes/template/template.php index 11d226d705..00fe26b9b1 100644 --- a/phpBB/includes/template/template.php +++ b/phpBB/includes/template/template.php @@ -1,580 +1,559 @@ - $user->img('icon_contact', 'CONTACT', 'full'); -* -* More in-depth... -* yadayada -*/ - -/** -* Base Template class. -* @package phpBB3 -*/ -class phpbb_template -{ - /** - * Template context. - * Stores template data used during template rendering. - * @var phpbb_template_context - */ - private $context; - - /** - * Path of the cache directory for the template - * @var string - */ - public $cachepath = ''; - - /** - * phpBB root path - * @var string - */ - private $phpbb_root_path; - - /** - * PHP file extension - * @var string - */ - private $php_ext; - - /** - * phpBB config instance - * @var phpbb_config - */ - private $config; - - /** - * Current user - * @var phpbb_user - */ - private $user; - - /** - * Template locator - * @var phpbb_template_locator - */ - private $locator; - - /** - * Location of templates directory within style directories - * @var string - */ - public $template_path = 'template/'; - - /** - * Constructor. - * - * @param string $phpbb_root_path phpBB root path - * @param user $user current user - * @param phpbb_template_locator $locator template locator - * @param phpbb_template_context $context template context - */ - public function __construct($phpbb_root_path, $php_ext, $config, $user, phpbb_template_locator $locator, phpbb_template_context $context) - { - $this->phpbb_root_path = $phpbb_root_path; - $this->php_ext = $php_ext; - $this->config = $config; - $this->user = $user; - $this->locator = $locator; - $this->template_path = $this->locator->template_path; - $this->context = $context; - } - - /** - * Sets the template filenames for handles. - * - * @param array $filname_array Should be a hash of handle => filename pairs. - */ - public function set_filenames(array $filename_array) - { - $this->locator->set_filenames($filename_array); - - return true; - } - - /** - * Clears all variables and blocks assigned to this template. - */ - public function destroy() - { - $this->context->clear(); - } - - /** - * Reset/empty complete block - * - * @param string $blockname Name of block to destroy - */ - public function destroy_block_vars($blockname) - { - $this->context->destroy_block_vars($blockname); - } - - /** - * Display a template for provided handle. - * - * The template will be loaded and compiled, if necessary, first. - * - * This function calls hooks. - * - * @param string $handle Handle to display - * @return bool True on success, false on failure - */ - public function display($handle) - { - $result = $this->call_hook($handle, __FUNCTION__); - if ($result !== false) - { - return $result[0]; - } - - return $this->load_and_render($handle); - } - - /** - * Loads a template for $handle, compiling it if necessary, and - * renders the template. - * - * @param string $handle Template handle to render - * @return bool True on success, false on failure - */ - private function load_and_render($handle) - { - $renderer = $this->_tpl_load($handle); - - if ($renderer) - { - $renderer->render($this->context, $this->get_lang()); - return true; - } - else - { - return false; - } - } - - /** - * Calls hook if any is defined. - * - * @param string $handle Template handle being displayed. - * @param string $method Method name of the caller. - */ - private function call_hook($handle, $method) - { - global $phpbb_hook; - - if (!empty($phpbb_hook) && $phpbb_hook->call_hook(array(__CLASS__, $method), $handle, $this)) - { - if ($phpbb_hook->hook_return(array(__CLASS__, $method))) - { - $result = $phpbb_hook->hook_return_result(array(__CLASS__, $method)); - return array($result); - } - } - - return false; - } - - /** - * Obtains language array. - * This is either lang property of $user property, or if - * it is not set an empty array. - * @return array language entries - */ - public function get_lang() - { - if (isset($this->user->lang)) - { - $lang = $this->user->lang; - } - else - { - $lang = array(); - } - return $lang; - } - - /** - * Display the handle and assign the output to a template variable - * or return the compiled result. - * - * @param string $handle Handle to operate on - * @param string $template_var Template variable to assign compiled handle to - * @param bool $return_contents If true return compiled handle, otherwise assign to $template_var - * @return bool|string If $return_content is true return string of the compiled handle, otherwise return true - * @throws RuntimeException - */ - public function assign_display($handle, $template_var = '', $return_contents = true) - { - $contents = $this->return_display($handle); - - if (!$template_var && !$return_contents) - { - throw new RuntimeException($this->user->lang('TEMPLATE_CANNOT_BE_ASSIGNED')); - } - - if ($template_var) - { - $this->assign_var($template_var, $contents); - } - - // If !$return_content evaluates to true, true will be returned - // Otherwise, the value of $contents will be returned - return !$return_contents ?: $contents; - } - - /** - * Return the compiled template code as a string - * - * @param string $handle Handle to operate on - * @return string Compiled template code; can be output directly to page - * @throws RuntimeException - */ - public function return_display($handle) - { - ob_start(); - $result = $this->display($handle); - $contents = ob_get_clean(); - - if ($result === false) - { - throw new RuntimeException($user->lang('TEMPLATE_HANDLE_NOT_FOUND')); - } - - return $contents; - } - - /** - * Obtains a template renderer for a template identified by specified - * handle. The template renderer can display the template later. - * - * Template source will first be compiled into php code. - * If template cache is writable the compiled php code will be stored - * on filesystem and template will not be subsequently recompiled. - * If template cache is not writable template source will be recompiled - * every time it is needed. DEBUG define and load_tplcompile - * configuration setting may be used to force templates to be always - * recompiled. - * - * Returns an object implementing phpbb_template_renderer, or null - * if template loading or compilation failed. Call render() on the - * renderer to display the template. This will result in template - * contents sent to the output stream (unless, of course, output - * buffering is in effect). - * - * @param string $handle Handle of the template to load - * @return phpbb_template_renderer Template renderer object, or null on failure - * @uses phpbb_template_compile is used to compile template source - */ - private function _tpl_load($handle) - { - $output_file = $this->_compiled_file_for_handle($handle); - - $recompile = defined('DEBUG') || - !file_exists($output_file) || - @filesize($output_file) === 0; - - if ($recompile || $this->config['load_tplcompile']) - { - // Set only if a recompile or an mtime check are required. - $source_file = $this->locator->get_source_file_for_handle($handle); - - if (!$recompile && @filemtime($output_file) < @filemtime($source_file)) - { - $recompile = true; - } - } - - // Recompile page if the original template is newer, otherwise load the compiled version - if (!$recompile) - { - return new phpbb_template_renderer_include($output_file, $this); - } - - $compile = new phpbb_template_compile($this->config['tpl_allow_php'], $this->locator, $this->phpbb_root_path); - - if ($compile->compile_file_to_file($source_file, $output_file) !== false) - { - $renderer = new phpbb_template_renderer_include($output_file, $this); - } - else if (($code = $compile->compile_file($source_file)) !== false) - { - $renderer = new phpbb_template_renderer_eval($code, $this); - } - else - { - $renderer = null; - } - - return $renderer; - } - - /** - * Determines compiled file path for handle $handle. - * - * @param string $handle Template handle (i.e. "friendly" template name) - * @return string Compiled file path - */ - private function _compiled_file_for_handle($handle) - { - $source_file = $this->locator->get_filename_for_handle($handle); - $compiled_file = $this->cachepath . str_replace('/', '.', $source_file) . '.' . $this->php_ext; - return $compiled_file; - } - - /** - * Assign key variable pairs from an array - * - * @param array $vararray A hash of variable name => value pairs - */ - public function assign_vars(array $vararray) - { - foreach ($vararray as $key => $val) - { - $this->assign_var($key, $val); - } - } - - /** - * Assign a single scalar value to a single key. - * - * Value can be a string, an integer or a boolean. - * - * @param string $varname Variable name - * @param string $varval Value to assign to variable - */ - public function assign_var($varname, $varval) - { - $this->context->assign_var($varname, $varval); - } - - /** - * Append text to the string value stored in a key. - * - * Text is appended using the string concatenation operator (.). - * - * @param string $varname Variable name - * @param string $varval Value to append to variable - */ - public function append_var($varname, $varval) - { - $this->context->append_var($varname, $varval); - } - - // Docstring is copied from phpbb_template_context method with the same name. - /** - * Assign key variable pairs from an array to a specified block - * @param string $blockname Name of block to assign $vararray to - * @param array $vararray A hash of variable name => value pairs - */ - public function assign_block_vars($blockname, array $vararray) - { - return $this->context->assign_block_vars($blockname, $vararray); - } - - // Docstring is copied from phpbb_template_context method with the same name. - /** - * Change already assigned key variable pair (one-dimensional - single loop entry) - * - * An example of how to use this function: - * {@example alter_block_array.php} - * - * @param string $blockname the blockname, for example 'loop' - * @param array $vararray the var array to insert/add or merge - * @param mixed $key Key to search for - * - * array: KEY => VALUE [the key/value pair to search for within the loop to determine the correct position] - * - * int: Position [the position to change or insert at directly given] - * - * If key is false the position is set to 0 - * If key is true the position is set to the last entry - * - * @param string $mode Mode to execute (valid modes are 'insert' and 'change') - * - * If insert, the vararray is inserted at the given position (position counting from zero). - * If change, the current block gets merged with the vararray (resulting in new key/value pairs be added and existing keys be replaced by the new value). - * - * Since counting begins by zero, inserting at the last position will result in this array: array(vararray, last positioned array) - * and inserting at position 1 will result in this array: array(first positioned array, vararray, following vars) - * - * @return bool false on error, true on success - */ - public function alter_block_array($blockname, array $vararray, $key = false, $mode = 'insert') - { - return $this->context->alter_block_array($blockname, $vararray, $key, $mode); - } - - /** - * Include a separate template. - * - * This function is marked public due to the way the template - * implementation uses it. It is actually an implementation function - * and should not be considered part of template class's public API. - * - * @param string $filename Template filename to include - * @param bool $include True to include the file, false to just load it - * @uses template_compile is used to compile uncached templates - */ - public function _tpl_include($filename, $include = true) - { - $this->locator->set_filenames(array($filename => $filename)); - - if (!$this->load_and_render($filename)) - { - // trigger_error cannot be used here, as the output already started - echo 'template->_tpl_include(): Failed including ' . htmlspecialchars($handle) . "\n"; - } - } - - /** - * Include a PHP file. - * - * If a relative path is passed in $filename, it is considered to be - * relative to board root ($phpbb_root_path). Absolute paths are - * also allowed. - * - * This function is marked public due to the way the template - * implementation uses it. It is actually an implementation function - * and should not be considered part of template class's public API. - * - * @param string $filename Path to PHP file to include - */ - public function _php_include($filename) - { - if (phpbb_is_absolute($filename)) - { - $file = $filename; - } - else - { - $file = $this->phpbb_root_path . $filename; - } - - if (!file_exists($file)) - { - // trigger_error cannot be used here, as the output already started - echo 'template->_php_include(): File ' . htmlspecialchars($file) . " does not exist\n"; - return; - } - include($file); - } - - /** - * Obtains filesystem path for a template file. - * - * The simplest use is specifying a single template file as a string - * in the first argument. This template file should be a basename - * of a template file in the selected style, or its parent styles - * if template inheritance is being utilized. - * - * Note: "selected style" is whatever style the style resource locator - * is configured for. - * - * The return value then will be a path, relative to the current - * directory or absolute, to the template file in the selected style - * or its closest parent. - * - * If the selected style does not have the template file being searched, - * (and if inheritance is involved, none of the parents have it either), - * false will be returned. - * - * Specifying true for $return_default will cause the function to - * return the first path which was checked for existence in the event - * that the template file was not found, instead of false. - * This is the path in the selected style itself, not any of its - * parents. - * - * $files can be given an array of templates instead of a single - * template. When given an array, the function will try to resolve - * each template in the array to a path, and will return the first - * path that exists, or false if none exist. - * - * If $return_full_path is false, then instead of returning a usable - * path (when the template is found) only the template's basename - * will be returned. This can be used to check which of the templates - * specified in $files exists, provided different file names are - * used for different templates. - * - * @param string or array $files List of templates to locate. If there is only - * one template, $files can be a string to make code easier to read. - * @param bool $return_default Determines what to return if template does not - * exist. If true, function will return location where template is - * supposed to be. If false, function will return false. - * @param bool $return_full_path If true, function will return full path - * to template. If false, function will return template file name. - * This parameter can be used to check which one of set of template - * files is available. - * @return string or boolean Source template path if template exists or $return_default is - * true. False if template does not exist and $return_default is false - */ - public function locate($files, $return_default = false, $return_full_path = true) - { - // add template path prefix - $templates = array(); - if (is_string($files)) - { - $templates[] = $this->template_path . $files; - } - else - { - foreach ($files as $file) - { - $templates[] = $this->template_path . $file; - } - } - - // use resource locator to find files - return $this->locator->get_first_file_location($templates, $return_default, $return_full_path); - } - - /** - * Include JS file - * - * @param string $file file name - * @param bool $locate True if file needs to be located - * @param bool $relative True if path is relative to phpBB root directory. Ignored if $locate == true - */ - public function _js_include($file, $locate = false, $relative = false) - { - // Locate file - if ($locate) - { - $located = $this->locator->get_first_file_location(array($file), false, true); - if ($located) - { - $file = $located; - } - } - else if ($relative) - { - $file = $this->phpbb_root_path . $file; - } - - $file .= (strpos($file, '?') === false) ? '?' : '&'; - $file .= 'assets_version=' . $this->config['assets_version']; - - // Add HTML code - $code = ''; - $this->context->append_var('SCRIPTS', $code); - } -} + $user->img('icon_contact', 'CONTACT', 'full'); +* +* More in-depth... +* yadayada +*/ + +/** +* Base Template class. +* @package phpBB3 +*/ +class phpbb_template +{ + /** + * Template context. + * Stores template data used during template rendering. + * @var phpbb_template_context + */ + private $context; + + /** + * Path of the cache directory for the template + * @var string + */ + public $cachepath = ''; + + /** + * phpBB root path + * @var string + */ + private $phpbb_root_path; + + /** + * PHP file extension + * @var string + */ + private $php_ext; + + /** + * phpBB config instance + * @var phpbb_config + */ + private $config; + + /** + * Current user + * @var phpbb_user + */ + private $user; + + /** + * Template locator + * @var phpbb_template_locator + */ + private $locator; + + /** + * Location of templates directory within style directories + * @var string + */ + public $template_path = 'template/'; + + /** + * Constructor. + * + * @param string $phpbb_root_path phpBB root path + * @param user $user current user + * @param phpbb_template_locator $locator template locator + * @param phpbb_template_context $context template context + */ + public function __construct($phpbb_root_path, $php_ext, $config, $user, phpbb_template_locator $locator, phpbb_template_context $context) + { + $this->phpbb_root_path = $phpbb_root_path; + $this->php_ext = $php_ext; + $this->config = $config; + $this->user = $user; + $this->locator = $locator; + $this->template_path = $this->locator->template_path; + $this->context = $context; + } + + /** + * Sets the template filenames for handles. + * + * @param array $filname_array Should be a hash of handle => filename pairs. + */ + public function set_filenames(array $filename_array) + { + $this->locator->set_filenames($filename_array); + + return true; + } + + /** + * Clears all variables and blocks assigned to this template. + */ + public function destroy() + { + $this->context->clear(); + } + + /** + * Reset/empty complete block + * + * @param string $blockname Name of block to destroy + */ + public function destroy_block_vars($blockname) + { + $this->context->destroy_block_vars($blockname); + } + + /** + * Display a template for provided handle. + * + * The template will be loaded and compiled, if necessary, first. + * + * This function calls hooks. + * + * @param string $handle Handle to display + * @return bool True on success, false on failure + */ + public function display($handle) + { + $result = $this->call_hook($handle, __FUNCTION__); + if ($result !== false) + { + return $result[0]; + } + + return $this->load_and_render($handle); + } + + /** + * Loads a template for $handle, compiling it if necessary, and + * renders the template. + * + * @param string $handle Template handle to render + * @return bool True on success, false on failure + */ + private function load_and_render($handle) + { + $renderer = $this->_tpl_load($handle); + + if ($renderer) + { + $renderer->render($this->context, $this->get_lang()); + return true; + } + else + { + return false; + } + } + + /** + * Calls hook if any is defined. + * + * @param string $handle Template handle being displayed. + * @param string $method Method name of the caller. + */ + private function call_hook($handle, $method) + { + global $phpbb_hook; + + if (!empty($phpbb_hook) && $phpbb_hook->call_hook(array(__CLASS__, $method), $handle, $this)) + { + if ($phpbb_hook->hook_return(array(__CLASS__, $method))) + { + $result = $phpbb_hook->hook_return_result(array(__CLASS__, $method)); + return array($result); + } + } + + return false; + } + + /** + * Obtains language array. + * This is either lang property of $user property, or if + * it is not set an empty array. + * @return array language entries + */ + public function get_lang() + { + if (isset($this->user->lang)) + { + $lang = $this->user->lang; + } + else + { + $lang = array(); + } + return $lang; + } + + /** + * Display the handle and assign the output to a template variable + * or return the compiled result. + * + * @param string $handle Handle to operate on + * @param string $template_var Template variable to assign compiled handle to + * @param bool $return_content If true return compiled handle, otherwise assign to $template_var + * @return bool|string false on failure, otherwise if $return_content is true return string of the compiled handle, otherwise return true + */ + public function assign_display($handle, $template_var = '', $return_content = true) + { + ob_start(); + $result = $this->display($handle); + $contents = ob_get_clean(); + if ($result === false) + { + return false; + } + + if ($return_content) + { + return $contents; + } + + $this->assign_var($template_var, $contents); + + return true; + } + + /** + * Obtains a template renderer for a template identified by specified + * handle. The template renderer can display the template later. + * + * Template source will first be compiled into php code. + * If template cache is writable the compiled php code will be stored + * on filesystem and template will not be subsequently recompiled. + * If template cache is not writable template source will be recompiled + * every time it is needed. DEBUG define and load_tplcompile + * configuration setting may be used to force templates to be always + * recompiled. + * + * Returns an object implementing phpbb_template_renderer, or null + * if template loading or compilation failed. Call render() on the + * renderer to display the template. This will result in template + * contents sent to the output stream (unless, of course, output + * buffering is in effect). + * + * @param string $handle Handle of the template to load + * @return phpbb_template_renderer Template renderer object, or null on failure + * @uses phpbb_template_compile is used to compile template source + */ + private function _tpl_load($handle) + { + $output_file = $this->_compiled_file_for_handle($handle); + + $recompile = defined('DEBUG') || + !file_exists($output_file) || + @filesize($output_file) === 0; + + if ($recompile || $this->config['load_tplcompile']) + { + // Set only if a recompile or an mtime check are required. + $source_file = $this->locator->get_source_file_for_handle($handle); + + if (!$recompile && @filemtime($output_file) < @filemtime($source_file)) + { + $recompile = true; + } + } + + // Recompile page if the original template is newer, otherwise load the compiled version + if (!$recompile) + { + return new phpbb_template_renderer_include($output_file, $this); + } + + $compile = new phpbb_template_compile($this->config['tpl_allow_php'], $this->locator, $this->phpbb_root_path); + + if ($compile->compile_file_to_file($source_file, $output_file) !== false) + { + $renderer = new phpbb_template_renderer_include($output_file, $this); + } + else if (($code = $compile->compile_file($source_file)) !== false) + { + $renderer = new phpbb_template_renderer_eval($code, $this); + } + else + { + $renderer = null; + } + + return $renderer; + } + + /** + * Determines compiled file path for handle $handle. + * + * @param string $handle Template handle (i.e. "friendly" template name) + * @return string Compiled file path + */ + private function _compiled_file_for_handle($handle) + { + $source_file = $this->locator->get_filename_for_handle($handle); + $compiled_file = $this->cachepath . str_replace('/', '.', $source_file) . '.' . $this->php_ext; + return $compiled_file; + } + + /** + * Assign key variable pairs from an array + * + * @param array $vararray A hash of variable name => value pairs + */ + public function assign_vars(array $vararray) + { + foreach ($vararray as $key => $val) + { + $this->assign_var($key, $val); + } + } + + /** + * Assign a single scalar value to a single key. + * + * Value can be a string, an integer or a boolean. + * + * @param string $varname Variable name + * @param string $varval Value to assign to variable + */ + public function assign_var($varname, $varval) + { + $this->context->assign_var($varname, $varval); + } + + /** + * Append text to the string value stored in a key. + * + * Text is appended using the string concatenation operator (.). + * + * @param string $varname Variable name + * @param string $varval Value to append to variable + */ + public function append_var($varname, $varval) + { + $this->context->append_var($varname, $varval); + } + + // Docstring is copied from phpbb_template_context method with the same name. + /** + * Assign key variable pairs from an array to a specified block + * @param string $blockname Name of block to assign $vararray to + * @param array $vararray A hash of variable name => value pairs + */ + public function assign_block_vars($blockname, array $vararray) + { + return $this->context->assign_block_vars($blockname, $vararray); + } + + // Docstring is copied from phpbb_template_context method with the same name. + /** + * Change already assigned key variable pair (one-dimensional - single loop entry) + * + * An example of how to use this function: + * {@example alter_block_array.php} + * + * @param string $blockname the blockname, for example 'loop' + * @param array $vararray the var array to insert/add or merge + * @param mixed $key Key to search for + * + * array: KEY => VALUE [the key/value pair to search for within the loop to determine the correct position] + * + * int: Position [the position to change or insert at directly given] + * + * If key is false the position is set to 0 + * If key is true the position is set to the last entry + * + * @param string $mode Mode to execute (valid modes are 'insert' and 'change') + * + * If insert, the vararray is inserted at the given position (position counting from zero). + * If change, the current block gets merged with the vararray (resulting in new key/value pairs be added and existing keys be replaced by the new value). + * + * Since counting begins by zero, inserting at the last position will result in this array: array(vararray, last positioned array) + * and inserting at position 1 will result in this array: array(first positioned array, vararray, following vars) + * + * @return bool false on error, true on success + */ + public function alter_block_array($blockname, array $vararray, $key = false, $mode = 'insert') + { + return $this->context->alter_block_array($blockname, $vararray, $key, $mode); + } + + /** + * Include a separate template. + * + * This function is marked public due to the way the template + * implementation uses it. It is actually an implementation function + * and should not be considered part of template class's public API. + * + * @param string $filename Template filename to include + * @param bool $include True to include the file, false to just load it + * @uses template_compile is used to compile uncached templates + */ + public function _tpl_include($filename, $include = true) + { + $this->locator->set_filenames(array($filename => $filename)); + + if (!$this->load_and_render($filename)) + { + // trigger_error cannot be used here, as the output already started + echo 'template->_tpl_include(): Failed including ' . htmlspecialchars($handle) . "\n"; + } + } + + /** + * Include a PHP file. + * + * If a relative path is passed in $filename, it is considered to be + * relative to board root ($phpbb_root_path). Absolute paths are + * also allowed. + * + * This function is marked public due to the way the template + * implementation uses it. It is actually an implementation function + * and should not be considered part of template class's public API. + * + * @param string $filename Path to PHP file to include + */ + public function _php_include($filename) + { + if (phpbb_is_absolute($filename)) + { + $file = $filename; + } + else + { + $file = $this->phpbb_root_path . $filename; + } + + if (!file_exists($file)) + { + // trigger_error cannot be used here, as the output already started + echo 'template->_php_include(): File ' . htmlspecialchars($file) . " does not exist\n"; + return; + } + include($file); + } + + /** + * Obtains filesystem path for a template file. + * + * The simplest use is specifying a single template file as a string + * in the first argument. This template file should be a basename + * of a template file in the selected style, or its parent styles + * if template inheritance is being utilized. + * + * Note: "selected style" is whatever style the style resource locator + * is configured for. + * + * The return value then will be a path, relative to the current + * directory or absolute, to the template file in the selected style + * or its closest parent. + * + * If the selected style does not have the template file being searched, + * (and if inheritance is involved, none of the parents have it either), + * false will be returned. + * + * Specifying true for $return_default will cause the function to + * return the first path which was checked for existence in the event + * that the template file was not found, instead of false. + * This is the path in the selected style itself, not any of its + * parents. + * + * $files can be given an array of templates instead of a single + * template. When given an array, the function will try to resolve + * each template in the array to a path, and will return the first + * path that exists, or false if none exist. + * + * If $return_full_path is false, then instead of returning a usable + * path (when the template is found) only the template's basename + * will be returned. This can be used to check which of the templates + * specified in $files exists, provided different file names are + * used for different templates. + * + * @param string or array $files List of templates to locate. If there is only + * one template, $files can be a string to make code easier to read. + * @param bool $return_default Determines what to return if template does not + * exist. If true, function will return location where template is + * supposed to be. If false, function will return false. + * @param bool $return_full_path If true, function will return full path + * to template. If false, function will return template file name. + * This parameter can be used to check which one of set of template + * files is available. + * @return string or boolean Source template path if template exists or $return_default is + * true. False if template does not exist and $return_default is false + */ + public function locate($files, $return_default = false, $return_full_path = true) + { + // add template path prefix + $templates = array(); + if (is_string($files)) + { + $templates[] = $this->template_path . $files; + } + else + { + foreach ($files as $file) + { + $templates[] = $this->template_path . $file; + } + } + + // use resource locator to find files + return $this->locator->get_first_file_location($templates, $return_default, $return_full_path); + } + + /** + * Include JS file + * + * @param string $file file name + * @param bool $locate True if file needs to be located + * @param bool $relative True if path is relative to phpBB root directory. Ignored if $locate == true + */ + public function _js_include($file, $locate = false, $relative = false) + { + // Locate file + if ($locate) + { + $located = $this->locator->get_first_file_location(array($file), false, true); + if ($located) + { + $file = $located; + } + } + else if ($relative) + { + $file = $this->phpbb_root_path . $file; + } + + $file .= (strpos($file, '?') === false) ? '?' : '&'; + $file .= 'assets_version=' . $this->config['assets_version']; + + // Add HTML code + $code = ''; + $this->context->append_var('SCRIPTS', $code); + } +} From 2e594504596eaf6ab758240a1cd012dfea79fe77 Mon Sep 17 00:00:00 2001 From: David King Date: Tue, 13 Nov 2012 10:42:20 -0500 Subject: [PATCH 17/67] [feature/controller] Use assign_display() instead of return_display() The latter was deemed unnecessary at the moment. PHPBB3-10864 --- phpBB/includes/controller/helper.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpBB/includes/controller/helper.php b/phpBB/includes/controller/helper.php index 0fc3ba0c67..8bb4427382 100644 --- a/phpBB/includes/controller/helper.php +++ b/phpBB/includes/controller/helper.php @@ -87,7 +87,7 @@ class phpbb_controller_helper page_footer(true, false, false); - return new Response($this->template->return_display('body'), $status_code); + return new Response($this->template->assign_display('body'), $status_code); } /** From c6a5699325171ca12c540772ed4e6c0c55318ec5 Mon Sep 17 00:00:00 2001 From: David King Date: Tue, 13 Nov 2012 10:46:46 -0500 Subject: [PATCH 18/67] [feature/controller-new] Fix line endings PHPBB3-10864 --- phpBB/includes/template/template.php | 1118 +++++++++++++------------- 1 file changed, 559 insertions(+), 559 deletions(-) diff --git a/phpBB/includes/template/template.php b/phpBB/includes/template/template.php index 00fe26b9b1..8a7dc6b2f3 100644 --- a/phpBB/includes/template/template.php +++ b/phpBB/includes/template/template.php @@ -1,559 +1,559 @@ - $user->img('icon_contact', 'CONTACT', 'full'); -* -* More in-depth... -* yadayada -*/ - -/** -* Base Template class. -* @package phpBB3 -*/ -class phpbb_template -{ - /** - * Template context. - * Stores template data used during template rendering. - * @var phpbb_template_context - */ - private $context; - - /** - * Path of the cache directory for the template - * @var string - */ - public $cachepath = ''; - - /** - * phpBB root path - * @var string - */ - private $phpbb_root_path; - - /** - * PHP file extension - * @var string - */ - private $php_ext; - - /** - * phpBB config instance - * @var phpbb_config - */ - private $config; - - /** - * Current user - * @var phpbb_user - */ - private $user; - - /** - * Template locator - * @var phpbb_template_locator - */ - private $locator; - - /** - * Location of templates directory within style directories - * @var string - */ - public $template_path = 'template/'; - - /** - * Constructor. - * - * @param string $phpbb_root_path phpBB root path - * @param user $user current user - * @param phpbb_template_locator $locator template locator - * @param phpbb_template_context $context template context - */ - public function __construct($phpbb_root_path, $php_ext, $config, $user, phpbb_template_locator $locator, phpbb_template_context $context) - { - $this->phpbb_root_path = $phpbb_root_path; - $this->php_ext = $php_ext; - $this->config = $config; - $this->user = $user; - $this->locator = $locator; - $this->template_path = $this->locator->template_path; - $this->context = $context; - } - - /** - * Sets the template filenames for handles. - * - * @param array $filname_array Should be a hash of handle => filename pairs. - */ - public function set_filenames(array $filename_array) - { - $this->locator->set_filenames($filename_array); - - return true; - } - - /** - * Clears all variables and blocks assigned to this template. - */ - public function destroy() - { - $this->context->clear(); - } - - /** - * Reset/empty complete block - * - * @param string $blockname Name of block to destroy - */ - public function destroy_block_vars($blockname) - { - $this->context->destroy_block_vars($blockname); - } - - /** - * Display a template for provided handle. - * - * The template will be loaded and compiled, if necessary, first. - * - * This function calls hooks. - * - * @param string $handle Handle to display - * @return bool True on success, false on failure - */ - public function display($handle) - { - $result = $this->call_hook($handle, __FUNCTION__); - if ($result !== false) - { - return $result[0]; - } - - return $this->load_and_render($handle); - } - - /** - * Loads a template for $handle, compiling it if necessary, and - * renders the template. - * - * @param string $handle Template handle to render - * @return bool True on success, false on failure - */ - private function load_and_render($handle) - { - $renderer = $this->_tpl_load($handle); - - if ($renderer) - { - $renderer->render($this->context, $this->get_lang()); - return true; - } - else - { - return false; - } - } - - /** - * Calls hook if any is defined. - * - * @param string $handle Template handle being displayed. - * @param string $method Method name of the caller. - */ - private function call_hook($handle, $method) - { - global $phpbb_hook; - - if (!empty($phpbb_hook) && $phpbb_hook->call_hook(array(__CLASS__, $method), $handle, $this)) - { - if ($phpbb_hook->hook_return(array(__CLASS__, $method))) - { - $result = $phpbb_hook->hook_return_result(array(__CLASS__, $method)); - return array($result); - } - } - - return false; - } - - /** - * Obtains language array. - * This is either lang property of $user property, or if - * it is not set an empty array. - * @return array language entries - */ - public function get_lang() - { - if (isset($this->user->lang)) - { - $lang = $this->user->lang; - } - else - { - $lang = array(); - } - return $lang; - } - - /** - * Display the handle and assign the output to a template variable - * or return the compiled result. - * - * @param string $handle Handle to operate on - * @param string $template_var Template variable to assign compiled handle to - * @param bool $return_content If true return compiled handle, otherwise assign to $template_var - * @return bool|string false on failure, otherwise if $return_content is true return string of the compiled handle, otherwise return true - */ - public function assign_display($handle, $template_var = '', $return_content = true) - { - ob_start(); - $result = $this->display($handle); - $contents = ob_get_clean(); - if ($result === false) - { - return false; - } - - if ($return_content) - { - return $contents; - } - - $this->assign_var($template_var, $contents); - - return true; - } - - /** - * Obtains a template renderer for a template identified by specified - * handle. The template renderer can display the template later. - * - * Template source will first be compiled into php code. - * If template cache is writable the compiled php code will be stored - * on filesystem and template will not be subsequently recompiled. - * If template cache is not writable template source will be recompiled - * every time it is needed. DEBUG define and load_tplcompile - * configuration setting may be used to force templates to be always - * recompiled. - * - * Returns an object implementing phpbb_template_renderer, or null - * if template loading or compilation failed. Call render() on the - * renderer to display the template. This will result in template - * contents sent to the output stream (unless, of course, output - * buffering is in effect). - * - * @param string $handle Handle of the template to load - * @return phpbb_template_renderer Template renderer object, or null on failure - * @uses phpbb_template_compile is used to compile template source - */ - private function _tpl_load($handle) - { - $output_file = $this->_compiled_file_for_handle($handle); - - $recompile = defined('DEBUG') || - !file_exists($output_file) || - @filesize($output_file) === 0; - - if ($recompile || $this->config['load_tplcompile']) - { - // Set only if a recompile or an mtime check are required. - $source_file = $this->locator->get_source_file_for_handle($handle); - - if (!$recompile && @filemtime($output_file) < @filemtime($source_file)) - { - $recompile = true; - } - } - - // Recompile page if the original template is newer, otherwise load the compiled version - if (!$recompile) - { - return new phpbb_template_renderer_include($output_file, $this); - } - - $compile = new phpbb_template_compile($this->config['tpl_allow_php'], $this->locator, $this->phpbb_root_path); - - if ($compile->compile_file_to_file($source_file, $output_file) !== false) - { - $renderer = new phpbb_template_renderer_include($output_file, $this); - } - else if (($code = $compile->compile_file($source_file)) !== false) - { - $renderer = new phpbb_template_renderer_eval($code, $this); - } - else - { - $renderer = null; - } - - return $renderer; - } - - /** - * Determines compiled file path for handle $handle. - * - * @param string $handle Template handle (i.e. "friendly" template name) - * @return string Compiled file path - */ - private function _compiled_file_for_handle($handle) - { - $source_file = $this->locator->get_filename_for_handle($handle); - $compiled_file = $this->cachepath . str_replace('/', '.', $source_file) . '.' . $this->php_ext; - return $compiled_file; - } - - /** - * Assign key variable pairs from an array - * - * @param array $vararray A hash of variable name => value pairs - */ - public function assign_vars(array $vararray) - { - foreach ($vararray as $key => $val) - { - $this->assign_var($key, $val); - } - } - - /** - * Assign a single scalar value to a single key. - * - * Value can be a string, an integer or a boolean. - * - * @param string $varname Variable name - * @param string $varval Value to assign to variable - */ - public function assign_var($varname, $varval) - { - $this->context->assign_var($varname, $varval); - } - - /** - * Append text to the string value stored in a key. - * - * Text is appended using the string concatenation operator (.). - * - * @param string $varname Variable name - * @param string $varval Value to append to variable - */ - public function append_var($varname, $varval) - { - $this->context->append_var($varname, $varval); - } - - // Docstring is copied from phpbb_template_context method with the same name. - /** - * Assign key variable pairs from an array to a specified block - * @param string $blockname Name of block to assign $vararray to - * @param array $vararray A hash of variable name => value pairs - */ - public function assign_block_vars($blockname, array $vararray) - { - return $this->context->assign_block_vars($blockname, $vararray); - } - - // Docstring is copied from phpbb_template_context method with the same name. - /** - * Change already assigned key variable pair (one-dimensional - single loop entry) - * - * An example of how to use this function: - * {@example alter_block_array.php} - * - * @param string $blockname the blockname, for example 'loop' - * @param array $vararray the var array to insert/add or merge - * @param mixed $key Key to search for - * - * array: KEY => VALUE [the key/value pair to search for within the loop to determine the correct position] - * - * int: Position [the position to change or insert at directly given] - * - * If key is false the position is set to 0 - * If key is true the position is set to the last entry - * - * @param string $mode Mode to execute (valid modes are 'insert' and 'change') - * - * If insert, the vararray is inserted at the given position (position counting from zero). - * If change, the current block gets merged with the vararray (resulting in new key/value pairs be added and existing keys be replaced by the new value). - * - * Since counting begins by zero, inserting at the last position will result in this array: array(vararray, last positioned array) - * and inserting at position 1 will result in this array: array(first positioned array, vararray, following vars) - * - * @return bool false on error, true on success - */ - public function alter_block_array($blockname, array $vararray, $key = false, $mode = 'insert') - { - return $this->context->alter_block_array($blockname, $vararray, $key, $mode); - } - - /** - * Include a separate template. - * - * This function is marked public due to the way the template - * implementation uses it. It is actually an implementation function - * and should not be considered part of template class's public API. - * - * @param string $filename Template filename to include - * @param bool $include True to include the file, false to just load it - * @uses template_compile is used to compile uncached templates - */ - public function _tpl_include($filename, $include = true) - { - $this->locator->set_filenames(array($filename => $filename)); - - if (!$this->load_and_render($filename)) - { - // trigger_error cannot be used here, as the output already started - echo 'template->_tpl_include(): Failed including ' . htmlspecialchars($handle) . "\n"; - } - } - - /** - * Include a PHP file. - * - * If a relative path is passed in $filename, it is considered to be - * relative to board root ($phpbb_root_path). Absolute paths are - * also allowed. - * - * This function is marked public due to the way the template - * implementation uses it. It is actually an implementation function - * and should not be considered part of template class's public API. - * - * @param string $filename Path to PHP file to include - */ - public function _php_include($filename) - { - if (phpbb_is_absolute($filename)) - { - $file = $filename; - } - else - { - $file = $this->phpbb_root_path . $filename; - } - - if (!file_exists($file)) - { - // trigger_error cannot be used here, as the output already started - echo 'template->_php_include(): File ' . htmlspecialchars($file) . " does not exist\n"; - return; - } - include($file); - } - - /** - * Obtains filesystem path for a template file. - * - * The simplest use is specifying a single template file as a string - * in the first argument. This template file should be a basename - * of a template file in the selected style, or its parent styles - * if template inheritance is being utilized. - * - * Note: "selected style" is whatever style the style resource locator - * is configured for. - * - * The return value then will be a path, relative to the current - * directory or absolute, to the template file in the selected style - * or its closest parent. - * - * If the selected style does not have the template file being searched, - * (and if inheritance is involved, none of the parents have it either), - * false will be returned. - * - * Specifying true for $return_default will cause the function to - * return the first path which was checked for existence in the event - * that the template file was not found, instead of false. - * This is the path in the selected style itself, not any of its - * parents. - * - * $files can be given an array of templates instead of a single - * template. When given an array, the function will try to resolve - * each template in the array to a path, and will return the first - * path that exists, or false if none exist. - * - * If $return_full_path is false, then instead of returning a usable - * path (when the template is found) only the template's basename - * will be returned. This can be used to check which of the templates - * specified in $files exists, provided different file names are - * used for different templates. - * - * @param string or array $files List of templates to locate. If there is only - * one template, $files can be a string to make code easier to read. - * @param bool $return_default Determines what to return if template does not - * exist. If true, function will return location where template is - * supposed to be. If false, function will return false. - * @param bool $return_full_path If true, function will return full path - * to template. If false, function will return template file name. - * This parameter can be used to check which one of set of template - * files is available. - * @return string or boolean Source template path if template exists or $return_default is - * true. False if template does not exist and $return_default is false - */ - public function locate($files, $return_default = false, $return_full_path = true) - { - // add template path prefix - $templates = array(); - if (is_string($files)) - { - $templates[] = $this->template_path . $files; - } - else - { - foreach ($files as $file) - { - $templates[] = $this->template_path . $file; - } - } - - // use resource locator to find files - return $this->locator->get_first_file_location($templates, $return_default, $return_full_path); - } - - /** - * Include JS file - * - * @param string $file file name - * @param bool $locate True if file needs to be located - * @param bool $relative True if path is relative to phpBB root directory. Ignored if $locate == true - */ - public function _js_include($file, $locate = false, $relative = false) - { - // Locate file - if ($locate) - { - $located = $this->locator->get_first_file_location(array($file), false, true); - if ($located) - { - $file = $located; - } - } - else if ($relative) - { - $file = $this->phpbb_root_path . $file; - } - - $file .= (strpos($file, '?') === false) ? '?' : '&'; - $file .= 'assets_version=' . $this->config['assets_version']; - - // Add HTML code - $code = ''; - $this->context->append_var('SCRIPTS', $code); - } -} + $user->img('icon_contact', 'CONTACT', 'full'); +* +* More in-depth... +* yadayada +*/ + +/** +* Base Template class. +* @package phpBB3 +*/ +class phpbb_template +{ + /** + * Template context. + * Stores template data used during template rendering. + * @var phpbb_template_context + */ + private $context; + + /** + * Path of the cache directory for the template + * @var string + */ + public $cachepath = ''; + + /** + * phpBB root path + * @var string + */ + private $phpbb_root_path; + + /** + * PHP file extension + * @var string + */ + private $php_ext; + + /** + * phpBB config instance + * @var phpbb_config + */ + private $config; + + /** + * Current user + * @var phpbb_user + */ + private $user; + + /** + * Template locator + * @var phpbb_template_locator + */ + private $locator; + + /** + * Location of templates directory within style directories + * @var string + */ + public $template_path = 'template/'; + + /** + * Constructor. + * + * @param string $phpbb_root_path phpBB root path + * @param user $user current user + * @param phpbb_template_locator $locator template locator + * @param phpbb_template_context $context template context + */ + public function __construct($phpbb_root_path, $php_ext, $config, $user, phpbb_template_locator $locator, phpbb_template_context $context) + { + $this->phpbb_root_path = $phpbb_root_path; + $this->php_ext = $php_ext; + $this->config = $config; + $this->user = $user; + $this->locator = $locator; + $this->template_path = $this->locator->template_path; + $this->context = $context; + } + + /** + * Sets the template filenames for handles. + * + * @param array $filname_array Should be a hash of handle => filename pairs. + */ + public function set_filenames(array $filename_array) + { + $this->locator->set_filenames($filename_array); + + return true; + } + + /** + * Clears all variables and blocks assigned to this template. + */ + public function destroy() + { + $this->context->clear(); + } + + /** + * Reset/empty complete block + * + * @param string $blockname Name of block to destroy + */ + public function destroy_block_vars($blockname) + { + $this->context->destroy_block_vars($blockname); + } + + /** + * Display a template for provided handle. + * + * The template will be loaded and compiled, if necessary, first. + * + * This function calls hooks. + * + * @param string $handle Handle to display + * @return bool True on success, false on failure + */ + public function display($handle) + { + $result = $this->call_hook($handle, __FUNCTION__); + if ($result !== false) + { + return $result[0]; + } + + return $this->load_and_render($handle); + } + + /** + * Loads a template for $handle, compiling it if necessary, and + * renders the template. + * + * @param string $handle Template handle to render + * @return bool True on success, false on failure + */ + private function load_and_render($handle) + { + $renderer = $this->_tpl_load($handle); + + if ($renderer) + { + $renderer->render($this->context, $this->get_lang()); + return true; + } + else + { + return false; + } + } + + /** + * Calls hook if any is defined. + * + * @param string $handle Template handle being displayed. + * @param string $method Method name of the caller. + */ + private function call_hook($handle, $method) + { + global $phpbb_hook; + + if (!empty($phpbb_hook) && $phpbb_hook->call_hook(array(__CLASS__, $method), $handle, $this)) + { + if ($phpbb_hook->hook_return(array(__CLASS__, $method))) + { + $result = $phpbb_hook->hook_return_result(array(__CLASS__, $method)); + return array($result); + } + } + + return false; + } + + /** + * Obtains language array. + * This is either lang property of $user property, or if + * it is not set an empty array. + * @return array language entries + */ + public function get_lang() + { + if (isset($this->user->lang)) + { + $lang = $this->user->lang; + } + else + { + $lang = array(); + } + return $lang; + } + + /** + * Display the handle and assign the output to a template variable + * or return the compiled result. + * + * @param string $handle Handle to operate on + * @param string $template_var Template variable to assign compiled handle to + * @param bool $return_content If true return compiled handle, otherwise assign to $template_var + * @return bool|string false on failure, otherwise if $return_content is true return string of the compiled handle, otherwise return true + */ + public function assign_display($handle, $template_var = '', $return_content = true) + { + ob_start(); + $result = $this->display($handle); + $contents = ob_get_clean(); + if ($result === false) + { + return false; + } + + if ($return_content) + { + return $contents; + } + + $this->assign_var($template_var, $contents); + + return true; + } + + /** + * Obtains a template renderer for a template identified by specified + * handle. The template renderer can display the template later. + * + * Template source will first be compiled into php code. + * If template cache is writable the compiled php code will be stored + * on filesystem and template will not be subsequently recompiled. + * If template cache is not writable template source will be recompiled + * every time it is needed. DEBUG define and load_tplcompile + * configuration setting may be used to force templates to be always + * recompiled. + * + * Returns an object implementing phpbb_template_renderer, or null + * if template loading or compilation failed. Call render() on the + * renderer to display the template. This will result in template + * contents sent to the output stream (unless, of course, output + * buffering is in effect). + * + * @param string $handle Handle of the template to load + * @return phpbb_template_renderer Template renderer object, or null on failure + * @uses phpbb_template_compile is used to compile template source + */ + private function _tpl_load($handle) + { + $output_file = $this->_compiled_file_for_handle($handle); + + $recompile = defined('DEBUG') || + !file_exists($output_file) || + @filesize($output_file) === 0; + + if ($recompile || $this->config['load_tplcompile']) + { + // Set only if a recompile or an mtime check are required. + $source_file = $this->locator->get_source_file_for_handle($handle); + + if (!$recompile && @filemtime($output_file) < @filemtime($source_file)) + { + $recompile = true; + } + } + + // Recompile page if the original template is newer, otherwise load the compiled version + if (!$recompile) + { + return new phpbb_template_renderer_include($output_file, $this); + } + + $compile = new phpbb_template_compile($this->config['tpl_allow_php'], $this->locator, $this->phpbb_root_path); + + if ($compile->compile_file_to_file($source_file, $output_file) !== false) + { + $renderer = new phpbb_template_renderer_include($output_file, $this); + } + else if (($code = $compile->compile_file($source_file)) !== false) + { + $renderer = new phpbb_template_renderer_eval($code, $this); + } + else + { + $renderer = null; + } + + return $renderer; + } + + /** + * Determines compiled file path for handle $handle. + * + * @param string $handle Template handle (i.e. "friendly" template name) + * @return string Compiled file path + */ + private function _compiled_file_for_handle($handle) + { + $source_file = $this->locator->get_filename_for_handle($handle); + $compiled_file = $this->cachepath . str_replace('/', '.', $source_file) . '.' . $this->php_ext; + return $compiled_file; + } + + /** + * Assign key variable pairs from an array + * + * @param array $vararray A hash of variable name => value pairs + */ + public function assign_vars(array $vararray) + { + foreach ($vararray as $key => $val) + { + $this->assign_var($key, $val); + } + } + + /** + * Assign a single scalar value to a single key. + * + * Value can be a string, an integer or a boolean. + * + * @param string $varname Variable name + * @param string $varval Value to assign to variable + */ + public function assign_var($varname, $varval) + { + $this->context->assign_var($varname, $varval); + } + + /** + * Append text to the string value stored in a key. + * + * Text is appended using the string concatenation operator (.). + * + * @param string $varname Variable name + * @param string $varval Value to append to variable + */ + public function append_var($varname, $varval) + { + $this->context->append_var($varname, $varval); + } + + // Docstring is copied from phpbb_template_context method with the same name. + /** + * Assign key variable pairs from an array to a specified block + * @param string $blockname Name of block to assign $vararray to + * @param array $vararray A hash of variable name => value pairs + */ + public function assign_block_vars($blockname, array $vararray) + { + return $this->context->assign_block_vars($blockname, $vararray); + } + + // Docstring is copied from phpbb_template_context method with the same name. + /** + * Change already assigned key variable pair (one-dimensional - single loop entry) + * + * An example of how to use this function: + * {@example alter_block_array.php} + * + * @param string $blockname the blockname, for example 'loop' + * @param array $vararray the var array to insert/add or merge + * @param mixed $key Key to search for + * + * array: KEY => VALUE [the key/value pair to search for within the loop to determine the correct position] + * + * int: Position [the position to change or insert at directly given] + * + * If key is false the position is set to 0 + * If key is true the position is set to the last entry + * + * @param string $mode Mode to execute (valid modes are 'insert' and 'change') + * + * If insert, the vararray is inserted at the given position (position counting from zero). + * If change, the current block gets merged with the vararray (resulting in new key/value pairs be added and existing keys be replaced by the new value). + * + * Since counting begins by zero, inserting at the last position will result in this array: array(vararray, last positioned array) + * and inserting at position 1 will result in this array: array(first positioned array, vararray, following vars) + * + * @return bool false on error, true on success + */ + public function alter_block_array($blockname, array $vararray, $key = false, $mode = 'insert') + { + return $this->context->alter_block_array($blockname, $vararray, $key, $mode); + } + + /** + * Include a separate template. + * + * This function is marked public due to the way the template + * implementation uses it. It is actually an implementation function + * and should not be considered part of template class's public API. + * + * @param string $filename Template filename to include + * @param bool $include True to include the file, false to just load it + * @uses template_compile is used to compile uncached templates + */ + public function _tpl_include($filename, $include = true) + { + $this->locator->set_filenames(array($filename => $filename)); + + if (!$this->load_and_render($filename)) + { + // trigger_error cannot be used here, as the output already started + echo 'template->_tpl_include(): Failed including ' . htmlspecialchars($handle) . "\n"; + } + } + + /** + * Include a PHP file. + * + * If a relative path is passed in $filename, it is considered to be + * relative to board root ($phpbb_root_path). Absolute paths are + * also allowed. + * + * This function is marked public due to the way the template + * implementation uses it. It is actually an implementation function + * and should not be considered part of template class's public API. + * + * @param string $filename Path to PHP file to include + */ + public function _php_include($filename) + { + if (phpbb_is_absolute($filename)) + { + $file = $filename; + } + else + { + $file = $this->phpbb_root_path . $filename; + } + + if (!file_exists($file)) + { + // trigger_error cannot be used here, as the output already started + echo 'template->_php_include(): File ' . htmlspecialchars($file) . " does not exist\n"; + return; + } + include($file); + } + + /** + * Obtains filesystem path for a template file. + * + * The simplest use is specifying a single template file as a string + * in the first argument. This template file should be a basename + * of a template file in the selected style, or its parent styles + * if template inheritance is being utilized. + * + * Note: "selected style" is whatever style the style resource locator + * is configured for. + * + * The return value then will be a path, relative to the current + * directory or absolute, to the template file in the selected style + * or its closest parent. + * + * If the selected style does not have the template file being searched, + * (and if inheritance is involved, none of the parents have it either), + * false will be returned. + * + * Specifying true for $return_default will cause the function to + * return the first path which was checked for existence in the event + * that the template file was not found, instead of false. + * This is the path in the selected style itself, not any of its + * parents. + * + * $files can be given an array of templates instead of a single + * template. When given an array, the function will try to resolve + * each template in the array to a path, and will return the first + * path that exists, or false if none exist. + * + * If $return_full_path is false, then instead of returning a usable + * path (when the template is found) only the template's basename + * will be returned. This can be used to check which of the templates + * specified in $files exists, provided different file names are + * used for different templates. + * + * @param string or array $files List of templates to locate. If there is only + * one template, $files can be a string to make code easier to read. + * @param bool $return_default Determines what to return if template does not + * exist. If true, function will return location where template is + * supposed to be. If false, function will return false. + * @param bool $return_full_path If true, function will return full path + * to template. If false, function will return template file name. + * This parameter can be used to check which one of set of template + * files is available. + * @return string or boolean Source template path if template exists or $return_default is + * true. False if template does not exist and $return_default is false + */ + public function locate($files, $return_default = false, $return_full_path = true) + { + // add template path prefix + $templates = array(); + if (is_string($files)) + { + $templates[] = $this->template_path . $files; + } + else + { + foreach ($files as $file) + { + $templates[] = $this->template_path . $file; + } + } + + // use resource locator to find files + return $this->locator->get_first_file_location($templates, $return_default, $return_full_path); + } + + /** + * Include JS file + * + * @param string $file file name + * @param bool $locate True if file needs to be located + * @param bool $relative True if path is relative to phpBB root directory. Ignored if $locate == true + */ + public function _js_include($file, $locate = false, $relative = false) + { + // Locate file + if ($locate) + { + $located = $this->locator->get_first_file_location(array($file), false, true); + if ($located) + { + $file = $located; + } + } + else if ($relative) + { + $file = $this->phpbb_root_path . $file; + } + + $file .= (strpos($file, '?') === false) ? '?' : '&'; + $file .= 'assets_version=' . $this->config['assets_version']; + + // Add HTML code + $code = ''; + $this->context->append_var('SCRIPTS', $code); + } +} From 46cb0fb068feeb89c939d8b145808d2a786de8dd Mon Sep 17 00:00:00 2001 From: David King Date: Tue, 13 Nov 2012 10:57:24 -0500 Subject: [PATCH 19/67] [feature/controller] Removed another empty construct method PHPBB3-10864 --- tests/controller/includes/controller/foo.php | 7 ------- 1 file changed, 7 deletions(-) diff --git a/tests/controller/includes/controller/foo.php b/tests/controller/includes/controller/foo.php index cd1c4849cb..04576e16c4 100644 --- a/tests/controller/includes/controller/foo.php +++ b/tests/controller/includes/controller/foo.php @@ -4,13 +4,6 @@ use Symfony\Component\HttpFoundation\Response; class phpbb_controller_foo { - /** - * Constructor - */ - public function __construct() - { - } - /** * Bar method * From 5877bf1b1b3dc2899543d99859e3bde6e70d8647 Mon Sep 17 00:00:00 2001 From: David King Date: Tue, 13 Nov 2012 11:02:01 -0500 Subject: [PATCH 20/67] [feature/controller] Update helper service given constructor change PHPBB3-10864 --- phpBB/config/services.yml | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/phpBB/config/services.yml b/phpBB/config/services.yml index 54d347debe..184ebb80ee 100644 --- a/phpBB/config/services.yml +++ b/phpBB/config/services.yml @@ -47,7 +47,10 @@ services: controller.helper: class: phpbb_controller_helper arguments: - - @service_container + - @template + - @user + - %core.root_path% + - .%core.php_ext% controller.resolver: class: phpbb_controller_resolver @@ -64,7 +67,7 @@ services: controller.provider: class: phpbb_controller_provider - + cron.task_collection: class: phpbb_di_service_collection arguments: @@ -113,7 +116,7 @@ services: - %core.root_path% - .%core.php_ext% - @cache.driver - + ext.finder: class: phpbb_extension_finder arguments: From d3aa8823b21990634f8b74676ac301739ddfc58b Mon Sep 17 00:00:00 2001 From: David King Date: Wed, 14 Nov 2012 15:42:13 -0500 Subject: [PATCH 21/67] [feature/controller] Use a dumped url matcher class to improve performance PHPBB3-10864 --- phpBB/config/services.yml | 21 ++------ .../includes/controller/route_collection.php | 36 ------------- phpBB/includes/event/kernel_subscriber.php | 52 ++++++++++++++++++- phpBB/includes/functions.php | 52 +++++++++++++++++++ 4 files changed, 106 insertions(+), 55 deletions(-) delete mode 100644 phpBB/includes/controller/route_collection.php diff --git a/phpBB/config/services.yml b/phpBB/config/services.yml index 184ebb80ee..4fe9fa52eb 100644 --- a/phpBB/config/services.yml +++ b/phpBB/config/services.yml @@ -123,7 +123,7 @@ services: - @ext.manager - %core.root_path% - @cache.driver - - .%core.php_ext% + - %core.php_ext% - _ext_finder http_kernel: @@ -137,22 +137,15 @@ services: arguments: - @template - @user + - @ext.finder + - %core.root_path% + - .%core.php_ext% tags: - { name: kernel.event_subscriber } request: class: phpbb_request - request.context: - class: Symfony\Component\Routing\RequestContext - - router_listener: - class: Symfony\Component\HttpKernel\EventListener\RouterListener - arguments: - - @url_matcher - tags: - - { name: kernel.event_subscriber } - style: class: phpbb_style arguments: @@ -189,11 +182,5 @@ services: template_context: class: phpbb_template_context - url_matcher: - class: Symfony\Component\Routing\Matcher\UrlMatcher - arguments: - - @controller.route_collection - - @request.context - user: class: phpbb_user diff --git a/phpBB/includes/controller/route_collection.php b/phpBB/includes/controller/route_collection.php deleted file mode 100644 index e6c7d3b543..0000000000 --- a/phpBB/includes/controller/route_collection.php +++ /dev/null @@ -1,36 +0,0 @@ -addCollection($provider->get_paths($finder)->find()); - } -} diff --git a/phpBB/includes/event/kernel_subscriber.php b/phpBB/includes/event/kernel_subscriber.php index 9737d9bc23..79ee4f4dc5 100644 --- a/phpBB/includes/event/kernel_subscriber.php +++ b/phpBB/includes/event/kernel_subscriber.php @@ -18,8 +18,11 @@ if (!defined('IN_PHPBB')) use Symfony\Component\EventDispatcher\EventSubscriberInterface; use Symfony\Component\HttpKernel\KernelEvents; use Symfony\Component\HttpKernel\Event\PostResponseEvent; +use Symfony\Component\HttpKernel\Event\GetResponseEvent; use Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent; use Symfony\Component\HttpFoundation\Response; +use Symfony\Component\HttpKernel\EventListener\RouterListener; +use Symfony\Component\Routing\RequestContext; class phpbb_event_kernel_subscriber implements EventSubscriberInterface { @@ -35,16 +38,40 @@ class phpbb_event_kernel_subscriber implements EventSubscriberInterface */ protected $user; + /** + * Extension finder object + * @var phpbb_extension_finder + */ + protected $finder; + + /** + * PHP extension + * @var string + */ + protected $php_ext; + + /** + * Root path + * @var string + */ + protected $root_path; + /** * Construct method * * @param phpbb_template $template Template object * @param phpbb_user $user User object + * @param phpbb_extension_finder $finder Extension finder object + * @param string $root_path Root path + * @param string $php_ext PHP extension */ - public function __construct(phpbb_template $template, phpbb_user $user) + public function __construct(phpbb_template $template, phpbb_user $user, phpbb_extension_finder $finder, $root_path, $php_ext) { $this->template = $template; $this->user = $user; + $this->finder = $finder; + $this->root_path = $root_path; + $this->php_ext = $php_ext; } /** @@ -81,12 +108,33 @@ class phpbb_event_kernel_subscriber implements EventSubscriberInterface page_footer(true, false, false); - $event->setResponse(new Response($this->template->return_display('body'), 404)); + $event->setResponse(new Response($this->template->assign_display('body'), 404)); + } + + /** + * This listener is run when the KernelEvents::REQUEST event is triggered + * + * This is responsible for setting up the routing information + * + * @param GetResponseEvent $event + * @return null + */ + public function on_kernel_request(GetResponseEvent $event) + { + $request = $event->getRequest(); + $context = new RequestContext(); + $context->fromRequest($request); + + $matcher = phpbb_create_url_matcher($this->finder, $context, $this->root_path, $this->php_ext); + + $router_listener = new RouterListener($matcher, $context); + $router_listener->onKernelRequest($event); } public static function getSubscribedEvents() { return array( + KernelEvents::REQUEST => 'on_kernel_request', KernelEvents::TERMINATE => 'on_kernel_terminate', KernelEvents::EXCEPTION => 'on_kernel_exception', ); diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php index fb05b74cd3..7cf5611dca 100644 --- a/phpBB/includes/functions.php +++ b/phpBB/includes/functions.php @@ -7,6 +7,9 @@ * */ +use Symfony\Component\Routing\Matcher\Dumper\PhpMatcherDumper; +use Symfony\Component\Routing\RequestContext; + /** * @ignore */ @@ -5444,3 +5447,52 @@ function phpbb_to_numeric($input) { return ($input > PHP_INT_MAX) ? (float) $input : (int) $input; } + +/** +* Create and/or return the cached phpbb_url_matcher class +* +* If the class already exists, it instantiates it +* +* @param phpbb_extension_finder $finder Extension finder +* @param RequestContext $context Symfony RequestContext object +* @param string $root_path Root path +* @param string $php_ext PHP extension +* @return phpbb_url_matcher +*/ +function phpbb_create_url_matcher(phpbb_extension_finder $finder, RequestContext $context, $root_path, $php_ext) +{ + $matcher = phpbb_load_url_matcher($finder, $context, $root_path, $php_ext); + if ($matcher === false) + { + $provider = new phpbb_controller_provider(); + $dumper = new PhpMatcherDumper($provider->get_paths($finder)->find()); + $cached_url_matcher_dump = $dumper->dump(array( + 'class' => 'phpbb_url_matcher', + )); + + file_put_contents($root_path . 'cache/url_matcher' . $php_ext, $cached_url_matcher_dump); + return phpbb_load_url_matcher($finder, $context, $root_path, $php_ext); + } + + return $matcher; +} + +/** +* Load the cached phpbb_url_matcher class +* +* @param phpbb_extension_finder $finder Extension finder +* @param RequestContext $context Symfony RequestContext object +* @param string $root_path Root path +* @param string $php_ext PHP extension +* @return phpbb_url_matcher|bool False if the file doesn't exist +*/ +function phpbb_load_url_matcher(phpbb_extension_finder $finder, RequestContext $context, $root_path, $php_ext) +{ + if (file_exists($root_path . 'cache/url_matcher' . $php_ext)) + { + include($root_path . 'cache/url_matcher' . $php_ext); + return new phpbb_url_matcher($context); + } + + return false; +} From 269a56e2c3817cc27f2b21b9eb864c4f2452bc13 Mon Sep 17 00:00:00 2001 From: David King Date: Wed, 14 Nov 2012 15:52:41 -0500 Subject: [PATCH 22/67] [feature/controller] Undo removal of period in ext.finder service definition PHPBB3-10864 --- phpBB/config/services.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpBB/config/services.yml b/phpBB/config/services.yml index 4fe9fa52eb..91610a3527 100644 --- a/phpBB/config/services.yml +++ b/phpBB/config/services.yml @@ -123,7 +123,7 @@ services: - @ext.manager - %core.root_path% - @cache.driver - - %core.php_ext% + - .%core.php_ext% - _ext_finder http_kernel: From 196c2d4bc346ab6a31fd0b752c788e37cf39459d Mon Sep 17 00:00:00 2001 From: David King Date: Wed, 14 Nov 2012 15:56:07 -0500 Subject: [PATCH 23/67] [feature/controller] Move new functions to their own file PHPBB3-10864 --- phpBB/includes/functions.php | 49 ----------------- phpBB/includes/functions_url_matcher.php | 68 ++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 49 deletions(-) create mode 100644 phpBB/includes/functions_url_matcher.php diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php index 7cf5611dca..88ce142195 100644 --- a/phpBB/includes/functions.php +++ b/phpBB/includes/functions.php @@ -5447,52 +5447,3 @@ function phpbb_to_numeric($input) { return ($input > PHP_INT_MAX) ? (float) $input : (int) $input; } - -/** -* Create and/or return the cached phpbb_url_matcher class -* -* If the class already exists, it instantiates it -* -* @param phpbb_extension_finder $finder Extension finder -* @param RequestContext $context Symfony RequestContext object -* @param string $root_path Root path -* @param string $php_ext PHP extension -* @return phpbb_url_matcher -*/ -function phpbb_create_url_matcher(phpbb_extension_finder $finder, RequestContext $context, $root_path, $php_ext) -{ - $matcher = phpbb_load_url_matcher($finder, $context, $root_path, $php_ext); - if ($matcher === false) - { - $provider = new phpbb_controller_provider(); - $dumper = new PhpMatcherDumper($provider->get_paths($finder)->find()); - $cached_url_matcher_dump = $dumper->dump(array( - 'class' => 'phpbb_url_matcher', - )); - - file_put_contents($root_path . 'cache/url_matcher' . $php_ext, $cached_url_matcher_dump); - return phpbb_load_url_matcher($finder, $context, $root_path, $php_ext); - } - - return $matcher; -} - -/** -* Load the cached phpbb_url_matcher class -* -* @param phpbb_extension_finder $finder Extension finder -* @param RequestContext $context Symfony RequestContext object -* @param string $root_path Root path -* @param string $php_ext PHP extension -* @return phpbb_url_matcher|bool False if the file doesn't exist -*/ -function phpbb_load_url_matcher(phpbb_extension_finder $finder, RequestContext $context, $root_path, $php_ext) -{ - if (file_exists($root_path . 'cache/url_matcher' . $php_ext)) - { - include($root_path . 'cache/url_matcher' . $php_ext); - return new phpbb_url_matcher($context); - } - - return false; -} diff --git a/phpBB/includes/functions_url_matcher.php b/phpBB/includes/functions_url_matcher.php new file mode 100644 index 0000000000..0a6e90703c --- /dev/null +++ b/phpBB/includes/functions_url_matcher.php @@ -0,0 +1,68 @@ +get_paths($finder)->find()); + $cached_url_matcher_dump = $dumper->dump(array( + 'class' => 'phpbb_url_matcher', + )); + + file_put_contents($root_path . 'cache/url_matcher' . $php_ext, $cached_url_matcher_dump); + return phpbb_load_url_matcher($finder, $context, $root_path, $php_ext); + } + + return $matcher; +} + +/** +* Load the cached phpbb_url_matcher class +* +* @param phpbb_extension_finder $finder Extension finder +* @param RequestContext $context Symfony RequestContext object +* @param string $root_path Root path +* @param string $php_ext PHP extension +* @return phpbb_url_matcher|bool False if the file doesn't exist +*/ +function phpbb_load_url_matcher(phpbb_extension_finder $finder, RequestContext $context, $root_path, $php_ext) +{ + if (file_exists($root_path . 'cache/url_matcher' . $php_ext)) + { + include($root_path . 'cache/url_matcher' . $php_ext); + return new phpbb_url_matcher($context); + } + + return false; +} From 4cb9ec522c7007b99eb5ef44cb1bfdb369478aff Mon Sep 17 00:00:00 2001 From: David King Date: Wed, 14 Nov 2012 16:06:12 -0500 Subject: [PATCH 24/67] [feature/controller] Separate Kernel listeners into their own classes PHPBB3-10864 --- phpBB/config/services.yml | 19 ++++- .../event/kernel_exception_subscriber.php | 79 +++++++++++++++++++ ...iber.php => kernel_request_subscriber.php} | 64 ++------------- .../event/kernel_terminate_subscriber.php | 43 ++++++++++ 4 files changed, 143 insertions(+), 62 deletions(-) create mode 100644 phpBB/includes/event/kernel_exception_subscriber.php rename phpBB/includes/event/{kernel_subscriber.php => kernel_request_subscriber.php} (50%) create mode 100644 phpBB/includes/event/kernel_terminate_subscriber.php diff --git a/phpBB/config/services.yml b/phpBB/config/services.yml index 91610a3527..37e6c0b5df 100644 --- a/phpBB/config/services.yml +++ b/phpBB/config/services.yml @@ -132,17 +132,28 @@ services: - @dispatcher - @controller.resolver - kernel_event_subscriber: - class: phpbb_event_kernel_subscriber + kernel_request_subscriber: + class: phpbb_event_kernel_request_subscriber arguments: - - @template - - @user - @ext.finder - %core.root_path% - .%core.php_ext% tags: - { name: kernel.event_subscriber } + kernel_exception_subscriber: + class: phpbb_event_kernel_exception_subscriber + arguments: + - @template + - @user + tags: + - { name: kernel.event_subscriber } + + kernel_terminate_subscriber: + class: phpbb_event_kernel_terminate_subscriber + tags: + - { name: kernel.event_subscriber } + request: class: phpbb_request diff --git a/phpBB/includes/event/kernel_exception_subscriber.php b/phpBB/includes/event/kernel_exception_subscriber.php new file mode 100644 index 0000000000..cd6ea40c70 --- /dev/null +++ b/phpBB/includes/event/kernel_exception_subscriber.php @@ -0,0 +1,79 @@ +template = $template; + $this->user = $user; + } + + /** + * This listener is run when the KernelEvents::EXCEPTION event is triggered + * + * @param GetResponseForExceptionEvent $event + * @return null + */ + public function on_kernel_exception(GetResponseForExceptionEvent $event) + { + page_header($this->user->lang('INFORMATION')); + + $this->template->assign_vars(array( + 'MESSAGE_TITLE' => $this->user->lang('INFORMATION'), + 'MESSAGE_TEXT' => $event->getException()->getMessage(), + )); + + $this->template->set_filenames(array( + 'body' => 'message_body.html', + )); + + page_footer(true, false, false); + + $event->setResponse(new Response($this->template->assign_display('body'), 404)); + } + + public static function getSubscribedEvents() + { + return array( + KernelEvents::EXCEPTION => 'on_kernel_exception', + ); + } +} diff --git a/phpBB/includes/event/kernel_subscriber.php b/phpBB/includes/event/kernel_request_subscriber.php similarity index 50% rename from phpBB/includes/event/kernel_subscriber.php rename to phpBB/includes/event/kernel_request_subscriber.php index 79ee4f4dc5..98079acabb 100644 --- a/phpBB/includes/event/kernel_subscriber.php +++ b/phpBB/includes/event/kernel_request_subscriber.php @@ -17,27 +17,12 @@ if (!defined('IN_PHPBB')) use Symfony\Component\EventDispatcher\EventSubscriberInterface; use Symfony\Component\HttpKernel\KernelEvents; -use Symfony\Component\HttpKernel\Event\PostResponseEvent; use Symfony\Component\HttpKernel\Event\GetResponseEvent; -use Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent; -use Symfony\Component\HttpFoundation\Response; use Symfony\Component\HttpKernel\EventListener\RouterListener; use Symfony\Component\Routing\RequestContext; -class phpbb_event_kernel_subscriber implements EventSubscriberInterface +class phpbb_event_kernel_request_subscriber implements EventSubscriberInterface { - /** - * Template object - * @var phpbb_template - */ - protected $template; - - /** - * User object - * @var phpbb_user - */ - protected $user; - /** * Extension finder object * @var phpbb_extension_finder @@ -59,13 +44,11 @@ class phpbb_event_kernel_subscriber implements EventSubscriberInterface /** * Construct method * - * @param phpbb_template $template Template object - * @param phpbb_user $user User object * @param phpbb_extension_finder $finder Extension finder object * @param string $root_path Root path * @param string $php_ext PHP extension */ - public function __construct(phpbb_template $template, phpbb_user $user, phpbb_extension_finder $finder, $root_path, $php_ext) + public function __construct(phpbb_extension_finder $finder, $root_path, $php_ext) { $this->template = $template; $this->user = $user; @@ -74,43 +57,6 @@ class phpbb_event_kernel_subscriber implements EventSubscriberInterface $this->php_ext = $php_ext; } - /** - * This listener is run when the KernelEvents::TERMINATE event is triggered - * This comes after a Response has been sent to the server; this is - * primarily cleanup stuff. - * - * @param PostResponseEvent $event - * @return null - */ - public function on_kernel_terminate(PostResponseEvent $event) - { - exit_handler(); - } - - /** - * This listener is run when the KernelEvents::EXCEPTION event is triggered - * - * @param GetResponseForExceptionEvent $event - * @return null - */ - public function on_kernel_exception(GetResponseForExceptionEvent $event) - { - page_header($this->user->lang('INFORMATION')); - - $this->template->assign_vars(array( - 'MESSAGE_TITLE' => $this->user->lang('INFORMATION'), - 'MESSAGE_TEXT' => $event->getException()->getMessage(), - )); - - $this->template->set_filenames(array( - 'body' => 'message_body.html', - )); - - page_footer(true, false, false); - - $event->setResponse(new Response($this->template->assign_display('body'), 404)); - } - /** * This listener is run when the KernelEvents::REQUEST event is triggered * @@ -125,6 +71,10 @@ class phpbb_event_kernel_subscriber implements EventSubscriberInterface $context = new RequestContext(); $context->fromRequest($request); + if (!function_exists('phpbb_create_url_matcher')) + { + include($this->root_path . 'includes/functions_url_matcher' . $this->php_ext); + } $matcher = phpbb_create_url_matcher($this->finder, $context, $this->root_path, $this->php_ext); $router_listener = new RouterListener($matcher, $context); @@ -135,8 +85,6 @@ class phpbb_event_kernel_subscriber implements EventSubscriberInterface { return array( KernelEvents::REQUEST => 'on_kernel_request', - KernelEvents::TERMINATE => 'on_kernel_terminate', - KernelEvents::EXCEPTION => 'on_kernel_exception', ); } } diff --git a/phpBB/includes/event/kernel_terminate_subscriber.php b/phpBB/includes/event/kernel_terminate_subscriber.php new file mode 100644 index 0000000000..1eaf890e42 --- /dev/null +++ b/phpBB/includes/event/kernel_terminate_subscriber.php @@ -0,0 +1,43 @@ + 'on_kernel_terminate', + ); + } +} From fa43edd8778dffd21146350f1749fad5c0755fb7 Mon Sep 17 00:00:00 2001 From: David King Date: Wed, 14 Nov 2012 16:42:52 -0500 Subject: [PATCH 25/67] [feature/controller] Further separate url matcher functionality PHPBB3-10864 --- .../event/kernel_request_subscriber.php | 6 +- phpBB/includes/functions_url_matcher.php | 89 +++++++++++++------ 2 files changed, 65 insertions(+), 30 deletions(-) diff --git a/phpBB/includes/event/kernel_request_subscriber.php b/phpBB/includes/event/kernel_request_subscriber.php index 98079acabb..85c8c5b173 100644 --- a/phpBB/includes/event/kernel_request_subscriber.php +++ b/phpBB/includes/event/kernel_request_subscriber.php @@ -50,8 +50,6 @@ class phpbb_event_kernel_request_subscriber implements EventSubscriberInterface */ public function __construct(phpbb_extension_finder $finder, $root_path, $php_ext) { - $this->template = $template; - $this->user = $user; $this->finder = $finder; $this->root_path = $root_path; $this->php_ext = $php_ext; @@ -71,11 +69,11 @@ class phpbb_event_kernel_request_subscriber implements EventSubscriberInterface $context = new RequestContext(); $context->fromRequest($request); - if (!function_exists('phpbb_create_url_matcher')) + if (!function_exists('phpbb_load_url_matcher')) { include($this->root_path . 'includes/functions_url_matcher' . $this->php_ext); } - $matcher = phpbb_create_url_matcher($this->finder, $context, $this->root_path, $this->php_ext); + $matcher = phpbb_get_url_matcher($this->finder, $context, $this->root_path, $this->php_ext); $router_listener = new RouterListener($matcher, $context); $router_listener->onKernelRequest($event); diff --git a/phpBB/includes/functions_url_matcher.php b/phpBB/includes/functions_url_matcher.php index 0a6e90703c..f628337dee 100644 --- a/phpBB/includes/functions_url_matcher.php +++ b/phpBB/includes/functions_url_matcher.php @@ -8,6 +8,7 @@ */ use Symfony\Component\Routing\Matcher\Dumper\PhpMatcherDumper; +use Symfony\Component\Routing\Matcher\UrlMatcher; use Symfony\Component\Routing\RequestContext; /** @@ -19,50 +20,86 @@ if (!defined('IN_PHPBB')) } /** -* Create and/or return the cached phpbb_url_matcher class -* -* If the class already exists, it instantiates it +* Create a new UrlMatcher class and dump it into the cache file * * @param phpbb_extension_finder $finder Extension finder * @param RequestContext $context Symfony RequestContext object * @param string $root_path Root path * @param string $php_ext PHP extension -* @return phpbb_url_matcher +* @return null */ -function phpbb_create_url_matcher(phpbb_extension_finder $finder, RequestContext $context, $root_path, $php_ext) +function phpbb_get_url_matcher(phpbb_extension_finder $finder, RequestContext $context, $root_path, $php_ext) { - $matcher = phpbb_load_url_matcher($finder, $context, $root_path, $php_ext); - if ($matcher === false) + if (defined('DEBUG')) { - $provider = new phpbb_controller_provider(); - $dumper = new PhpMatcherDumper($provider->get_paths($finder)->find()); - $cached_url_matcher_dump = $dumper->dump(array( - 'class' => 'phpbb_url_matcher', - )); - - file_put_contents($root_path . 'cache/url_matcher' . $php_ext, $cached_url_matcher_dump); - return phpbb_load_url_matcher($finder, $context, $root_path, $php_ext); + return phpbb_create_url_matcher($finder, $context); } - return $matcher; + if (phpbb_url_matcher_dumped($root_path, $php_ext) === false) + { + phpbb_create_dumped_url_matcher($finder, $context, $root_path, $php_ext); + } + + return phpbb_load_url_matcher($context, $root_path, $php_ext); +} + +/** +* Create a new UrlMatcher class and dump it into the cache file +* +* @param phpbb_extension_finder $finder Extension finder +* @param RequestContext $context Symfony RequestContext object +* @param string $root_path Root path +* @param string $php_ext PHP extension +* @return null +*/ +function phpbb_create_dumped_url_matcher(phpbb_extension_finder $finder, RequestContext $context, $root_path, $php_ext) +{ + $provider = new phpbb_controller_provider(); + $dumper = new PhpMatcherDumper($provider->get_paths($finder)->find()); + $cached_url_matcher_dump = $dumper->dump(array( + 'class' => 'phpbb_url_matcher', + )); + + file_put_contents($root_path . 'cache/url_matcher' . $php_ext, $cached_url_matcher_dump); +} + +/** +* Create a non-cached UrlMatcher +* +* @param phpbb_extension_finder $finder Extension finder +* @param RequestContext $context Symfony RequestContext object +* @return UrlMatcher +*/ +function phpbb_create_url_matcher(phpbb_extension_finder $finder, RequestContext $context) +{ + $provider = new phpbb_controller_provider(); + return new UrlMatcher($provider->get_paths($finder)->find(), $context); } /** * Load the cached phpbb_url_matcher class * -* @param phpbb_extension_finder $finder Extension finder * @param RequestContext $context Symfony RequestContext object * @param string $root_path Root path * @param string $php_ext PHP extension -* @return phpbb_url_matcher|bool False if the file doesn't exist +* @return phpbb_url_matcher */ -function phpbb_load_url_matcher(phpbb_extension_finder $finder, RequestContext $context, $root_path, $php_ext) +function phpbb_load_url_matcher(RequestContext $context, $root_path, $php_ext) { - if (file_exists($root_path . 'cache/url_matcher' . $php_ext)) - { - include($root_path . 'cache/url_matcher' . $php_ext); - return new phpbb_url_matcher($context); - } - - return false; + require($root_path . 'cache/url_matcher' . $php_ext); + return new phpbb_url_matcher($context); +} + +/** +* Determine whether we have our dumped URL matcher +* +* The class is automatically dumped to the cache directory +* +* @param string $root_path Root path +* @param string $php_ext PHP extension +* @return bool True if it exists, false if not +*/ +function phpbb_url_matcher_dumped($root_path, $php_ext) +{ + return file_exists($root_path . 'cache/url_matcher' . $php_ext); } From c54c3ee422a9bfb0878aecf80cbb298e230e4fd4 Mon Sep 17 00:00:00 2001 From: David King Date: Wed, 14 Nov 2012 17:04:45 -0500 Subject: [PATCH 26/67] [feature/controller] A few minor nitpickings PHPBB3-10864 --- phpBB/includes/functions_url_matcher.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/phpBB/includes/functions_url_matcher.php b/phpBB/includes/functions_url_matcher.php index f628337dee..782acc4c20 100644 --- a/phpBB/includes/functions_url_matcher.php +++ b/phpBB/includes/functions_url_matcher.php @@ -35,7 +35,7 @@ function phpbb_get_url_matcher(phpbb_extension_finder $finder, RequestContext $c return phpbb_create_url_matcher($finder, $context); } - if (phpbb_url_matcher_dumped($root_path, $php_ext) === false) + if (!phpbb_url_matcher_dumped($root_path, $php_ext)) { phpbb_create_dumped_url_matcher($finder, $context, $root_path, $php_ext); } @@ -55,7 +55,8 @@ function phpbb_get_url_matcher(phpbb_extension_finder $finder, RequestContext $c function phpbb_create_dumped_url_matcher(phpbb_extension_finder $finder, RequestContext $context, $root_path, $php_ext) { $provider = new phpbb_controller_provider(); - $dumper = new PhpMatcherDumper($provider->get_paths($finder)->find()); + $routes = $provider->get_paths($finder)->find(); + $dumper = new PhpMatcherDumper($routes); $cached_url_matcher_dump = $dumper->dump(array( 'class' => 'phpbb_url_matcher', )); From aead33432ae67857009f9570a5ec720d86f7575b Mon Sep 17 00:00:00 2001 From: David King Date: Wed, 14 Nov 2012 17:14:21 -0500 Subject: [PATCH 27/67] [feature/controller] Remove dumped container when cache is purged PHPBB3-10864 --- phpBB/includes/cache/driver/file.php | 1 + phpBB/includes/cache/driver/memory.php | 1 + 2 files changed, 2 insertions(+) diff --git a/phpBB/includes/cache/driver/file.php b/phpBB/includes/cache/driver/file.php index 32bdb1918a..5014ba18af 100644 --- a/phpBB/includes/cache/driver/file.php +++ b/phpBB/includes/cache/driver/file.php @@ -215,6 +215,7 @@ class phpbb_cache_driver_file extends phpbb_cache_driver_base while (($entry = readdir($dir)) !== false) { if (strpos($entry, 'container_') !== 0 && + strpos($entry, 'url_matcher') !== 0 && strpos($entry, 'sql_') !== 0 && strpos($entry, 'data_') !== 0 && strpos($entry, 'ctpl_') !== 0 && diff --git a/phpBB/includes/cache/driver/memory.php b/phpBB/includes/cache/driver/memory.php index 1ea9a3e9e7..f6c42c0ea6 100644 --- a/phpBB/includes/cache/driver/memory.php +++ b/phpBB/includes/cache/driver/memory.php @@ -163,6 +163,7 @@ abstract class phpbb_cache_driver_memory extends phpbb_cache_driver_base while (($entry = readdir($dir)) !== false) { if (strpos($entry, 'container_') !== 0 && + strpos($entry, 'url_matcher') !== 0 && strpos($entry, 'sql_') !== 0 && strpos($entry, 'data_') !== 0 && strpos($entry, 'ctpl_') !== 0 && From b4eff4f06acce78536a392b733f2438f4d1d1c52 Mon Sep 17 00:00:00 2001 From: David King Date: Thu, 15 Nov 2012 13:54:41 -0500 Subject: [PATCH 28/67] [feature/controller] Remove URL Base from helper class I had forgotten that the container sends the same instance of objects to all services that request it, so in this case all controllers would share the same base url path, which is not desired. PHPBB3-10864 --- phpBB/includes/controller/helper.php | 22 +--------------------- 1 file changed, 1 insertion(+), 21 deletions(-) diff --git a/phpBB/includes/controller/helper.php b/phpBB/includes/controller/helper.php index 8bb4427382..6b697c7e46 100644 --- a/phpBB/includes/controller/helper.php +++ b/phpBB/includes/controller/helper.php @@ -47,12 +47,6 @@ class phpbb_controller_helper */ protected $php_ext; - /** - * Base URL - * @var array - */ - protected $url_base; - /** * Constructor * @@ -101,21 +95,7 @@ class phpbb_controller_helper */ public function url(array $url_parts, $query = '') { - return append_sid($this->root_path . $this->url_base . implode('/', $url_parts), $query); - } - - /** - * Set base to prepend to urls generated by url() - * This allows extensions to have a certain 'directory' under which - * all their pages are served, but not have to type it every time - * - * @param array $url_parts Each array element is a 'folder' - * i.e. array('my', 'ext') maps to ./app.php/my/ext - * @return null - */ - public function set_url_base(array $url_parts) - { - $this->url_base = !empty($url_parts) ? implode('/', $url_parts) . '/' : ''; + return append_sid($this->root_path . implode('/', $url_parts), $query); } /** From db1d49d559a337f7266a9e9f0cfaf3eb025b0ed1 Mon Sep 17 00:00:00 2001 From: David King Date: Thu, 15 Nov 2012 13:59:30 -0500 Subject: [PATCH 29/67] [feature/controller] Rename get_paths to import_paths_from_finder Also removed unused variable from url_matcher function PHPBB3-10864 --- phpBB/includes/controller/provider.php | 20 ++++---------------- phpBB/includes/functions_url_matcher.php | 10 +++++----- 2 files changed, 9 insertions(+), 21 deletions(-) diff --git a/phpBB/includes/controller/provider.php b/phpBB/includes/controller/provider.php index 25deedb5d1..b2a5b9f6b2 100644 --- a/phpBB/includes/controller/provider.php +++ b/phpBB/includes/controller/provider.php @@ -39,7 +39,7 @@ class phpbb_controller_provider */ public function __construct($routing_paths = array()) { - $this->set_paths($routing_paths); + $this->routing_paths = $routing_paths; } /** @@ -48,28 +48,16 @@ class phpbb_controller_provider * * @return The current instance of this object for method chaining */ - public function get_paths(phpbb_extension_finder $finder) + public function import_paths_from_finder(phpbb_extension_finder $finder) { // We hardcode the path to the core config directory // because the finder cannot find it - $this->set_paths(array_merge(array('config'), array_map('dirname', array_keys($finder + $this->routing_paths = array_merge(array('config'), array_map('dirname', array_keys($finder ->directory('config') ->prefix('routing') ->suffix('yml') ->find() - )))); - - return $this; - } - - /** - * Set the $routing_paths property with a given list of paths - * - * @return The current instance of this object for method chaining - */ - public function set_paths(array $paths) - { - $this->routing_paths = $paths; + ))); return $this; } diff --git a/phpBB/includes/functions_url_matcher.php b/phpBB/includes/functions_url_matcher.php index 782acc4c20..7280cb74eb 100644 --- a/phpBB/includes/functions_url_matcher.php +++ b/phpBB/includes/functions_url_matcher.php @@ -37,7 +37,7 @@ function phpbb_get_url_matcher(phpbb_extension_finder $finder, RequestContext $c if (!phpbb_url_matcher_dumped($root_path, $php_ext)) { - phpbb_create_dumped_url_matcher($finder, $context, $root_path, $php_ext); + phpbb_create_dumped_url_matcher($finder, $root_path, $php_ext); } return phpbb_load_url_matcher($context, $root_path, $php_ext); @@ -47,15 +47,14 @@ function phpbb_get_url_matcher(phpbb_extension_finder $finder, RequestContext $c * Create a new UrlMatcher class and dump it into the cache file * * @param phpbb_extension_finder $finder Extension finder -* @param RequestContext $context Symfony RequestContext object * @param string $root_path Root path * @param string $php_ext PHP extension * @return null */ -function phpbb_create_dumped_url_matcher(phpbb_extension_finder $finder, RequestContext $context, $root_path, $php_ext) +function phpbb_create_dumped_url_matcher(phpbb_extension_finder $finder, $root_path, $php_ext) { $provider = new phpbb_controller_provider(); - $routes = $provider->get_paths($finder)->find(); + $routes = $provider->import_paths_from_finder($finder)->find(); $dumper = new PhpMatcherDumper($routes); $cached_url_matcher_dump = $dumper->dump(array( 'class' => 'phpbb_url_matcher', @@ -74,7 +73,8 @@ function phpbb_create_dumped_url_matcher(phpbb_extension_finder $finder, Request function phpbb_create_url_matcher(phpbb_extension_finder $finder, RequestContext $context) { $provider = new phpbb_controller_provider(); - return new UrlMatcher($provider->get_paths($finder)->find(), $context); + $routes = $provider->import_paths_from_finder($finder)->find(); + return new UrlMatcher($routes, $context); } /** From 8e480a87253fd2e42af3fc4daaed2f49b9ae65c5 Mon Sep 17 00:00:00 2001 From: David King Date: Thu, 15 Nov 2012 14:48:45 -0500 Subject: [PATCH 30/67] [feature/controller] Move include to app.php PHPBB3-10864 --- phpBB/app.php | 1 + phpBB/includes/event/kernel_request_subscriber.php | 5 ----- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/phpBB/app.php b/phpBB/app.php index 9ff9069104..a2dbb1ae5b 100644 --- a/phpBB/app.php +++ b/phpBB/app.php @@ -17,6 +17,7 @@ define('IN_PHPBB', true); $phpbb_root_path = (defined('PHPBB_ROOT_PATH')) ? PHPBB_ROOT_PATH : './'; $phpEx = substr(strrchr(__FILE__, '.'), 1); include($phpbb_root_path . 'common.' . $phpEx); +include($this->root_path . 'includes/functions_url_matcher' . $this->php_ext); // Start session management $user->session_begin(); diff --git a/phpBB/includes/event/kernel_request_subscriber.php b/phpBB/includes/event/kernel_request_subscriber.php index 85c8c5b173..afb8464f80 100644 --- a/phpBB/includes/event/kernel_request_subscriber.php +++ b/phpBB/includes/event/kernel_request_subscriber.php @@ -69,12 +69,7 @@ class phpbb_event_kernel_request_subscriber implements EventSubscriberInterface $context = new RequestContext(); $context->fromRequest($request); - if (!function_exists('phpbb_load_url_matcher')) - { - include($this->root_path . 'includes/functions_url_matcher' . $this->php_ext); - } $matcher = phpbb_get_url_matcher($this->finder, $context, $this->root_path, $this->php_ext); - $router_listener = new RouterListener($matcher, $context); $router_listener->onKernelRequest($event); } From 91ce6e8b16abb831e1e18b73cf857a257a59e432 Mon Sep 17 00:00:00 2001 From: David King Date: Thu, 15 Nov 2012 14:59:14 -0500 Subject: [PATCH 31/67] [feature/controller] Turn off URL rewriting by default, add comments for usage PHPBB3-10864 --- phpBB/.htaccess | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/phpBB/.htaccess b/phpBB/.htaccess index 68021177f2..1713f8e522 100644 --- a/phpBB/.htaccess +++ b/phpBB/.htaccess @@ -1,7 +1,11 @@ Options +FollowSymLinks -RewriteEngine on +# +# Uncomment the following line if you will be using any of the URL +# rewriting below. +# +#RewriteEngine on # # Uncomment the statement below if you want to make use of @@ -10,9 +14,15 @@ RewriteEngine on # #RewriteRule .* - [E=HTTP_AUTHORIZATION:%{HTTP:Authorization},L] -RewriteCond %{REQUEST_FILENAME} !-f -RewriteCond %{REQUEST_FILENAME} !-d -RewriteRule ^(.*)$ app.php [QSA,L] +# +# Uncomment the following 3 lines if you want to rewrite URLs passed through +# the front controller to not use app.php in the actual URL. In other words, +# an controller that would by default be accessed at /app.php/my/controller +# can now be accessed at /my/controller directly. +# +#RewriteCond %{REQUEST_FILENAME} !-f +#RewriteCond %{REQUEST_FILENAME} !-d +#RewriteRule ^(.*)$ app.php [QSA,L] From db071d68541d132f5b06da652a2214b664552b1e Mon Sep 17 00:00:00 2001 From: David King Date: Thu, 15 Nov 2012 15:05:52 -0500 Subject: [PATCH 32/67] [feature/controller] Fix use of $this in global scope PHPBB3-10864 --- phpBB/app.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpBB/app.php b/phpBB/app.php index a2dbb1ae5b..a7d645cb09 100644 --- a/phpBB/app.php +++ b/phpBB/app.php @@ -17,7 +17,7 @@ define('IN_PHPBB', true); $phpbb_root_path = (defined('PHPBB_ROOT_PATH')) ? PHPBB_ROOT_PATH : './'; $phpEx = substr(strrchr(__FILE__, '.'), 1); include($phpbb_root_path . 'common.' . $phpEx); -include($this->root_path . 'includes/functions_url_matcher' . $this->php_ext); +include($phpbb_root_path . 'includes/functions_url_matcher' . $phpEx); // Start session management $user->session_begin(); From d0269629dcee2dda176807bdd944415d1713db7b Mon Sep 17 00:00:00 2001 From: David King Date: Thu, 15 Nov 2012 15:20:47 -0500 Subject: [PATCH 33/67] [feature/controller] Documentation about Symlinks in .htaccess PHPBB3-10864 --- phpBB/.htaccess | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/phpBB/.htaccess b/phpBB/.htaccess index 1713f8e522..9f635dba57 100644 --- a/phpBB/.htaccess +++ b/phpBB/.htaccess @@ -1,5 +1,3 @@ -Options +FollowSymLinks - # # Uncomment the following line if you will be using any of the URL @@ -23,6 +21,12 @@ Options +FollowSymLinks #RewriteCond %{REQUEST_FILENAME} !-f #RewriteCond %{REQUEST_FILENAME} !-d #RewriteRule ^(.*)$ app.php [QSA,L] + +# +# On Windows, you must also uncomment the following line so that SymLinks +# are followed. +# +#Options +FollowSymLinks From 14f44c17ad76878ed540cd5dd56a3a62b30dbd15 Mon Sep 17 00:00:00 2001 From: David King Date: Thu, 15 Nov 2012 16:14:11 -0500 Subject: [PATCH 34/67] [feature/controller] phpbb_controller_exception instead of RuntimeException PHPBB3-10864 --- phpBB/includes/controller/exception.php | 24 ++++++++++++++++++++++++ phpBB/includes/controller/resolver.php | 11 ++++++----- 2 files changed, 30 insertions(+), 5 deletions(-) create mode 100644 phpBB/includes/controller/exception.php diff --git a/phpBB/includes/controller/exception.php b/phpBB/includes/controller/exception.php new file mode 100644 index 0000000000..da2fefc600 --- /dev/null +++ b/phpBB/includes/controller/exception.php @@ -0,0 +1,24 @@ +user->lang['CONTROLLER_NOT_SPECIFIED']); + throw new phpbb_controller_exception($this->user->lang['CONTROLLER_NOT_SPECIFIED']); } // Require a method name along with the service name if (stripos($controller, ':') === false) { - throw new RuntimeException($this->user->lang['CONTROLLER_METHOD_NOT_SPECIFIED']); + throw new phpbb_controller_exception($this->user->lang['CONTROLLER_METHOD_NOT_SPECIFIED']); } list($service, $method) = explode(':', $controller); if (!$this->container->has($service)) { - throw new RuntimeException($this->user->lang('CONTROLLER_SERVICE_UNDEFINED', $service)); + throw new phpbb_controller_exception($this->user->lang('CONTROLLER_SERVICE_UNDEFINED', $service)); } $controller_object = $this->container->get($service); @@ -92,6 +92,7 @@ class phpbb_controller_resolver implements ControllerResolverInterface * @param Symfony\Component\HttpFoundation\Request $request Symfony Request object * @param string $controller Controller class name * @return bool False + * @throws phpbb_controller_exception */ public function getArguments(Request $request, $controller) { @@ -114,7 +115,7 @@ class phpbb_controller_resolver implements ControllerResolverInterface } else { - throw new RuntimeException($user->lang('CONTROLLER_ARGUMENT_VALUE_MISSING', $param->getPosition() + 1, get_class($object) . ':' . $method, $param->name)); + throw new phpbb_controller_exception($user->lang('CONTROLLER_ARGUMENT_VALUE_MISSING', $param->getPosition() + 1, get_class($object) . ':' . $method, $param->name)); } } From 235b0194f140a369137bd4d92bfd1b10d5e08787 Mon Sep 17 00:00:00 2001 From: David King Date: Thu, 15 Nov 2012 16:16:25 -0500 Subject: [PATCH 35/67] [feature/controller] Update wording in .htaccess documentation comments PHPBB3-10864 --- phpBB/.htaccess | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/phpBB/.htaccess b/phpBB/.htaccess index 9f635dba57..d61ef7b893 100644 --- a/phpBB/.htaccess +++ b/phpBB/.htaccess @@ -15,8 +15,8 @@ # # Uncomment the following 3 lines if you want to rewrite URLs passed through # the front controller to not use app.php in the actual URL. In other words, -# an controller that would by default be accessed at /app.php/my/controller -# can now be accessed at /my/controller directly. +# a controller is by default accessed at /app.php/my/controller, but will then +# be accessed at /my/controller directly. # #RewriteCond %{REQUEST_FILENAME} !-f #RewriteCond %{REQUEST_FILENAME} !-d From 45b3ab8e81a3cffae4d0ada8785620ea4209c207 Mon Sep 17 00:00:00 2001 From: David King Date: Thu, 15 Nov 2012 16:18:02 -0500 Subject: [PATCH 36/67] [feature/controller] Move Response definition into a variable PHPBB3-10864 --- phpBB/includes/event/kernel_exception_subscriber.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/phpBB/includes/event/kernel_exception_subscriber.php b/phpBB/includes/event/kernel_exception_subscriber.php index cd6ea40c70..e2668d4560 100644 --- a/phpBB/includes/event/kernel_exception_subscriber.php +++ b/phpBB/includes/event/kernel_exception_subscriber.php @@ -67,7 +67,8 @@ class phpbb_event_kernel_exception_subscriber implements EventSubscriberInterfac page_footer(true, false, false); - $event->setResponse(new Response($this->template->assign_display('body'), 404)); + $response = new Response($this->template->assign_display('body'), 404); + $event->setResponse($response); } public static function getSubscribedEvents() From f42b36185c86dfe19974c866ce7e284263aff371 Mon Sep 17 00:00:00 2001 From: David King Date: Thu, 15 Nov 2012 16:24:32 -0500 Subject: [PATCH 37/67] [feature/controller] Better explanation of the Options +FollowSymLinks line PHPBB3-10864 --- phpBB/.htaccess | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/phpBB/.htaccess b/phpBB/.htaccess index d61ef7b893..3283b9beb8 100644 --- a/phpBB/.htaccess +++ b/phpBB/.htaccess @@ -23,8 +23,9 @@ #RewriteRule ^(.*)$ app.php [QSA,L] # -# On Windows, you must also uncomment the following line so that SymLinks -# are followed. +# If symbolic links are not already being followed, +# uncomment the line below. +# http://anothersysadmin.wordpress.com/2008/06/10/mod_rewrite-forbidden-403-with-apache-228/ # #Options +FollowSymLinks From b2598662af0b3a37b228cee03cbbf7d7160c7a11 Mon Sep 17 00:00:00 2001 From: David King Date: Thu, 15 Nov 2012 16:26:00 -0500 Subject: [PATCH 38/67] [feature/controller] Clarify working paths after enabling rewriting PHPBB3-10864 --- phpBB/.htaccess | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpBB/.htaccess b/phpBB/.htaccess index 3283b9beb8..d0f65e345c 100644 --- a/phpBB/.htaccess +++ b/phpBB/.htaccess @@ -16,7 +16,7 @@ # Uncomment the following 3 lines if you want to rewrite URLs passed through # the front controller to not use app.php in the actual URL. In other words, # a controller is by default accessed at /app.php/my/controller, but will then -# be accessed at /my/controller directly. +# be accessible at either /app.php/my/controller or just /my/controller # #RewriteCond %{REQUEST_FILENAME} !-f #RewriteCond %{REQUEST_FILENAME} !-d From a87a5dd566bd4b9481cdca86fa6f4bc3363d58de Mon Sep 17 00:00:00 2001 From: David King Date: Thu, 15 Nov 2012 16:31:32 -0500 Subject: [PATCH 39/67] [feature/controller] Fix tests PHPBB3-10864 --- tests/controller/controller_test.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/controller/controller_test.php b/tests/controller/controller_test.php index b73019a6a0..577feee517 100644 --- a/tests/controller/controller_test.php +++ b/tests/controller/controller_test.php @@ -33,7 +33,7 @@ class phpbb_controller_test extends phpbb_test_case { $provider = new phpbb_controller_provider; $routes = $provider - ->get_paths($this->extension_manager->get_finder()) + ->import_paths_from_finder($this->extension_manager->get_finder()) ->find('./tests/controller/'); // This will need to be updated if any new routes are defined From c9588572c9f40223de3445a211ca85b18f5294a4 Mon Sep 17 00:00:00 2001 From: David King Date: Fri, 16 Nov 2012 09:20:15 -0500 Subject: [PATCH 40/67] [feature/controller] Add period before $phpEx PHPBB3-10864 --- phpBB/app.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpBB/app.php b/phpBB/app.php index a7d645cb09..f1023ff1b5 100644 --- a/phpBB/app.php +++ b/phpBB/app.php @@ -17,7 +17,7 @@ define('IN_PHPBB', true); $phpbb_root_path = (defined('PHPBB_ROOT_PATH')) ? PHPBB_ROOT_PATH : './'; $phpEx = substr(strrchr(__FILE__, '.'), 1); include($phpbb_root_path . 'common.' . $phpEx); -include($phpbb_root_path . 'includes/functions_url_matcher' . $phpEx); +include($phpbb_root_path . 'includes/functions_url_matcher.' . $phpEx); // Start session management $user->session_begin(); From 76917558832b0102716448770f365585aac224e9 Mon Sep 17 00:00:00 2001 From: David King Date: Fri, 16 Nov 2012 09:23:57 -0500 Subject: [PATCH 41/67] [feature/controller] Adapt functional tests given new controller framework PHPBB3-10864 --- .../functional/extension_controller_test.php | 129 +++++++----------- .../fixtures/ext/error/class/controller.php | 14 -- .../fixtures/ext/error/class/ext.php | 6 - .../ext/error/classtype/controller.php | 15 -- .../fixtures/ext/error/classtype/ext.php | 6 - .../ext/error/disabled/controller.php | 14 -- .../fixtures/ext/error/disabled/ext.php | 6 - .../fixtures/ext/foo/bar/config/routing.yml | 3 + .../fixtures/ext/foo/bar/config/services.yml | 3 + .../fixtures/ext/foo/bar/controller.php | 14 -- .../ext/foo/bar/controller/controller.php | 10 ++ tests/functional/fixtures/ext/foo/bar/ext.php | 12 +- .../prosilver/template/foobar_body.html | 5 - .../fixtures/ext/foobar/controller.php | 14 -- tests/functional/fixtures/ext/foobar/ext.php | 6 - .../prosilver/template/foobar_body.html | 5 - 16 files changed, 69 insertions(+), 193 deletions(-) delete mode 100644 tests/functional/fixtures/ext/error/class/controller.php delete mode 100644 tests/functional/fixtures/ext/error/class/ext.php delete mode 100644 tests/functional/fixtures/ext/error/classtype/controller.php delete mode 100644 tests/functional/fixtures/ext/error/classtype/ext.php delete mode 100644 tests/functional/fixtures/ext/error/disabled/controller.php delete mode 100644 tests/functional/fixtures/ext/error/disabled/ext.php create mode 100755 tests/functional/fixtures/ext/foo/bar/config/routing.yml create mode 100755 tests/functional/fixtures/ext/foo/bar/config/services.yml delete mode 100644 tests/functional/fixtures/ext/foo/bar/controller.php create mode 100755 tests/functional/fixtures/ext/foo/bar/controller/controller.php mode change 100644 => 100755 tests/functional/fixtures/ext/foo/bar/ext.php delete mode 100644 tests/functional/fixtures/ext/foo/bar/styles/prosilver/template/foobar_body.html delete mode 100644 tests/functional/fixtures/ext/foobar/controller.php delete mode 100644 tests/functional/fixtures/ext/foobar/ext.php delete mode 100644 tests/functional/fixtures/ext/foobar/styles/prosilver/template/foobar_body.html diff --git a/tests/functional/extension_controller_test.php b/tests/functional/extension_controller_test.php index d92a830365..2f07c8a70f 100644 --- a/tests/functional/extension_controller_test.php +++ b/tests/functional/extension_controller_test.php @@ -13,6 +13,14 @@ class phpbb_functional_extension_controller_test extends phpbb_functional_test_case { protected $phpbb_extension_manager; + + static protected $fixtures = array( + 'foo/bar/config/routing.yml', + 'foo/bar/config/services.yml', + 'foo/bar/controller/controller.php', + 'foo/bar/ext.php', + ); + /** * This should only be called once before the tests are run. * This is used to copy the fixtures to the phpBB install @@ -22,15 +30,10 @@ class phpbb_functional_extension_controller_test extends phpbb_functional_test_c global $phpbb_root_path; parent::setUpBeforeClass(); - // these directories need to be created before the files can be copied $directories = array( - $phpbb_root_path . 'ext/error/class/', - $phpbb_root_path . 'ext/error/classtype/', - $phpbb_root_path . 'ext/error/disabled/', $phpbb_root_path . 'ext/foo/bar/', - $phpbb_root_path . 'ext/foo/bar/styles/prosilver/template/', - $phpbb_root_path . 'ext/foobar/', - $phpbb_root_path . 'ext/foobar/styles/prosilver/template/', + $phpbb_root_path . 'ext/foo/bar/config/', + $phpbb_root_path . 'ext/foo/bar/controller/', ); foreach ($directories as $dir) @@ -40,23 +43,8 @@ class phpbb_functional_extension_controller_test extends phpbb_functional_test_c mkdir($dir, 0777, true); } } - - $fixtures = array( - 'error/class/controller.php', - 'error/class/ext.php', - 'error/classtype/controller.php', - 'error/classtype/ext.php', - 'error/disabled/controller.php', - 'error/disabled/ext.php', - 'foo/bar/controller.php', - 'foo/bar/ext.php', - 'foo/bar/styles/prosilver/template/foobar_body.html', - 'foobar/controller.php', - 'foobar/ext.php', - 'foobar/styles/prosilver/template/foobar_body.html', - ); - - foreach ($fixtures as $fixture) + + foreach (self::$fixtures as $fixture) { if (!copy("tests/functional/fixtures/ext/$fixture", "{$phpbb_root_path}ext/$fixture")) { @@ -65,6 +53,27 @@ class phpbb_functional_extension_controller_test extends phpbb_functional_test_c } } + /** + * This should only be called once after the tests are run. + * This is used to remove the fixtures from the phpBB install + */ + static public function tearDownAfterClass() + { + global $phpbb_root_path; + foreach (self::$fixtures as $fixture) + { + if (!unlink("{$phpbb_root_path}ext/$fixture")) + { + echo 'Could not delete file ' . $fixture; + } + } + + rmdir("{$phpbb_root_path}ext/foo/bar/config"); + rmdir("{$phpbb_root_path}ext/foo/bar/controller"); + rmdir("{$phpbb_root_path}ext/foo/bar"); + rmdir("{$phpbb_root_path}ext/foo"); + } + public function setUp() { parent::setUp(); @@ -75,72 +84,28 @@ class phpbb_functional_extension_controller_test extends phpbb_functional_test_c } /** - * Check an extension at ./ext/foobar/ which should have the class - * phpbb_ext_foobar_controller - */ - public function test_foobar() - { - $this->phpbb_extension_manager->enable('foobar'); - $crawler = $this->request('GET', 'index.php?ext=foobar'); - $this->assert_response_success(); - $this->assertContains("This is for testing purposes.", $crawler->filter('#page-body')->text()); - $this->phpbb_extension_manager->purge('foobar'); - } - - /** - * Check an extension at ./ext/foo/bar/ which should have the class - * phpbb_ext_foo_bar_controller + * Check a controller for extension foo/bar */ public function test_foo_bar() { $this->phpbb_extension_manager->enable('foo/bar'); - $crawler = $this->request('GET', 'index.php?ext=foo/bar'); - $this->assert_response_success(); - $this->assertContains("This is for testing purposes.", $crawler->filter('#page-body')->text()); - $this->phpbb_extension_manager->purge('foo/bar'); + $crawler = $this->request('GET', 'app.php/foo/bar'); + $this->assertContains("foo/bar controller handle() method", $crawler->filter('body')->text()); + $this->phpbb_extension_manager->purge('foobar'); } /** - * Check the error produced by extension at ./ext/error/class which has class - * phpbb_ext_foobar_controller + * Check the error produced by extension at ./ext/does/not/exist + * + * If an extension is disabled, its routes are not loaded. Because we + * are not looking for a controller based on a specified extension, + * we don't know the difference between a route in a disabled + * extension and a route that is not defined anyway; it is the same + * error message. */ - public function test_error_class_name() + public function test_error_ext_disabled_or_404() { - $this->phpbb_extension_manager->enable('error/class'); - $crawler = $this->request('GET', 'index.php?ext=error/class'); - $this->assertContains("The extension error/class is missing a controller class and cannot be accessed through the front-end.", $crawler->filter('#message')->text()); - $this->phpbb_extension_manager->purge('error/class'); - } - - /** - * Check the error produced by extension at ./ext/error/classtype which has class - * phpbb_ext_error_classtype_controller but does not implement phpbb_extension_controller_interface - */ - public function test_error_class_type() - { - $this->phpbb_extension_manager->enable('error/classtype'); - $crawler = $this->request('GET', 'index.php?ext=error/classtype'); - $this->assertContains("The extension controller class phpbb_ext_error_classtype_controller is not an instance of the phpbb_extension_controller_interface.", $crawler->filter('#message')->text()); - $this->phpbb_extension_manager->purge('error/classtype'); - } - - /** - * Check the error produced by extension at ./ext/error/disabled that is (obviously) - * a disabled extension - */ - public function test_error_ext_disabled() - { - $crawler = $this->request('GET', 'index.php?ext=error/disabled'); - $this->assertContains("The extension error/disabled is not enabled", $crawler->filter('#message')->text()); - } - - /** - * Check the error produced by extension at ./ext/error/404 that is (obviously) - * not existant - */ - public function test_error_ext_missing() - { - $crawler = $this->request('GET', 'index.php?ext=error/404'); - $this->assertContains("The extension error/404 does not exist.", $crawler->filter('#message')->text()); + $crawler = $this->request('GET', 'app.php/does/not/exist'); + $this->assertContains('No route found for "GET /does/not/exist"', $crawler->filter('body')->text()); } } diff --git a/tests/functional/fixtures/ext/error/class/controller.php b/tests/functional/fixtures/ext/error/class/controller.php deleted file mode 100644 index 74bbbee540..0000000000 --- a/tests/functional/fixtures/ext/error/class/controller.php +++ /dev/null @@ -1,14 +0,0 @@ -template->set_filenames(array( - 'body' => 'index_body.html' - )); - - page_header('Test extension'); - page_footer(); - } -} diff --git a/tests/functional/fixtures/ext/error/class/ext.php b/tests/functional/fixtures/ext/error/class/ext.php deleted file mode 100644 index f97ad2b838..0000000000 --- a/tests/functional/fixtures/ext/error/class/ext.php +++ /dev/null @@ -1,6 +0,0 @@ -set_filenames(array( - 'body' => 'index_body.html' - )); - - page_header('Test extension'); - page_footer(); - } -} diff --git a/tests/functional/fixtures/ext/error/classtype/ext.php b/tests/functional/fixtures/ext/error/classtype/ext.php deleted file mode 100644 index 35b1cd15a2..0000000000 --- a/tests/functional/fixtures/ext/error/classtype/ext.php +++ /dev/null @@ -1,6 +0,0 @@ -template->set_filenames(array( - 'body' => 'index_body.html' - )); - - page_header('Test extension'); - page_footer(); - } -} diff --git a/tests/functional/fixtures/ext/error/disabled/ext.php b/tests/functional/fixtures/ext/error/disabled/ext.php deleted file mode 100644 index aec8051848..0000000000 --- a/tests/functional/fixtures/ext/error/disabled/ext.php +++ /dev/null @@ -1,6 +0,0 @@ -template->set_filenames(array( - 'body' => 'foobar_body.html' - )); - - page_header('Test extension'); - page_footer(); - } -} diff --git a/tests/functional/fixtures/ext/foo/bar/controller/controller.php b/tests/functional/fixtures/ext/foo/bar/controller/controller.php new file mode 100755 index 0000000000..4c5274951f --- /dev/null +++ b/tests/functional/fixtures/ext/foo/bar/controller/controller.php @@ -0,0 +1,10 @@ + - -
This is for testing purposes.
- - diff --git a/tests/functional/fixtures/ext/foobar/controller.php b/tests/functional/fixtures/ext/foobar/controller.php deleted file mode 100644 index ff35f12ee0..0000000000 --- a/tests/functional/fixtures/ext/foobar/controller.php +++ /dev/null @@ -1,14 +0,0 @@ -template->set_filenames(array( - 'body' => 'foobar_body.html' - )); - - page_header('Test extension'); - page_footer(); - } -} diff --git a/tests/functional/fixtures/ext/foobar/ext.php b/tests/functional/fixtures/ext/foobar/ext.php deleted file mode 100644 index 7cf443d369..0000000000 --- a/tests/functional/fixtures/ext/foobar/ext.php +++ /dev/null @@ -1,6 +0,0 @@ - - -
This is for testing purposes.
- - From 74bc46049303952e5f16f61e9f63fa2eeaffbadc Mon Sep 17 00:00:00 2001 From: David King Date: Fri, 16 Nov 2012 09:43:08 -0500 Subject: [PATCH 42/67] [feature/controller] Rename improperly named controller exception class PHPBB3-10864 --- phpBB/includes/controller/exception.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpBB/includes/controller/exception.php b/phpBB/includes/controller/exception.php index da2fefc600..faa8b6b584 100644 --- a/phpBB/includes/controller/exception.php +++ b/phpBB/includes/controller/exception.php @@ -19,6 +19,6 @@ if (!defined('IN_PHPBB')) * Controller exception class * @package phpBB3 */ -class phpbb_controller_provider extends RuntimeException +class phpbb_controller_exception extends RuntimeException { } From 120267e58030eb4b7184e0d54166dfc8d905c3f7 Mon Sep 17 00:00:00 2001 From: David King Date: Fri, 16 Nov 2012 10:27:55 -0500 Subject: [PATCH 43/67] [feature/controller] Correctly access user object PHPBB3-10864 --- phpBB/includes/controller/resolver.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpBB/includes/controller/resolver.php b/phpBB/includes/controller/resolver.php index 0e724ec608..5674a65118 100644 --- a/phpBB/includes/controller/resolver.php +++ b/phpBB/includes/controller/resolver.php @@ -115,7 +115,7 @@ class phpbb_controller_resolver implements ControllerResolverInterface } else { - throw new phpbb_controller_exception($user->lang('CONTROLLER_ARGUMENT_VALUE_MISSING', $param->getPosition() + 1, get_class($object) . ':' . $method, $param->name)); + throw new phpbb_controller_exception($this->user->lang('CONTROLLER_ARGUMENT_VALUE_MISSING', $param->getPosition() + 1, get_class($object) . ':' . $method, $param->name)); } } From e516680859744e69b696ac0054ec3aefd94175b7 Mon Sep 17 00:00:00 2001 From: David King Date: Fri, 16 Nov 2012 10:28:35 -0500 Subject: [PATCH 44/67] [feature/controller] Use sizeof() instead of count() as per guidelines PHPBB3-10864 --- tests/controller/controller_test.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/controller/controller_test.php b/tests/controller/controller_test.php index 577feee517..5a4c65e109 100644 --- a/tests/controller/controller_test.php +++ b/tests/controller/controller_test.php @@ -37,7 +37,7 @@ class phpbb_controller_test extends phpbb_test_case ->find('./tests/controller/'); // This will need to be updated if any new routes are defined - $this->assertEquals(2, count($routes)); + $this->assertEquals(2, sizeof($routes)); } public function test_controller_resolver() From 230897723c9e5fc5534d9781b04ffd8dac70d51b Mon Sep 17 00:00:00 2001 From: David King Date: Fri, 16 Nov 2012 10:29:35 -0500 Subject: [PATCH 45/67] [feature/controller] Reword comment for clarification PHPBB3-10864 --- tests/controller/controller_test.php | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/tests/controller/controller_test.php b/tests/controller/controller_test.php index 5a4c65e109..198fb3c6dd 100644 --- a/tests/controller/controller_test.php +++ b/tests/controller/controller_test.php @@ -43,9 +43,8 @@ class phpbb_controller_test extends phpbb_test_case public function test_controller_resolver() { $container = new ContainerBuilder(); - // For some reason, I cannot get it to load more than one services - // file at a time, even when givein multiple paths - // So instead, I am looping through all of the paths + // YamlFileLoader only uses one path at a time, so we need to loop + // through all of the ones we are using. foreach (array(__DIR__.'/config', __DIR__.'/ext/foo/config') as $path) { $loader = new YamlFileLoader($container, new FileLocator($path)); @@ -53,7 +52,7 @@ class phpbb_controller_test extends phpbb_test_case } // Autoloading classes within the tests folder does not work - // so I'll include them manually + // so I'll include them manually. if (!class_exists('phpbb_ext_foo_controller')) { include(__DIR__.'/ext/foo/controller.php'); From 0c75d3d7da9c5fd7ee1560597db684c9d1704c22 Mon Sep 17 00:00:00 2001 From: David King Date: Fri, 16 Nov 2012 10:30:50 -0500 Subject: [PATCH 46/67] [feature/controller] Add test for missing argument in controller class PHPBB3-10864 --- tests/functional/extension_controller_test.php | 10 +++++++++- .../functional/fixtures/ext/foo/bar/config/routing.yml | 4 ++++ .../fixtures/ext/foo/bar/controller/controller.php | 5 +++++ tests/functional/fixtures/ext/foo/bar/ext.php | 2 +- 4 files changed, 19 insertions(+), 2 deletions(-) diff --git a/tests/functional/extension_controller_test.php b/tests/functional/extension_controller_test.php index 2f07c8a70f..b1e910aeac 100644 --- a/tests/functional/extension_controller_test.php +++ b/tests/functional/extension_controller_test.php @@ -91,7 +91,15 @@ class phpbb_functional_extension_controller_test extends phpbb_functional_test_c $this->phpbb_extension_manager->enable('foo/bar'); $crawler = $this->request('GET', 'app.php/foo/bar'); $this->assertContains("foo/bar controller handle() method", $crawler->filter('body')->text()); - $this->phpbb_extension_manager->purge('foobar'); + $this->phpbb_extension_manager->purge('foo/bar'); + } + + public function test_missing_argument() + { + $this->phpbb_extension_manager->enable('foo/bar'); + $crawler = $this->request('GET', 'app.php/foo/baz'); + $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'); } /** diff --git a/tests/functional/fixtures/ext/foo/bar/config/routing.yml b/tests/functional/fixtures/ext/foo/bar/config/routing.yml index 7ecaba9e71..945ef51366 100755 --- a/tests/functional/fixtures/ext/foo/bar/config/routing.yml +++ b/tests/functional/fixtures/ext/foo/bar/config/routing.yml @@ -1,3 +1,7 @@ foo_bar_controller: pattern: /foo/bar defaults: { _controller: foo_bar.controller:handle } + +foo_baz_controller: + pattern: /foo/baz + defaults: { _controller: foo_bar.controller:baz } diff --git a/tests/functional/fixtures/ext/foo/bar/controller/controller.php b/tests/functional/fixtures/ext/foo/bar/controller/controller.php index 4c5274951f..def5184e8c 100755 --- a/tests/functional/fixtures/ext/foo/bar/controller/controller.php +++ b/tests/functional/fixtures/ext/foo/bar/controller/controller.php @@ -7,4 +7,9 @@ class phpbb_ext_foo_bar_controller { return new Response('foo/bar controller handle() method', 200); } + + public function baz($test) + { + return new Response('Value of "test" URL argument is: ' . $test); + } } diff --git a/tests/functional/fixtures/ext/foo/bar/ext.php b/tests/functional/fixtures/ext/foo/bar/ext.php index b50f95990e..7170209d53 100755 --- a/tests/functional/fixtures/ext/foo/bar/ext.php +++ b/tests/functional/fixtures/ext/foo/bar/ext.php @@ -1,6 +1,6 @@ Date: Fri, 16 Nov 2012 10:35:55 -0500 Subject: [PATCH 47/67] [feature/controller] Fix param block for controller callable PHPBB3-10864 --- phpBB/includes/controller/resolver.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpBB/includes/controller/resolver.php b/phpBB/includes/controller/resolver.php index 5674a65118..901aa7eaa0 100644 --- a/phpBB/includes/controller/resolver.php +++ b/phpBB/includes/controller/resolver.php @@ -90,7 +90,7 @@ class phpbb_controller_resolver implements ControllerResolverInterface * controller. * * @param Symfony\Component\HttpFoundation\Request $request Symfony Request object - * @param string $controller Controller class name + * @param mixed $controller A callable (controller class, method) * @return bool False * @throws phpbb_controller_exception */ From d41b1146e8947919ed9bdd41e6a30ad15e91d262 Mon Sep 17 00:00:00 2001 From: David King Date: Fri, 16 Nov 2012 10:37:07 -0500 Subject: [PATCH 48/67] [feature/controller] Rename $root_path class property to $phpbb_root_path PHPBB3-10864 --- phpBB/includes/controller/helper.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/phpBB/includes/controller/helper.php b/phpBB/includes/controller/helper.php index 6b697c7e46..6dc3ec4626 100644 --- a/phpBB/includes/controller/helper.php +++ b/phpBB/includes/controller/helper.php @@ -39,7 +39,7 @@ class phpbb_controller_helper * phpBB root path * @var string */ - protected $root_path; + protected $phpbb_root_path; /** * PHP extension @@ -52,14 +52,14 @@ class phpbb_controller_helper * * @param phpbb_template $template Template object * @param phpbb_user $user User object - * @param string $root_path phpBB root path + * @param string $phpbb_root_path phpBB root path * @param string $php_ext PHP extension */ - public function __construct(phpbb_template $template, phpbb_user $user, $root_path, $php_ext) + public function __construct(phpbb_template $template, phpbb_user $user, $phpbb_root_path, $php_ext) { $this->template = $template; $this->user = $user; - $this->root_path = $root_path; + $this->phpbb_root_path = $phpbb_root_path; $this->php_ext = $php_ext; } @@ -95,7 +95,7 @@ class phpbb_controller_helper */ public function url(array $url_parts, $query = '') { - return append_sid($this->root_path . implode('/', $url_parts), $query); + return append_sid($this->phpbb_root_path . implode('/', $url_parts), $query); } /** From 1952a4f276930edd16ca51fd690ed3e7965071da Mon Sep 17 00:00:00 2001 From: David King Date: Fri, 16 Nov 2012 10:38:40 -0500 Subject: [PATCH 49/67] [feature/controller] Flip method parameters, require $message PHPBB3-10864 --- phpBB/includes/controller/helper.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/phpBB/includes/controller/helper.php b/phpBB/includes/controller/helper.php index 6dc3ec4626..6cacc8fefa 100644 --- a/phpBB/includes/controller/helper.php +++ b/phpBB/includes/controller/helper.php @@ -101,11 +101,11 @@ class phpbb_controller_helper /** * Output an error, effectively the same thing as trigger_error * - * @param string $code The error code (e.g. 404, 500, 503, etc.) * @param string $message The error message + * @param string $code The error code (e.g. 404, 500, 503, etc.) * @return Response A Reponse instance */ - public function error($code = 500, $message = '') + public function error($message, $code = 500) { $this->template->assign_vars(array( 'MESSAGE_TEXT' => $message, From ba1acdca0364a9cd54c705d946fbae2144c59554 Mon Sep 17 00:00:00 2001 From: David King Date: Fri, 16 Nov 2012 10:42:10 -0500 Subject: [PATCH 50/67] [feature/controller] Use warning instead of echo for copy() and unlink() PHPBB3-10864 --- tests/functional/extension_controller_test.php | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/tests/functional/extension_controller_test.php b/tests/functional/extension_controller_test.php index b1e910aeac..65a9c80df6 100644 --- a/tests/functional/extension_controller_test.php +++ b/tests/functional/extension_controller_test.php @@ -46,10 +46,7 @@ class phpbb_functional_extension_controller_test extends phpbb_functional_test_c foreach (self::$fixtures as $fixture) { - if (!copy("tests/functional/fixtures/ext/$fixture", "{$phpbb_root_path}ext/$fixture")) - { - echo 'Could not copy file ' . $fixture; - } + copy("tests/functional/fixtures/ext/$fixture", "{$phpbb_root_path}ext/$fixture"); } } @@ -62,10 +59,7 @@ class phpbb_functional_extension_controller_test extends phpbb_functional_test_c global $phpbb_root_path; foreach (self::$fixtures as $fixture) { - if (!unlink("{$phpbb_root_path}ext/$fixture")) - { - echo 'Could not delete file ' . $fixture; - } + unlink("{$phpbb_root_path}ext/$fixture"); } rmdir("{$phpbb_root_path}ext/foo/bar/config"); From 5b013ddf5c48e71166dcefd6d384aea1d801698a Mon Sep 17 00:00:00 2001 From: David King Date: Fri, 16 Nov 2012 11:41:05 -0500 Subject: [PATCH 51/67] [feature/controller] Add controller functional test with template PHPBB3-10864 --- .../functional/extension_controller_test.php | 32 ++++++++++++++++--- .../fixtures/ext/foo/bar/config/routing.yml | 4 +++ .../fixtures/ext/foo/bar/config/services.yml | 3 ++ .../ext/foo/bar/controller/controller.php | 15 +++++++++ .../prosilver/template/foo_bar_body.html | 3 ++ 5 files changed, 52 insertions(+), 5 deletions(-) create mode 100644 tests/functional/fixtures/ext/foo/bar/styles/prosilver/template/foo_bar_body.html diff --git a/tests/functional/extension_controller_test.php b/tests/functional/extension_controller_test.php index 65a9c80df6..9cc2e0e32f 100644 --- a/tests/functional/extension_controller_test.php +++ b/tests/functional/extension_controller_test.php @@ -18,7 +18,7 @@ class phpbb_functional_extension_controller_test extends phpbb_functional_test_c 'foo/bar/config/routing.yml', 'foo/bar/config/services.yml', 'foo/bar/controller/controller.php', - 'foo/bar/ext.php', + 'foo/bar/styles/prosilver/template/foo_bar_body.html', ); /** @@ -34,6 +34,7 @@ class phpbb_functional_extension_controller_test extends phpbb_functional_test_c $phpbb_root_path . 'ext/foo/bar/', $phpbb_root_path . 'ext/foo/bar/config/', $phpbb_root_path . 'ext/foo/bar/controller/', + $phpbb_root_path . 'ext/foo/bar/styles/prosilver/template', ); foreach ($directories as $dir) @@ -43,10 +44,12 @@ class phpbb_functional_extension_controller_test extends phpbb_functional_test_c mkdir($dir, 0777, true); } } - + foreach (self::$fixtures as $fixture) { - copy("tests/functional/fixtures/ext/$fixture", "{$phpbb_root_path}ext/$fixture"); + copy( + "tests/functional/fixtures/ext/$fixture", + "{$phpbb_root_path}ext/$fixture"); } } @@ -57,6 +60,7 @@ class phpbb_functional_extension_controller_test extends phpbb_functional_test_c static public function tearDownAfterClass() { global $phpbb_root_path; + foreach (self::$fixtures as $fixture) { unlink("{$phpbb_root_path}ext/$fixture"); @@ -64,6 +68,9 @@ class phpbb_functional_extension_controller_test extends phpbb_functional_test_c rmdir("{$phpbb_root_path}ext/foo/bar/config"); rmdir("{$phpbb_root_path}ext/foo/bar/controller"); + rmdir("{$phpbb_root_path}ext/foo/bar/styles/prosilver/template"); + rmdir("{$phpbb_root_path}ext/foo/bar/styles/prosilver"); + rmdir("{$phpbb_root_path}ext/foo/bar/styles"); rmdir("{$phpbb_root_path}ext/foo/bar"); rmdir("{$phpbb_root_path}ext/foo"); } @@ -78,7 +85,7 @@ class phpbb_functional_extension_controller_test extends phpbb_functional_test_c } /** - * Check a controller for extension foo/bar + * Check a controller for extension foo/bar. */ public function test_foo_bar() { @@ -88,6 +95,21 @@ class phpbb_functional_extension_controller_test extends phpbb_functional_test_c $this->phpbb_extension_manager->purge('foo/bar'); } + /** + * Check the output of a controller using the template system + */ + public function test_controller_with_template() + { + $this->phpbb_extension_manager->enable('foo/bar'); + $crawler = $this->request('GET', 'app.php/foo/template'); + $this->assertContains("I am a variable", $crawler->filter('#content')->text()); + $this->phpbb_extension_manager->purge('foo/bar'); + } + + /** + * Check the error produced by calling a controller without a required + * argument. + */ public function test_missing_argument() { $this->phpbb_extension_manager->enable('foo/bar'); @@ -97,7 +119,7 @@ class phpbb_functional_extension_controller_test extends phpbb_functional_test_c } /** - * Check the error produced by extension at ./ext/does/not/exist + * Check the error produced by extension at ./ext/does/not/exist. * * If an extension is disabled, its routes are not loaded. Because we * are not looking for a controller based on a specified extension, diff --git a/tests/functional/fixtures/ext/foo/bar/config/routing.yml b/tests/functional/fixtures/ext/foo/bar/config/routing.yml index 945ef51366..123f5f1b73 100755 --- a/tests/functional/fixtures/ext/foo/bar/config/routing.yml +++ b/tests/functional/fixtures/ext/foo/bar/config/routing.yml @@ -5,3 +5,7 @@ foo_bar_controller: foo_baz_controller: pattern: /foo/baz defaults: { _controller: foo_bar.controller:baz } + +foo_template_controller: + pattern: /foo/template + defaults: { _controller: foo_bar.controller:template } diff --git a/tests/functional/fixtures/ext/foo/bar/config/services.yml b/tests/functional/fixtures/ext/foo/bar/config/services.yml index d1db6fcb1d..33ced55af9 100755 --- a/tests/functional/fixtures/ext/foo/bar/config/services.yml +++ b/tests/functional/fixtures/ext/foo/bar/config/services.yml @@ -1,3 +1,6 @@ services: foo_bar.controller: class: phpbb_ext_foo_bar_controller + arguments: + - @controller.helper + - @template diff --git a/tests/functional/fixtures/ext/foo/bar/controller/controller.php b/tests/functional/fixtures/ext/foo/bar/controller/controller.php index def5184e8c..50ea5d034b 100755 --- a/tests/functional/fixtures/ext/foo/bar/controller/controller.php +++ b/tests/functional/fixtures/ext/foo/bar/controller/controller.php @@ -3,6 +3,14 @@ use Symfony\Component\HttpFoundation\Response; class phpbb_ext_foo_bar_controller { + protected $template; + + public function __construct(phpbb_controller_helper $helper, phpbb_template $template) + { + $this->template = $template; + $this->helper = $helper; + } + public function handle() { return new Response('foo/bar controller handle() method', 200); @@ -12,4 +20,11 @@ class phpbb_ext_foo_bar_controller { return new Response('Value of "test" URL argument is: ' . $test); } + + public function template() + { + $this->template->assign_var('A_VARIABLE', 'I am a variable'); + + return $this->helper->render('foo_bar_body.html'); + } } diff --git a/tests/functional/fixtures/ext/foo/bar/styles/prosilver/template/foo_bar_body.html b/tests/functional/fixtures/ext/foo/bar/styles/prosilver/template/foo_bar_body.html new file mode 100644 index 0000000000..8fb6994d3d --- /dev/null +++ b/tests/functional/fixtures/ext/foo/bar/styles/prosilver/template/foo_bar_body.html @@ -0,0 +1,3 @@ + +
{A_VARIABLE}
+ From abf2575bdbad84ca2d139290789852ee51efd31c Mon Sep 17 00:00:00 2001 From: David King Date: Fri, 16 Nov 2012 13:04:12 -0500 Subject: [PATCH 52/67] [feature/controller] Remove URL rewriting by default PHPBB3-10864 --- phpBB/web.config | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/phpBB/web.config b/phpBB/web.config index e31a48b991..a73c328626 100644 --- a/phpBB/web.config +++ b/phpBB/web.config @@ -12,18 +12,6 @@ - - - - - - - - - - - - From 4efbb893b7b8ada8766847dc59724faef9c18142 Mon Sep 17 00:00:00 2001 From: David King Date: Fri, 16 Nov 2012 17:36:39 -0500 Subject: [PATCH 53/67] [feature/controller] Fix line endings and permissions, and check responses PHPBB3-10864 --- tests/functional/extension_controller_test.php | 4 ++++ .../fixtures/ext/foo/bar/config/routing.yml | 0 .../fixtures/ext/foo/bar/config/services.yml | 0 .../fixtures/ext/foo/bar/controller/controller.php | 0 tests/functional/fixtures/ext/foo/bar/ext.php | 12 ++++++------ 5 files changed, 10 insertions(+), 6 deletions(-) mode change 100755 => 100644 tests/functional/fixtures/ext/foo/bar/config/routing.yml mode change 100755 => 100644 tests/functional/fixtures/ext/foo/bar/config/services.yml mode change 100755 => 100644 tests/functional/fixtures/ext/foo/bar/controller/controller.php mode change 100755 => 100644 tests/functional/fixtures/ext/foo/bar/ext.php diff --git a/tests/functional/extension_controller_test.php b/tests/functional/extension_controller_test.php index 9cc2e0e32f..ba4a4e8ef0 100644 --- a/tests/functional/extension_controller_test.php +++ b/tests/functional/extension_controller_test.php @@ -91,6 +91,7 @@ class phpbb_functional_extension_controller_test extends phpbb_functional_test_c { $this->phpbb_extension_manager->enable('foo/bar'); $crawler = $this->request('GET', 'app.php/foo/bar'); + $this->assert_response_success(); $this->assertContains("foo/bar controller handle() method", $crawler->filter('body')->text()); $this->phpbb_extension_manager->purge('foo/bar'); } @@ -102,6 +103,7 @@ class phpbb_functional_extension_controller_test extends phpbb_functional_test_c { $this->phpbb_extension_manager->enable('foo/bar'); $crawler = $this->request('GET', 'app.php/foo/template'); + $this->assert_response_success(); $this->assertContains("I am a variable", $crawler->filter('#content')->text()); $this->phpbb_extension_manager->purge('foo/bar'); } @@ -114,6 +116,7 @@ class phpbb_functional_extension_controller_test extends phpbb_functional_test_c { $this->phpbb_extension_manager->enable('foo/bar'); $crawler = $this->request('GET', 'app.php/foo/baz'); + $this->assertEquals(404, $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'); } @@ -130,6 +133,7 @@ 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/does/not/exist'); + $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 old mode 100755 new mode 100644 diff --git a/tests/functional/fixtures/ext/foo/bar/config/services.yml b/tests/functional/fixtures/ext/foo/bar/config/services.yml old mode 100755 new mode 100644 diff --git a/tests/functional/fixtures/ext/foo/bar/controller/controller.php b/tests/functional/fixtures/ext/foo/bar/controller/controller.php old mode 100755 new mode 100644 diff --git a/tests/functional/fixtures/ext/foo/bar/ext.php b/tests/functional/fixtures/ext/foo/bar/ext.php old mode 100755 new mode 100644 index 7170209d53..74359d51ab --- a/tests/functional/fixtures/ext/foo/bar/ext.php +++ b/tests/functional/fixtures/ext/foo/bar/ext.php @@ -1,6 +1,6 @@ - Date: Sat, 17 Nov 2012 17:48:20 -0500 Subject: [PATCH 54/67] [feature/controller] Use query string, not path info, for controller access This is hopefully just temporary until we can fix the relative path issue. PHPBB3-10864 --- phpBB/app.php | 8 ++++++++ phpBB/common.php | 7 ------- phpBB/includes/functions.php | 16 +--------------- 3 files changed, 9 insertions(+), 22 deletions(-) diff --git a/phpBB/app.php b/phpBB/app.php index f1023ff1b5..13bf3f0be1 100644 --- a/phpBB/app.php +++ b/phpBB/app.php @@ -7,6 +7,8 @@ * */ +use Symfony\Component\HttpFoundation\Request; + /** */ @@ -24,6 +26,12 @@ $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', '', false, phpbb_request_interface::GET); +$uri = '/' . $controller; +$symfony_request = Request::create($uri); + $http_kernel = $phpbb_container->get('http_kernel'); $response = $http_kernel->handle($symfony_request); $response->send(); diff --git a/phpBB/common.php b/phpBB/common.php index a5ffcea8e4..e99b9edee5 100644 --- a/phpBB/common.php +++ b/phpBB/common.php @@ -8,8 +8,6 @@ * Minimum Requirement: PHP 5.3.3 */ -use Symfony\Component\HttpFoundation\Request; - /** */ if (!defined('IN_PHPBB')) @@ -105,11 +103,6 @@ $phpbb_class_loader_ext->set_cache($phpbb_container->get('cache.driver')); // set up caching $cache = $phpbb_container->get('cache'); -// Instantiate the Symfony Request object -// This must be done before phpbb_request -// because otherwise globals are disabled -$symfony_request = Request::createFromGlobals(); - // Instantiate some basic classes $phpbb_dispatcher = $phpbb_container->get('dispatcher'); $request = $phpbb_container->get('request'); diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php index 88ce142195..17fc16ef86 100644 --- a/phpBB/includes/functions.php +++ b/phpBB/includes/functions.php @@ -2346,20 +2346,6 @@ function append_sid($url, $params = false, $is_amp = true, $session_id = false) $params = false; } - // Make sure we have a Symfony Request object; tests do not have one - // unless they need it. - if ($symfony_request) - { - // Correct the path when we are accessing it through a controller - // This simply rewrites the value given by $phpbb_root_path to the - // script_path in config. - $path_info = $symfony_request->getPathInfo(); - if (!empty($path_info) && $path_info != '/') - { - $url = $config['script_path'] . '/' . substr($url, strlen($phpbb_root_path)); - } - } - $append_sid_overwrite = false; /** @@ -5056,7 +5042,7 @@ function page_header($page_title = '', $display_online_list = true, $item_id = 0 // Determine board url - we may need it later $board_url = generate_board_url() . '/'; - $web_path = (defined('PHPBB_USE_BOARD_URL_PATH') && PHPBB_USE_BOARD_URL_PATH) ? $board_url : $config['script_path'] . '/'; + $web_path = (defined('PHPBB_USE_BOARD_URL_PATH') && PHPBB_USE_BOARD_URL_PATH) ? $board_url : $phpbb_root_path; // Send a proper content-language to the output $user_lang = $user->lang['USER_LANG']; From 4d6f6351dd0563b26105d15b98052d907f9c52ed Mon Sep 17 00:00:00 2001 From: David King Date: Sat, 17 Nov 2012 18:05:32 -0500 Subject: [PATCH 55/67] [feature/controller] Allow injecting Symfony Request into controllers PHPBB3-10864 --- phpBB/includes/controller/resolver.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/phpBB/includes/controller/resolver.php b/phpBB/includes/controller/resolver.php index 901aa7eaa0..ee469aa9c8 100644 --- a/phpBB/includes/controller/resolver.php +++ b/phpBB/includes/controller/resolver.php @@ -109,6 +109,10 @@ class phpbb_controller_resolver implements ControllerResolverInterface { $arguments[] = $attributes[$param->name]; } + else if ($param->getClass() && $param->getClass()->isInstance($request)) + { + $arguments[] = $request; + } else if ($param->isDefaultValueAvailable()) { $arguments[] = $param->getDefaultValue(); From 7a3d9ed85dfd7f32a938070fff854e56bf39738e Mon Sep 17 00:00:00 2001 From: David King Date: Sun, 18 Nov 2012 13:11:24 -0500 Subject: [PATCH 56/67] [feature/controller] Fix functional tests to use query string for controllers PHPBB3-10864 --- tests/functional/extension_controller_test.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/functional/extension_controller_test.php b/tests/functional/extension_controller_test.php index ba4a4e8ef0..482fe9dfd3 100644 --- a/tests/functional/extension_controller_test.php +++ b/tests/functional/extension_controller_test.php @@ -90,7 +90,7 @@ class phpbb_functional_extension_controller_test extends phpbb_functional_test_c public function test_foo_bar() { $this->phpbb_extension_manager->enable('foo/bar'); - $crawler = $this->request('GET', 'app.php/foo/bar'); + $crawler = $this->request('GET', 'app.php?controller=foo/bar'); $this->assert_response_success(); $this->assertContains("foo/bar controller handle() method", $crawler->filter('body')->text()); $this->phpbb_extension_manager->purge('foo/bar'); @@ -102,7 +102,7 @@ class phpbb_functional_extension_controller_test extends phpbb_functional_test_c public function test_controller_with_template() { $this->phpbb_extension_manager->enable('foo/bar'); - $crawler = $this->request('GET', 'app.php/foo/template'); + $crawler = $this->request('GET', 'app.php?controller=foo/template'); $this->assert_response_success(); $this->assertContains("I am a variable", $crawler->filter('#content')->text()); $this->phpbb_extension_manager->purge('foo/bar'); @@ -115,7 +115,7 @@ class phpbb_functional_extension_controller_test extends phpbb_functional_test_c public function test_missing_argument() { $this->phpbb_extension_manager->enable('foo/bar'); - $crawler = $this->request('GET', 'app.php/foo/baz'); + $crawler = $this->request('GET', 'app.php?controller=foo/baz'); $this->assertEquals(404, $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'); @@ -132,7 +132,7 @@ 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/does/not/exist'); + $crawler = $this->request('GET', 'app.php?controller=does/not/exist'); $this->assertEquals(404, $this->client->getResponse()->getStatus()); $this->assertContains('No route found for "GET /does/not/exist"', $crawler->filter('body')->text()); } From 09d7367dfc80f87b2fa36785a1e4285666c2d58a Mon Sep 17 00:00:00 2001 From: David King Date: Sun, 18 Nov 2012 13:32:54 -0500 Subject: [PATCH 57/67] [feature/controller] Remove url rewriting until we use pathinfo in controllers PHPBB3-10864 --- phpBB/.htaccess | 28 +++------------------------- 1 file changed, 3 insertions(+), 25 deletions(-) diff --git a/phpBB/.htaccess b/phpBB/.htaccess index d0f65e345c..474f9774c2 100644 --- a/phpBB/.htaccess +++ b/phpBB/.htaccess @@ -1,34 +1,12 @@ - -# -# Uncomment the following line if you will be using any of the URL -# rewriting below. -# -#RewriteEngine on - # # Uncomment the statement below if you want to make use of # HTTP authentication and it does not already work. # This could be required if you are for example using PHP via Apache CGI. # +# +#RewriteEngine on #RewriteRule .* - [E=HTTP_AUTHORIZATION:%{HTTP:Authorization},L] - -# -# Uncomment the following 3 lines if you want to rewrite URLs passed through -# the front controller to not use app.php in the actual URL. In other words, -# a controller is by default accessed at /app.php/my/controller, but will then -# be accessible at either /app.php/my/controller or just /my/controller -# -#RewriteCond %{REQUEST_FILENAME} !-f -#RewriteCond %{REQUEST_FILENAME} !-d -#RewriteRule ^(.*)$ app.php [QSA,L] - -# -# If symbolic links are not already being followed, -# uncomment the line below. -# http://anothersysadmin.wordpress.com/2008/06/10/mod_rewrite-forbidden-403-with-apache-228/ -# -#Options +FollowSymLinks - +# Order Allow,Deny From 53caf83233c962adbb68dcfb0f8172ebf788b8f7 Mon Sep 17 00:00:00 2001 From: David King Date: Sun, 18 Nov 2012 13:35:04 -0500 Subject: [PATCH 58/67] [feature/controller] Remove now-unused code PHPBB3-10864 --- phpBB/includes/functions.php | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php index 17fc16ef86..02a9e33f2a 100644 --- a/phpBB/includes/functions.php +++ b/phpBB/includes/functions.php @@ -7,9 +7,6 @@ * */ -use Symfony\Component\Routing\Matcher\Dumper\PhpMatcherDumper; -use Symfony\Component\Routing\RequestContext; - /** * @ignore */ @@ -2338,7 +2335,7 @@ function phpbb_on_page($template, $user, $base_url, $num_items, $per_page, $star function append_sid($url, $params = false, $is_amp = true, $session_id = false) { global $_SID, $_EXTRA_URL, $phpbb_hook; - global $phpbb_dispatcher, $phpbb_root_path, $config, $symfony_request; + global $phpbb_dispatcher; if ($params === '' || (is_array($params) && empty($params))) { From 50a96a2a2d25734e3df451b0f821817213f085e6 Mon Sep 17 00:00:00 2001 From: David King Date: Sun, 18 Nov 2012 13:40:24 -0500 Subject: [PATCH 59/67] [feature/controller] Update routing documentation for using query string PHPBB3-10864 --- phpBB/config/routing.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/phpBB/config/routing.yml b/phpBB/config/routing.yml index f6f728fa47..d8e890d063 100644 --- a/phpBB/config/routing.yml +++ b/phpBB/config/routing.yml @@ -4,6 +4,6 @@ # pattern: /foo # defaults: { _controller: foo_sevice:method } # -# The above will be accessed via app.php/foo and it will instantiate the -# "foo_service" service and call the "method" method. +# The above will be accessed via app.php?controller=foo and it will +# instantiate the "foo_service" service and call the "method" method. # From 60c0a1dd2ac2c733a670093ad440e3ba6be3be4d Mon Sep 17 00:00:00 2001 From: David King Date: Sun, 18 Nov 2012 15:51:05 -0500 Subject: [PATCH 60/67] [feature/controller] Don't use $user->lang() before container compilation PHPBB3-10864 --- phpBB/includes/di/pass/kernel_pass.php | 7 +++---- phpBB/language/en/app.php | 5 ----- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/phpBB/includes/di/pass/kernel_pass.php b/phpBB/includes/di/pass/kernel_pass.php index d186ff2767..a701ebcfa6 100644 --- a/phpBB/includes/di/pass/kernel_pass.php +++ b/phpBB/includes/di/pass/kernel_pass.php @@ -29,7 +29,6 @@ class phpbb_di_pass_kernel_pass implements CompilerPassInterface public function process(ContainerBuilder $container) { $definition = $container->getDefinition('dispatcher'); - $user = $container->get('user'); foreach ($container->findTaggedServiceIds('kernel.event_listener') as $id => $events) { @@ -39,12 +38,12 @@ class phpbb_di_pass_kernel_pass implements CompilerPassInterface if (!isset($event['event'])) { - throw new InvalidArgumentException($user->lang('NO_EVENT_ATTRIBUTE', $id)); + throw new InvalidArgumentException(sprintf('Service "%1$s" must define the "event" attribute on "kernel.event_listener" tags.', $id)); } if (!isset($event['method'])) { - throw new InvalidArgumentException($user->lang('NO_METHOD_ATTRIBUTE', $id)); + throw new InvalidArgumentException(sprintf('Service "%1$s" must define the "method" attribute on "kernel.event_listener" tags.', $id)); } $definition->addMethodCall('addListenerService', array($event['event'], array($id, $event['method']), $priority)); @@ -60,7 +59,7 @@ class phpbb_di_pass_kernel_pass implements CompilerPassInterface $interface = 'Symfony\Component\EventDispatcher\EventSubscriberInterface'; if (!$refClass->implementsInterface($interface)) { - throw new InvalidArgumentException($user->lang('SUBSCRIBER_WRONG_TYPE', $id, $interface)); + throw new InvalidArgumentException(sprintf('Service "%1$s" must implement interface "%2$s".', $id, $interface)); } $definition->addMethodCall('addSubscriberService', array($id, $class)); diff --git a/phpBB/language/en/app.php b/phpBB/language/en/app.php index 2cbeaa2cba..2d67246369 100644 --- a/phpBB/language/en/app.php +++ b/phpBB/language/en/app.php @@ -47,11 +47,6 @@ $lang = array_merge($lang, array( 'CONTROLLER_SERVICE_UNDEFINED' => 'The service for controller "%s" is not defined in ./config/services.yml.', 'CONTROLLER_RETURN_TYPE_INVALID' => 'The controller object %s must return a Symfony\Component\HttpFoundation\Response object.', - // Event Listener/Subscriber error messages - 'NO_EVENT_ATTRIBUTE' => 'Service "%1$s" must define the "event" attribute on "kernel.event_listener" tags.', - 'NO_METHOD_ATTRIBUTE' => 'Service "%1$s" must define the "method" attribute on "kernel.event_listener" tags.', - 'SUBSCRIBER_WRONG_TYPE' => 'Service "%1$s" must implement interface "%2$s".', - // Core error controller messages 'PAGE_NOT_FOUND_ERROR' => 'The page you have requested does not exist.', 'NOT_AUTHORISED_ERROR' => 'You do not have permission to access this page.', From 2f50d656481dca3c2aa28cd5035ce0d44e5c4977 Mon Sep 17 00:00:00 2001 From: David King Date: Sun, 18 Nov 2012 15:51:32 -0500 Subject: [PATCH 61/67] [feature/controller] Remove unused language strings PHPBB3-10864 --- phpBB/language/en/app.php | 6 ------ 1 file changed, 6 deletions(-) diff --git a/phpBB/language/en/app.php b/phpBB/language/en/app.php index 2d67246369..cb56015c30 100644 --- a/phpBB/language/en/app.php +++ b/phpBB/language/en/app.php @@ -46,10 +46,4 @@ $lang = array_merge($lang, array( 'CONTROLLER_SERVICE_NOT_GIVEN' => 'The controller "%s" must have a service specified in ./config/routing.yml.', 'CONTROLLER_SERVICE_UNDEFINED' => 'The service for controller "%s" is not defined in ./config/services.yml.', 'CONTROLLER_RETURN_TYPE_INVALID' => 'The controller object %s must return a Symfony\Component\HttpFoundation\Response object.', - - // Core error controller messages - 'PAGE_NOT_FOUND_ERROR' => 'The page you have requested does not exist.', - 'NOT_AUTHORISED_ERROR' => 'You do not have permission to access this page.', - 'NOT_AUTHENTICATED_ERROR' => 'You must log in to access this page.', - 'INTERNAL_SERVER_ERROR_ERROR' => 'An unknown error occured.', )); From 0f4f81b0966e29b5aaae5bf94e46260474ec0cb2 Mon Sep 17 00:00:00 2001 From: David King Date: Sun, 18 Nov 2012 15:52:35 -0500 Subject: [PATCH 62/67] [feature/controller] Create Symfony Request in new function PHPBB3-10864 --- phpBB/app.php | 6 ++---- phpBB/includes/functions.php | 39 ++++++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 4 deletions(-) diff --git a/phpBB/app.php b/phpBB/app.php index 13bf3f0be1..c085f966c8 100644 --- a/phpBB/app.php +++ b/phpBB/app.php @@ -7,8 +7,6 @@ * */ -use Symfony\Component\HttpFoundation\Request; - /** */ @@ -28,9 +26,9 @@ $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', '', false, phpbb_request_interface::GET); +$controller = $request->variable('controller', ''); $uri = '/' . $controller; -$symfony_request = Request::create($uri); +$symfony_request = phpbb_create_symfony_request($uri, $request); $http_kernel = $phpbb_container->get('http_kernel'); $response = $http_kernel->handle($symfony_request); diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php index 02a9e33f2a..820d96c9aa 100644 --- a/phpBB/includes/functions.php +++ b/phpBB/includes/functions.php @@ -7,6 +7,8 @@ * */ +use Symfony\Component\HttpFoundation\Request; + /** * @ignore */ @@ -5430,3 +5432,40 @@ function phpbb_to_numeric($input) { return ($input > PHP_INT_MAX) ? (float) $input : (int) $input; } + +/** +* Create a Symfony Request object from a given URI and phpbb_request object +* +* @param string $uri Request URI +* @param phpbb_request $request Request object +* @return Request A Symfony Request object +*/ +function phpbb_create_symfony_request($uri, phpbb_request $request) +{ + $request_method = $request->server('REQUEST_METHOD'); + $parameter_names = array(); + $parameter_names['request'] = array_merge( + $request->variable_names(phpbb_request_interface::GET), + // POST overwrites duplicated GET parameters + $request->variable_names(phpbb_request_interface::POST) + ); + $parameter_names['server'] = $request->variable_names(phpbb_request_interface::SERVER); + $parameter_names['files'] = $request->variable_names(phpbb_request_interface::FILES); + $parameter_names['cookie'] = $request->variable_names(phpbb_request_interface::COOKIE); + + $parameters = array( + 'request' => array(), + 'cookie' => array(), + 'files' => array(), + 'server' => array(), + ); + foreach ($parameter_names as $type => $names) + { + foreach ($names as $name) + { + $parameters[$type][$name] = $request->variable($name, ''); + } + } + + return Request::create($uri, $request_method, $parameters['request'], $parameters['cookie'], $parameters['files'], $parameters['server']); +} From e2bf66d0658ae7d7bb253083b73d5769c117746a Mon Sep 17 00:00:00 2001 From: David King Date: Sun, 18 Nov 2012 15:58:47 -0500 Subject: [PATCH 63/67] [feature/controller] Add documentation about input being HTML-escaped PHPBB3-10864 --- phpBB/includes/functions.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php index 820d96c9aa..cdc05ca649 100644 --- a/phpBB/includes/functions.php +++ b/phpBB/includes/functions.php @@ -5436,6 +5436,9 @@ function phpbb_to_numeric($input) /** * Create a Symfony Request object from a given URI and phpbb_request object * +* Note that everything passed into the Request object has already been HTML +* escaped by the phpbb_request object. +* * @param string $uri Request URI * @param phpbb_request $request Request object * @return Request A Symfony Request object From 30043502814cd42d824dc1d6bcb25bebc60adbed Mon Sep 17 00:00:00 2001 From: David King Date: Mon, 19 Nov 2012 11:47:42 -0500 Subject: [PATCH 64/67] [feature/controller] Correctly create Symfony object from globals PHPBB3-10864 --- phpBB/includes/functions.php | 62 ++++++++++++++++++++---------------- 1 file changed, 34 insertions(+), 28 deletions(-) diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php index cdc05ca649..ee147969f9 100644 --- a/phpBB/includes/functions.php +++ b/phpBB/includes/functions.php @@ -5434,41 +5434,47 @@ function phpbb_to_numeric($input) } /** -* Create a Symfony Request object from a given URI and phpbb_request object +* Create a Symfony Request object from phpbb_request object * -* Note that everything passed into the Request object has already been HTML -* escaped by the phpbb_request object. -* -* @param string $uri Request URI * @param phpbb_request $request Request object * @return Request A Symfony Request object */ -function phpbb_create_symfony_request($uri, phpbb_request $request) +function phpbb_create_symfony_request(phpbb_request $request) { - $request_method = $request->server('REQUEST_METHOD'); - $parameter_names = array(); - $parameter_names['request'] = array_merge( - $request->variable_names(phpbb_request_interface::GET), - // POST overwrites duplicated GET parameters - $request->variable_names(phpbb_request_interface::POST) - ); - $parameter_names['server'] = $request->variable_names(phpbb_request_interface::SERVER); - $parameter_names['files'] = $request->variable_names(phpbb_request_interface::FILES); - $parameter_names['cookie'] = $request->variable_names(phpbb_request_interface::COOKIE); + // This function is meant to sanitize the global input arrays + $sanitizer = function(&$value, $key) { + $type_cast_helper = new phpbb_request_type_cast_helper(); + $type_cast_helper->set_var($value, $value, gettype($value), true); + }; - $parameters = array( - 'request' => array(), - 'cookie' => array(), - 'files' => array(), - 'server' => array(), - ); - foreach ($parameter_names as $type => $names) + // We need to re-enable the super globals so we can access them here + $request->enable_super_globals(); + $get_parameters = $_GET; + $post_parameters = $_POST; + $server_parameters = $_SERVER; + $files_parameters = $_FILES; + $cookie_parameters = $_COOKIE; + // And now disable them again for security + $request->disable_super_globals(); + + array_walk_recursive($get_parameters, $sanitizer); + array_walk_recursive($post_parameters, $sanitizer); + + // 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', ''); + $path_info = '/' . $controller; + $request_uri = $server_parameters['REQUEST_URI']; + + // Remove the query string from REQUEST_URI + if ($pos = strpos($request_uri, '?')) { - foreach ($names as $name) - { - $parameters[$type][$name] = $request->variable($name, ''); - } + $request_uri = substr($request_uri, 0, $pos); } - return Request::create($uri, $request_method, $parameters['request'], $parameters['cookie'], $parameters['files'], $parameters['server']); + // Add the path info (i.e. controller route) to the REQUEST_URI + $server_parameters['REQUEST_URI'] = $request_uri . $path_info; + $server_parameters['SCRIPT_NAME'] = ''; + + return new Request($get_parameters, $post_parameters, array(), $cookie_parameters, $files_parameters, $server_parameters); } From f8614bfc84ba9b9cc814b8f78e343005620f18f8 Mon Sep 17 00:00:00 2001 From: David King Date: Mon, 19 Nov 2012 12:37:20 -0500 Subject: [PATCH 65/67] [feature/controller] Check for proper status codes from controllers PHPBB3-10864 --- phpBB/app.php | 7 +------ .../includes/event/kernel_exception_subscriber.php | 9 +++++++-- tests/functional/extension_controller_test.php | 13 ++++++++++++- .../fixtures/ext/foo/bar/config/routing.yml | 4 ++++ .../fixtures/ext/foo/bar/controller/controller.php | 5 +++++ 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'); + } } From 01ec6085939d74e6a37c3ef041434db1c4b8f3e4 Mon Sep 17 00:00:00 2001 From: David King Date: Mon, 19 Nov 2012 12:55:15 -0500 Subject: [PATCH 66/67] [feature/controller] Fix comments, check against more general HttpException PHPBB3-10864 --- phpBB/includes/event/kernel_exception_subscriber.php | 4 ++-- tests/functional/extension_controller_test.php | 7 ++++--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/phpBB/includes/event/kernel_exception_subscriber.php b/phpBB/includes/event/kernel_exception_subscriber.php index e843df2e68..f90989a74c 100644 --- a/phpBB/includes/event/kernel_exception_subscriber.php +++ b/phpBB/includes/event/kernel_exception_subscriber.php @@ -18,7 +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\HttpKernel\Exception\HttpException; use Symfony\Component\HttpFoundation\Response; class phpbb_event_kernel_exception_subscriber implements EventSubscriberInterface @@ -71,7 +71,7 @@ class phpbb_event_kernel_exception_subscriber implements EventSubscriberInterfac page_footer(true, false, false); - $status_code = $exception instanceof NotFoundHttpException ? $exception->getStatusCode() : 500; + $status_code = $exception instanceof HttpException ? $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 c7b585354e..f28b321942 100644 --- a/tests/functional/extension_controller_test.php +++ b/tests/functional/extension_controller_test.php @@ -121,7 +121,10 @@ class phpbb_functional_extension_controller_test extends phpbb_functional_test_c $this->phpbb_extension_manager->purge('foo/bar'); } - public function test_exception_thrown_status_code() + /** + * Check the status code resulting from an exception thrown by a controller + */ + public function test_exception_should_result_in_500_status_code() { $this->phpbb_extension_manager->enable('foo/bar'); $crawler = $this->request('GET', 'app.php?controller=foo/exception'); @@ -142,8 +145,6 @@ 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()); } From 305f41cf1a540984fd7a71b61a601b1794e3bd04 Mon Sep 17 00:00:00 2001 From: David King Date: Mon, 19 Nov 2012 13:16:55 -0500 Subject: [PATCH 67/67] [feature/controller] Fix misnamed route for functional test PHPBB3-10864 --- tests/functional/fixtures/ext/foo/bar/config/routing.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/functional/fixtures/ext/foo/bar/config/routing.yml b/tests/functional/fixtures/ext/foo/bar/config/routing.yml index 7eb604f746..09a30a8c67 100644 --- a/tests/functional/fixtures/ext/foo/bar/config/routing.yml +++ b/tests/functional/fixtures/ext/foo/bar/config/routing.yml @@ -11,5 +11,5 @@ foo_template_controller: defaults: { _controller: foo_bar.controller:template } foo_exception_controller: - pattern: /foo/foo_exception + pattern: /foo/exception defaults: { _controller: foo_bar.controller:exception }