Commit de2c0295 authored by amateescu's avatar amateescu
Browse files

Issue #2783387 by amateescu: Rethink the way we store and use paths.

parent 68beef53
......@@ -245,7 +245,7 @@ abstract class XmlSitemapTestBase extends WebTestBase {
protected function assertRawSitemapLinks() {
$links = func_get_args();
foreach ($links as $link) {
$path = Url::fromUri('base://' . $link['loc'], array('language' => xmlsitemap_language_load($link['language']), 'absolute' => TRUE))->toString();
$path = Url::fromUri('internal:' . $link['loc'], array('language' => xmlsitemap_language_load($link['language']), 'absolute' => TRUE))->toString();
$this->assertRaw($link['loc'], t('Link %path found in the sitemap.', array('%path' => $path)));
}
}
......@@ -253,7 +253,7 @@ abstract class XmlSitemapTestBase extends WebTestBase {
protected function assertNoRawSitemapLinks() {
$links = func_get_args();
foreach ($links as $link) {
$path = Url::fromUri('base://' . $link['loc'], array('language' => xmlsitemap_language_load($link['language']), 'absolute' => TRUE))->toString();
$path = Url::fromUri('internal:' . $link['loc'], array('language' => xmlsitemap_language_load($link['language']), 'absolute' => TRUE))->toString();
$this->assertNoRaw($link['loc'], t('Link %path not found in the sitemap.', array('%path' => $path)));
}
}
......@@ -270,7 +270,7 @@ abstract class XmlSitemapTestBase extends WebTestBase {
);
// Make the default path easier to read than a random string.
$link += array('loc' => $link['type'] . '-' . $link['id']);
$link += array('loc' => '/' . $link['type'] . '-' . $link['id']);
$last_id = max($last_id, $link['id']) + 1;
$this->linkStorage->save($link);
......
......@@ -195,14 +195,14 @@ class XmlSitemapUnitTest extends XmlSitemapTestBase {
*/
public function testLinkDelete() {
// Add our testing data.
$link1 = $this->addSitemapLink(array('loc' => 'testing1', 'status' => 0));
$link2 = $this->addSitemapLink(array('loc' => 'testing1', 'status' => 1));
$link1 = $this->addSitemapLink(array('loc' => '/testing1', 'status' => 0));
$link2 = $this->addSitemapLink(array('loc' => '/testing1', 'status' => 1));
$link3 = $this->addSitemapLink(array('status' => 0));
$this->state->set('xmlsitemap_regenerate_needed', FALSE);
// Test delete multiple links.
// Test that the regenerate flag is set when visible links are deleted.
$deleted = $this->linkStorage->deleteMultiple(array('loc' => 'testing1'));
$deleted = $this->linkStorage->deleteMultiple(array('loc' => '/testing1'));
$this->assertEqual($deleted, 2);
$this->assertFalse($this->linkStorage->load($link1['type'], $link1['id']));
$this->assertFalse($this->linkStorage->load($link2['type'], $link2['id']));
......@@ -264,8 +264,8 @@ class XmlSitemapUnitTest extends XmlSitemapTestBase {
*/
public function testDuplicatePaths() {
$this->drupalLogin($this->admin_user);
$link1 = $this->addSitemapLink(array('loc' => 'duplicate'));
$link2 = $this->addSitemapLink(array('loc' => 'duplicate'));
$link1 = $this->addSitemapLink(array('loc' => '/duplicate'));
$link2 = $this->addSitemapLink(array('loc' => '/duplicate'));
$this->regenerateSitemap();
$this->drupalGetSitemap();
$this->assertUniqueText('duplicate');
......@@ -279,7 +279,7 @@ class XmlSitemapUnitTest extends XmlSitemapTestBase {
$this->config->set('minimum_lifetime', 300)->save();
$this->regenerateSitemap();
$link = $this->addSitemapLink(array('loc' => 'lifetime-test'));
$link = $this->addSitemapLink(array('loc' => '/lifetime-test'));
$this->cronRun();
$this->drupalGetSitemap();
$this->assertResponse(200);
......
......@@ -201,7 +201,7 @@ class XmlSitemapGenerator implements XmlSitemapGeneratorInterface {
$url_options = $sitemap->uri['options'];
$url_options += array(
'absolute' => TRUE,
'xmlsitemap_base_url' => $this->state->get('xmlsitemap_base_url'),
'base_url' => rtrim($this->state->get('xmlsitemap_base_url'), '/'),
'language' => $this->languageManager->getDefaultLanguage(),
'alias' => $this->config->get('prefetch_aliases'),
);
......@@ -225,20 +225,17 @@ class XmlSitemapGenerator implements XmlSitemapGeneratorInterface {
while ($link = $links->fetchAssoc()) {
$link['language'] = $link['language'] != LanguageInterface::LANGCODE_NOT_SPECIFIED ? xmlsitemap_language_load($link['language']) : $url_options['language'];
if ($url_options['alias']) {
if (!empty($link['loc']) && $url_options['alias']) {
$link['loc'] = $this->getPathAlias($link['loc'], $link['language']->getId());
}
if ($url_options['base_url']) {
$link['loc'] = rtrim($url_options['base_url'], '/') . '/' . ltrim($link['loc'], '/');
}
$link_options = array(
'language' => $link['language'],
'xmlsitemap_link' => $link,
'xmlsitemap_sitemap' => $sitemap,
);
// @todo Add a separate hook_xmlsitemap_link_url_alter() here?
$link['loc'] = empty($link['loc']) ? '<front>' : $link['loc'];
$link_url = Url::fromUri($link['loc'], [], $link_options + $url_options)->toString();
$link['loc'] = empty($link['loc']) ? '/' : $link['loc'];
$link_url = Url::fromUri('internal:' . $link['loc'], $link_options + $url_options)->toString();
// Skip this link if it was a duplicate of the last one.
// @todo Figure out a way to do this before generation so we can report
......
......@@ -288,7 +288,7 @@ function xmlsitemap_install() {
->fields(array(
'type' => 'frontpage',
'id' => 0,
'loc' => '',
'loc' => '/',
'priority' => \Drupal::config('xmlsitemap.settings')->get('frontpage_priority'),
'changefreq' => \Drupal::config('xmlsitemap.settings')->get('frontpage_changefreq'),
'language' => LanguageInterface::LANGCODE_NOT_SPECIFIED,
......@@ -346,7 +346,17 @@ function xmlsitemap_uninstall() {
* Change the primary key of the 'xmlsitemap' table to include the language.
*/
function xmlsitemap_update_8001() {
Database::getConnection()->schema()->dropPrimaryKey('xmlsitemap');
Database::getConnection()->schema()->addPrimaryKey('xmlsitemap', array('id', 'type', 'language'));
Database::getConnection()->schema()->dropIndex('xmlsitemap', 'language');
\Drupal::database()->schema()->dropPrimaryKey('xmlsitemap');
\Drupal::database()->schema()->addPrimaryKey('xmlsitemap', array('id', 'type', 'language'));
\Drupal::database()->schema()->dropIndex('xmlsitemap', 'language');
}
/**
* Update the path of the frontpage link.
*/
function xmlsitemap_update_8002() {
\Drupal::database()->update('xmlsitemap')
->fields(['loc' => '/'])
->condition('type', 'frontpage')
->execute();
}
......@@ -4,6 +4,7 @@ namespace Drupal\xmlsitemap_custom\Controller;
use Drupal\Component\Utility\Unicode;
use Drupal\Core\Controller\ControllerBase;
use Drupal\Core\Link;
use Drupal\Core\Url;
/**
......@@ -43,7 +44,7 @@ class XmlSitemapCustomListController extends ControllerBase {
foreach ($result as $link) {
$language = $this->languageManager()->getLanguage($link->language);
$row = array();
$row['loc'] = $this->l($link->loc, Url::fromUri('base://' . $link->loc));
$row['loc'] = Link::fromTextAndUrl($link->loc, Url::fromUri('internal:' . $link->loc));
$row['priority'] = number_format($link->priority, 1);
$row['changefreq'] = $link->changefreq ? Unicode::ucfirst(xmlsitemap_get_changefreq($link->changefreq)) : t('None');
if (isset($header['language'])) {
......
......@@ -2,6 +2,7 @@
namespace Drupal\xmlsitemap_custom\Form;
use Drupal\Component\Utility\Unicode;
use Drupal\Core\Database\Connection;
use Drupal\Core\Form\FormBase;
use Drupal\Core\Http\ClientFactory;
......@@ -147,8 +148,9 @@ class XmlSitemapCustomAddForm extends FormBase {
$form['loc'] = array(
'#type' => 'textfield',
'#title' => $this->t('Path to link'),
'#field_prefix' => Url::fromRoute('<front>', [], array('absolute' => TRUE)),
'#field_prefix' => rtrim(Url::fromRoute('<front>', [], array('absolute' => TRUE))->toString(), '/'),
'#default_value' => $link['loc'] ? $this->aliasManager->getPathByAlias($link['loc'], $link['language']) : '',
'#description' => $this->t('Use a relative path with a slash in front. For example, "/about".'),
'#required' => TRUE,
'#size' => 30,
);
......@@ -198,6 +200,11 @@ class XmlSitemapCustomAddForm extends FormBase {
public function validateForm(array &$form, FormStateInterface $form_state) {
$link = $form_state->getValues();
if (Unicode::substr($link['loc'], 0, 1) !== '/') {
$form_state->setErrorByName('loc', $this->t('The path should start with /.'));
return;
}
// Make sure we trim and normalize the path first.
$link['loc'] = trim($link['loc']);
$link['loc'] = $this->aliasManager->getPathByAlias($link['loc'], $link['language']);
......@@ -217,7 +224,7 @@ class XmlSitemapCustomAddForm extends FormBase {
}
try {
$client = $this->httpClientFactory->fromOptions(['config/curl', array(CURLOPT_FOLLOWLOCATION => FALSE)]);
$client->get(Url::fromUserInput('/' . ltrim($link['loc'], '/'), ['absolute' => TRUE])->toString());
$client->get(Url::fromUserInput($link['loc'], ['absolute' => TRUE])->toString());
}
catch (ClientException $e) {
$form_state->setErrorByName('loc', $this->t('The custom link @link is either invalid or it cannot be accessed by anonymous users.', array('@link' => $link['loc'])));
......
......@@ -2,6 +2,7 @@
namespace Drupal\xmlsitemap_custom\Form;
use Drupal\Component\Utility\Unicode;
use Drupal\Core\Form\FormBase;
use Drupal\Core\Http\ClientFactory;
use Drupal\Core\Language\LanguageInterface;
......@@ -118,8 +119,9 @@ class XmlSitemapCustomEditForm extends FormBase {
$form['loc'] = array(
'#type' => 'textfield',
'#title' => $this->t('Path to link'),
'#field_prefix' => Url::fromRoute('<front>', [], array('absolute' => TRUE)),
'#field_prefix' => rtrim(Url::fromRoute('<front>', [], array('absolute' => TRUE))->toString(), '/'),
'#default_value' => $this->custom_link['loc'],
'#description' => $this->t('Use a relative path with a slash in front. For example, "/about".'),
'#required' => TRUE,
'#size' => 30,
);
......@@ -168,12 +170,20 @@ class XmlSitemapCustomEditForm extends FormBase {
*/
public function validateForm(array &$form, FormStateInterface $form_state) {
$link = $form_state->getValues();
if (Unicode::substr($link['loc'], 0, 1) !== '/') {
$form_state->setErrorByName('loc', $this->t('The path should start with /.'));
return;
}
// Make sure we trim and normalize the path first.
$link['loc'] = trim($link['loc']);
$link['loc'] = $this->aliasManager->getPathByAlias($link['loc'], $link['language']);
$form_state->setValue('loc', $link['loc']);
try {
$client = $this->httpClientFactory->fromOptions(['config/curl', array(CURLOPT_FOLLOWLOCATION => FALSE)]);
$client->get(Url::fromUserInput('/' . ltrim($link['loc'], '/'), ['absolute' => TRUE])->toString());
$client->get(Url::fromUserInput($link['loc'], ['absolute' => TRUE])->toString());
}
catch (ClientException $e) {
$form_state->setErrorByName('loc', $this->t('The custom link @link is either invalid or it cannot be accessed by anonymous users.', array('@link' => $link['loc'])));
......
......@@ -54,20 +54,20 @@ class XmlSitemapCustomFunctionalTest extends XmlSitemapTestBase {
$this->clickLink(t('Add custom link'));
// Test an invalid path.
$edit['loc'] = 'invalid-testing-path';
$edit['loc'] = '/invalid-testing-path';
$this->drupalPostForm(NULL, $edit, t('Save'));
$this->assertText(t('The custom link @link is either invalid or it cannot be accessed by anonymous users.', array('@link' => $edit['loc'])));
$this->assertNoSitemapLink(array('type' => 'custom', 'loc' => $edit['loc']));
// Test a path not accessible to anonymous user.
$edit['loc'] = 'admin/people';
$edit['loc'] = '/admin/people';
$this->drupalPostForm(NULL, $edit, t('Save'));
$this->assertText(t('The custom link @link is either invalid or it cannot be accessed by anonymous users.', array('@link' => $edit['loc'])));
$this->assertNoSitemapLink(array('type' => 'custom', 'loc' => $edit['loc']));
// Test that the current page, which should not give a false positive for
// $menu_item['access'] since the result has been cached already.
$edit['loc'] = 'admin/config/search/xmlsitemap/custom/add';
$edit['loc'] = '/admin/config/search/xmlsitemap/custom/add';
$this->drupalPostForm(NULL, $edit, t('Save'));
$this->assertText(t('The custom link @link is either invalid or it cannot be accessed by anonymous users.', array('@link' => $edit['loc'])));
$this->assertNoSitemapLink(array('type' => 'custom', 'loc' => $edit['loc']));
......@@ -78,25 +78,25 @@ class XmlSitemapCustomFunctionalTest extends XmlSitemapTestBase {
*/
public function testCustomFileLinks() {
// Test an invalid file.
$edit['loc'] = $this->randomMachineName();
$edit['loc'] = '/' . $this->randomMachineName();
$this->drupalPostForm('admin/config/search/xmlsitemap/custom/add', $edit, t('Save'));
$this->assertText(t('The custom link @link is either invalid or it cannot be accessed by anonymous users.', array('@link' => $edit['loc'])));
$this->assertNoSitemapLink(array('type' => 'custom', 'loc' => $edit['loc']));
// Test an unaccessible file .
$edit['loc'] = '.htaccess';
$edit['loc'] = '/.htaccess';
$this->drupalPostForm('admin/config/search/xmlsitemap/custom/add', $edit, t('Save'));
$this->assertText(t('The custom link @link is either invalid or it cannot be accessed by anonymous users.', array('@link' => $edit['loc'])));
$this->assertNoSitemapLink(array('type' => 'custom', 'loc' => $edit['loc']));
// Test a valid file.
$edit['loc'] = 'core/misc/drupal.js';
$edit['loc'] = '/core/misc/drupal.js';
$this->drupalPostForm('admin/config/search/xmlsitemap/custom/add', $edit, t('Save'));
$this->assertText(t('The custom link for @link was saved.', array('@link' => $edit['loc'])));
$links = $this->linkStorage->loadMultiple(array('type' => 'custom', 'loc' => $edit['loc']));
$this->assertEqual(count($links), 1, t('Custom link saved in the database.'));
//Test a duplicate url.
$edit['loc'] = 'core/misc/drupal.js';
$edit['loc'] = '/core/misc/drupal.js';
$this->drupalPostForm('admin/config/search/xmlsitemap/custom/add', $edit, t('Save'));
$this->assertText(t('There is already an existing link in the sitemap with the path @link.', array('@link' => $edit['loc'])));
$links = $this->linkStorage->loadMultiple(array('type' => 'custom', 'loc' => $edit['loc']));
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment