Issue #2449143 by damiankloip, tedbow, Wim Leers, dawehner, Pavan B S, Tybor, effulgentsia, larowlan: REST views specify HTML as a possible request format, so if there is a "regular" HTML view on the same path, it will serve JSON

8.5.x
Lee Rowlands 2017-10-11 15:52:34 +10:00
parent 342b49212e
commit 4997192be0
No known key found for this signature in database
GPG Key ID: 2B829A3DF9204DC4
9 changed files with 225 additions and 75 deletions

View File

@ -70,7 +70,7 @@ class RestExport extends PathPluginBase implements ResponseDisplayPluginInterfac
*
* @var string
*/
protected $mimeType;
protected $mimeType = 'application/json';
/**
* The renderer.
@ -93,6 +93,13 @@ class RestExport extends PathPluginBase implements ResponseDisplayPluginInterfac
*/
protected $authenticationProviders;
/**
* The serialization format providers, keyed by format.
*
* @var string[]
*/
protected $formatProviders;
/**
* Constructs a RestExport object.
*
@ -110,12 +117,15 @@ class RestExport extends PathPluginBase implements ResponseDisplayPluginInterfac
* The renderer.
* @param string[] $authentication_providers
* The authentication providers, keyed by ID.
* @param string[] $serializer_format_providers
* The serialization format providers, keyed by format.
*/
public function __construct(array $configuration, $plugin_id, $plugin_definition, RouteProviderInterface $route_provider, StateInterface $state, RendererInterface $renderer, array $authentication_providers) {
public function __construct(array $configuration, $plugin_id, $plugin_definition, RouteProviderInterface $route_provider, StateInterface $state, RendererInterface $renderer, array $authentication_providers, array $serializer_format_providers) {
parent::__construct($configuration, $plugin_id, $plugin_definition, $route_provider, $state);
$this->renderer = $renderer;
$this->authenticationProviders = $authentication_providers;
$this->formatProviders = $serializer_format_providers;
}
/**
@ -129,8 +139,8 @@ class RestExport extends PathPluginBase implements ResponseDisplayPluginInterfac
$container->get('router.route_provider'),
$container->get('state'),
$container->get('renderer'),
$container->getParameter('authentication_providers')
$container->getParameter('authentication_providers'),
$container->getParameter('serializer.format_providers')
);
}
/**
@ -139,21 +149,22 @@ class RestExport extends PathPluginBase implements ResponseDisplayPluginInterfac
public function initDisplay(ViewExecutable $view, array &$display, array &$options = NULL) {
parent::initDisplay($view, $display, $options);
$request_content_type = $this->view->getRequest()->getRequestFormat();
// Only use the requested content type if it's not 'html'. If it is then
// default to 'json' to aid debugging.
// @todo Remove the need for this when we have better content negotiation.
if ($request_content_type != 'html') {
$this->setContentType($request_content_type);
}
// If the requested content type is 'html' and the default 'json' is not
// selected as a format option in the view display, fallback to the first
// format in the array.
elseif (!empty($options['style']['options']['formats']) && !isset($options['style']['options']['formats'][$this->getContentType()])) {
$this->setContentType(reset($options['style']['options']['formats']));
// If the default 'json' format is not selected as a format option in the
// view display, fallback to the first format available for the default.
if (!empty($options['style']['options']['formats']) && !isset($options['style']['options']['formats'][$this->getContentType()])) {
$default_format = reset($options['style']['options']['formats']);
$this->setContentType($default_format);
}
$this->setMimeType($this->view->getRequest()->getMimeType($this->contentType));
// Only use the requested content type if it's not 'html'. This allows
// still falling back to the default for things like views preview.
$request_content_type = $this->view->getRequest()->getRequestFormat();
if ($request_content_type !== 'html') {
$this->setContentType($request_content_type);
}
$this->setMimeType($this->view->getRequest()->getMimeType($this->getContentType()));
}
/**
@ -327,17 +338,21 @@ class RestExport extends PathPluginBase implements ResponseDisplayPluginInterfac
if ($route = $collection->get("view.$view_id.$display_id")) {
$style_plugin = $this->getPlugin('style');
// REST exports should only respond to get methods.
// REST exports should only respond to GET methods.
$route->setMethods(['GET']);
// Format as a string using pipes as a delimiter.
if ($formats = $style_plugin->getFormats()) {
// Allow a REST Export View to be returned with an HTML-only accept
// format. That allows browsers or other non-compliant systems to access
// the view, as it is unlikely to have a conflicting HTML representation
// anyway.
$route->setRequirement('_format', implode('|', $formats + ['html']));
$formats = $style_plugin->getFormats();
// If there are no configured formats, add all formats that serialization
// is known to support.
if (!$formats) {
$formats = $this->getFormatOptions();
}
// Format as a string using pipes as a delimiter.
$route->setRequirement('_format', implode('|', $formats));
// Add authentication to the route if it was set. If no authentication was
// set, the default authentication will be used, which is cookie based by
// default.
@ -421,15 +436,15 @@ class RestExport extends PathPluginBase implements ResponseDisplayPluginInterfac
$build['#suffix'] = '</pre>';
unset($build['#markup']);
}
elseif ($this->view->getRequest()->getFormat($this->view->element['#content_type']) !== 'html') {
// This display plugin is primarily for returning non-HTML formats.
// However, we still invoke the renderer to collect cacheability metadata.
// Because the renderer is designed for HTML rendering, it filters
// #markup for XSS unless it is already known to be safe, but that filter
// only works for HTML. Therefore, we mark the contents as safe to bypass
// the filter. So long as we are returning this in a non-HTML response
// (checked above), this is safe, because an XSS attack only works when
// executed by an HTML agent.
else {
// This display plugin is for returning non-HTML formats. However, we
// still invoke the renderer to collect cacheability metadata. Because the
// renderer is designed for HTML rendering, it filters #markup for XSS
// unless it is already known to be safe, but that filter only works for
// HTML. Therefore, we mark the contents as safe to bypass the filter. So
// long as we are returning this in a non-HTML response,
// this is safe, because an XSS attack only works when executed by an HTML
// agent.
// @todo Decide how to support non-HTML in the render API in
// https://www.drupal.org/node/2501313.
$build['#markup'] = ViewsRenderPipelineMarkup::create($build['#markup']);
@ -465,4 +480,15 @@ class RestExport extends PathPluginBase implements ResponseDisplayPluginInterfac
return $dependencies;
}
/**
* Returns an array of format options.
*
* @return string[]
* An array of format options. Both key and value are the same.
*/
protected function getFormatOptions() {
$formats = array_keys($this->formatProviders);
return array_combine($formats, $formats);
}
}

View File

@ -0,0 +1,70 @@
langcode: en
status: true
dependencies:
module:
- rest
- user
id: test_serializer_shared_path
label: 'Test serializer shared path'
module: rest
description: ''
tag: ''
base_table: entity_test
base_field: id
core: 8.x
display:
default:
display_plugin: default
id: default
display_title: Master
position: null
display_options:
access:
type: perm
options:
perm: 'access content'
cache:
type: tag
query:
type: views_query
exposed_form:
type: basic
style:
type: serializer
row:
type: data_entity
sorts:
id:
id: standard
table: entity_test
field: id
order: DESC
plugin_id: date
entity_type: entity_test
entity_field: id
title: 'Test serialize'
arguments: { }
rest_export_1:
display_plugin: rest_export
id: rest_export_1
display_title: serializer
position: null
display_options:
defaults:
access: false
path: test/serialize/shared
page_1:
display_plugin: page
id: page_1
display_title: page
position: null
display_options:
defaults:
access: false
style: false
row: false
style:
type: default
row:
type: entity:entity_test
path: test/serialize/shared

View File

@ -43,7 +43,7 @@ class StyleSerializerTest extends ViewTestBase {
*
* @var array
*/
public static $testViews = ['test_serializer_display_field', 'test_serializer_display_entity', 'test_serializer_display_entity_translated', 'test_serializer_node_display_field', 'test_serializer_node_exposed_filter'];
public static $testViews = ['test_serializer_display_field', 'test_serializer_display_entity', 'test_serializer_display_entity_translated', 'test_serializer_node_display_field', 'test_serializer_node_exposed_filter', 'test_serializer_shared_path'];
/**
* A user with administrative privileges to look at test entity and configure views.
@ -85,6 +85,9 @@ class StyleSerializerTest extends ViewTestBase {
$url = $this->buildUrl('test/serialize/auth_with_perm');
$response = \Drupal::httpClient()->get($url, [
'auth' => [$this->adminUser->getUsername(), $this->adminUser->pass_raw],
'query' => [
'_format' => 'json',
],
]);
// Ensure that any changes to variables in the other thread are picked up.
@ -134,7 +137,7 @@ class StyleSerializerTest extends ViewTestBase {
$this->assertIdentical($actual_json, (string) drupal_render_root($output), 'The expected JSON preview output was found.');
// Test a 403 callback.
$this->drupalGet('test/serialize/denied');
$this->drupalGet('test/serialize/denied', ['query' => ['_format' => 'json']]);
$this->assertResponse(403);
// Test the entity rows.
@ -169,7 +172,7 @@ class StyleSerializerTest extends ViewTestBase {
$this->assertIdentical($actual_json, $expected, 'The expected HAL output was found.');
$this->assertCacheTags($expected_cache_tags);
// Change the default format to xml.
// Change the format to xml.
$view->setDisplay('rest_export_1');
$view->getDisplay()->setOption('style', [
'type' => 'serializer',
@ -207,6 +210,28 @@ class StyleSerializerTest extends ViewTestBase {
$this->assertSame(trim($expected), $actual_xml);
}
/**
* Verifies REST export views work on the same path as a page display.
*/
public function testSharedPagePath() {
// Test with no format as well as html explicitly.
$this->drupalGet('test/serialize/shared');
$this->assertResponse(200);
$this->assertHeader('content-type', 'text/html; charset=UTF-8');
$this->drupalGet('test/serialize/shared', ['query' => ['_format' => 'html']]);
$this->assertResponse(200);
$this->assertHeader('content-type', 'text/html; charset=UTF-8');
$this->drupalGet('test/serialize/shared', ['query' => ['_format' => 'json']]);
$this->assertResponse(200);
$this->assertHeader('content-type', 'application/json');
$this->drupalGet('test/serialize/shared', ['query' => ['_format' => 'xml']]);
$this->assertResponse(200);
$this->assertHeader('content-type', 'text/xml; charset=UTF-8');
}
/**
* Verifies site maintenance mode functionality.
*/
@ -336,6 +361,11 @@ class StyleSerializerTest extends ViewTestBase {
$style_options = 'admin/structure/views/nojs/display/test_serializer_display_field/rest_export_1/style_options';
// Test with no format.
$this->drupalGet('test/serialize/field');
$this->assertHeader('content-type', 'text/html; charset=UTF-8');
$this->assertResponse(406, 'A 406 response was returned when no format was requested.');
// Select only 'xml' as an accepted format.
$this->drupalPostForm($style_options, ['style_options[formats][xml]' => 'xml'], t('Apply'));
$this->drupalPostForm(NULL, [], t('Save'));
@ -353,37 +383,27 @@ class StyleSerializerTest extends ViewTestBase {
$this->drupalPostForm($style_options, ['style_options[formats][json]' => 'json'], t('Apply'));
$this->drupalPostForm(NULL, [], t('Save'));
// Should return a 200.
// @todo This should be fixed when we have better content negotiation.
$this->drupalGet('test/serialize/field');
$this->assertHeader('content-type', 'application/json');
$this->assertResponse(200, 'A 200 response was returned when any format was requested.');
// Should return a 200. Emulates a sample Firefox header.
// Should return a 406. Emulates a sample Firefox header.
$this->drupalGet('test/serialize/field', [], ['Accept: text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8']);
$this->assertHeader('content-type', 'application/json');
$this->assertResponse(200, 'A 200 response was returned when a browser accept header was requested.');
$this->assertHeader('content-type', 'text/html; charset=UTF-8');
$this->assertResponse(406, 'A 406 response was returned when a browser accept header was requested.');
// Should return a 406.
$this->drupalGet('test/serialize/field', ['query' => ['_format' => 'html']]);
$this->assertHeader('content-type', 'text/html; charset=UTF-8');
$this->assertResponse(406, 'A 406 response was returned when HTML was requested.');
// Should return a 200.
$this->drupalGet('test/serialize/field', ['query' => ['_format' => 'json']]);
$this->assertHeader('content-type', 'application/json');
$this->assertResponse(200, 'A 200 response was returned when JSON was requested.');
$headers = $this->drupalGetHeaders();
$this->assertEqual($headers['Content-Type'], ['application/json'], 'The header Content-type is correct.');
// Should return a 200.
$this->drupalGet('test/serialize/field', ['query' => ['_format' => 'xml']]);
$this->assertHeader('content-type', 'text/xml; charset=UTF-8');
$this->assertResponse(200, 'A 200 response was returned when XML was requested');
$headers = $this->drupalGetHeaders();
$this->assertSame(['text/xml; charset=UTF-8'], $headers['Content-Type']);
// Should return a 406.
$this->drupalGet('test/serialize/field', ['query' => ['_format' => 'html']]);
// We want to show the first format by default, see
// \Drupal\rest\Plugin\views\style\Serializer::render.
$this->assertHeader('content-type', 'application/json');
$this->assertResponse(200, 'A 200 response was returned when HTML was requested.');
// Now configure now format, so all of them should be allowed.
// Now configure no format, so both serialization formats should be allowed.
$this->drupalPostForm($style_options, ['style_options[formats][json]' => '0', 'style_options[formats][xml]' => '0'], t('Apply'));
// Should return a 200.
@ -394,12 +414,11 @@ class StyleSerializerTest extends ViewTestBase {
$this->drupalGet('test/serialize/field', ['query' => ['_format' => 'xml']]);
$this->assertHeader('content-type', 'text/xml; charset=UTF-8');
$this->assertResponse(200, 'A 200 response was returned when XML was requested');
// Should return a 200.
// Should return a 406 for HTML still.
$this->drupalGet('test/serialize/field', ['query' => ['_format' => 'html']]);
// We want to show the first format by default, see
// \Drupal\rest\Plugin\views\style\Serializer::render.
$this->assertHeader('content-type', 'application/json');
$this->assertResponse(200, 'A 200 response was returned when HTML was requested.');
$this->assertHeader('content-type', 'text/html; charset=UTF-8');
$this->assertResponse(406, 'A 406 response was returned when HTML was requested.');
}
/**
@ -598,7 +617,7 @@ class StyleSerializerTest extends ViewTestBase {
// Check if we receive the expected result.
$result = $this->xpath('//div[@id="views-live-preview"]/pre');
$json_preview = $result[0]->getText();
$this->assertSame($json_preview, $this->drupalGet('test/serialize/field'), 'The expected JSON preview output was found.');
$this->assertSame($json_preview, $this->drupalGet('test/serialize/field', ['query' => ['_format' => 'json']]), 'The expected JSON preview output was found.');
}
/**

View File

@ -84,6 +84,8 @@ class CollectRoutesTest extends UnitTestCase {
->method('getSortedProviders')
->will($this->returnValue(['basic_auth' => 'data', 'cookie' => 'data']));
$container->setParameter('serializer.format_providers', ['json']);
\Drupal::setContainer($container);
$this->restExport = RestExport::create($container, [], "test_routes", []);

View File

@ -817,7 +817,7 @@ abstract class WebTestBase extends TestBase {
* The result of the request.
*/
protected function drupalGetWithFormat($path, $format, array $options = [], array $headers = []) {
$options += ['query' => ['_format' => $format]];
$options = array_merge_recursive(['query' => ['_format' => $format]], $options);
return $this->drupalGet($path, $options, $headers);
}

View File

@ -84,6 +84,18 @@ class Page extends PathPluginBase {
);
}
/**
* {@inheritdoc}
*/
protected function getRoute($view_id, $display_id) {
$route = parent::getRoute($view_id, $display_id);
// Explicitly set HTML as the format for Page displays.
$route->setRequirement('_format', 'html');
return $route;
}
/**
* Sets the current page views render array.
*

View File

@ -163,8 +163,9 @@ class BasicTest extends WizardTestBase {
$this->drupalPostForm('admin/structure/views/add', $view4, t('Save and edit'));
$this->assertRaw(t('The view %view has been saved.', ['%view' => $view4['label']]));
// Check that the REST export path works.
$this->drupalGet($view4['rest_export[path]']);
// Check that the REST export path works. JSON will work, as all core
// formats will be allowed. JSON and XML by default.
$this->drupalGet($view4['rest_export[path]'], ['query' => ['_format' => 'json']]);
$this->assertResponse(200);
$data = Json::decode($this->getSession()->getPage()->getContent());
$this->assertEqual(count($data), 1, 'Only the node of type page is exported.');

View File

@ -15,6 +15,7 @@ use Drupal\user\SharedTempStoreFactory;
use Drupal\views\Views;
use Symfony\Component\DependencyInjection\ContainerInterface;
use Symfony\Component\HttpFoundation\RequestStack;
use Symfony\Component\HttpKernel\Exception\NotAcceptableHttpException;
/**
* Form controller for the Views edit form.
@ -417,15 +418,25 @@ class ViewEditForm extends ViewFormBase {
// path.
elseif ($view->status() && $view->getExecutable()->displayHandlers->get($display['id'])->hasPath()) {
$path = $view->getExecutable()->displayHandlers->get($display['id'])->getPath();
if ($path && (strpos($path, '%') === FALSE)) {
if (!parse_url($path, PHP_URL_SCHEME)) {
// @todo Views should expect and store a leading /. See:
// https://www.drupal.org/node/2423913
$url = Url::fromUserInput('/' . ltrim($path, '/'));
// Wrap this in a try/catch as trying to generate links to some
// routes may throw a NotAcceptableHttpException if they do not
// respond to HTML, such as RESTExports.
try {
if (!parse_url($path, PHP_URL_SCHEME)) {
// @todo Views should expect and store a leading /. See:
// https://www.drupal.org/node/2423913
$url = Url::fromUserInput('/' . ltrim($path, '/'));
}
else {
$url = Url::fromUri("base:$path");
}
}
else {
$url = Url::fromUri("base:$path");
catch (NotAcceptableHttpException $e) {
$url = '/' . $path;
}
$build['top']['actions']['path'] = [
'#type' => 'link',
'#title' => $this->t('View @display_title', ['@display_title' => $display_title]),

View File

@ -9,6 +9,7 @@ use Drupal\Core\Entity\EntityStorageInterface;
use Drupal\Core\Entity\EntityTypeInterface;
use Drupal\Core\Url;
use Symfony\Component\DependencyInjection\ContainerInterface;
use Symfony\Component\HttpKernel\Exception\NotAcceptableHttpException;
/**
* Defines a class to build a listing of view entities.
@ -255,9 +256,17 @@ class ViewListBuilder extends ConfigEntityListBuilder {
if ($display->hasPath()) {
$path = $display->getPath();
if ($view->status() && strpos($path, '%') === FALSE) {
// @todo Views should expect and store a leading /. See:
// https://www.drupal.org/node/2423913
$rendered_path = \Drupal::l('/' . $path, Url::fromUserInput('/' . $path));
// Wrap this in a try/catch as trying to generate links to some
// routes may throw a NotAcceptableHttpException if they do not
// respond to HTML, such as RESTExports.
try {
// @todo Views should expect and store a leading /. See:
// https://www.drupal.org/node/2423913
$rendered_path = \Drupal::l('/' . $path, Url::fromUserInput('/' . $path));
}
catch (NotAcceptableHttpException $e) {
$rendered_path = '/' . $path;
}
}
else {
$rendered_path = '/' . $path;