Problem/Motivation

Allow redirect between two different domains.
Example:
When user enters 'example.site.org' redirect him to 'site.org/example'

Proposed resolution

Use similar functionality as the host redirect module.

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#47 allow_redirect_between-2833457-47.patch20.76 KBGinovski
#47 interdiff-2833457-46-47.txt1.95 KBGinovski
#46 form-domain-red.png37.49 KBGinovski
#46 allow_redirect_between-2833457-46.patch20.73 KBGinovski
#46 interdiff-2833457-44-46.txt1.8 KBGinovski
#44 allow_redirect_between-2833457-44.patch19.9 KBGinovski
#44 interdiff-2833457-41-44.txt4.07 KBGinovski
#42 allow_redirect_between-2833457-41.patch18.13 KBGinovski
#42 interdiff-2833457-39-41.txt1.49 KBGinovski
#39 interdiff-2833457-38-39.txt7.88 KBGinovski
#39 allow_redirect_between-2833457-39.patch17.85 KBGinovski
#37 allow_redirect_between-2833457-37.patch18.2 KBGinovski
#33 domain-redirect-2833457-33.patch17.4 KBGinovski
#31 allow_redirect_between-2833457-31.patch13.74 KBGinovski
#31 interdiff-2833457-29-31.txt9.55 KBGinovski
#31 domain_form.png20.64 KBGinovski
#29 allow_redirect_between-2733457-29.patch10.2 KBGinovski
#29 interdiff-2833457-27-29.txt3.41 KBGinovski
#27 allow_redirect_between-2833457-27.patch9.8 KBGinovski
#27 interdiff-2833457-25-27.txt1.97 KBGinovski
#25 testfail.png36.54 KBGinovski
#25 allow_redirect_between-2833457-25.patch9.94 KBGinovski
#25 interdiff-2833457-23-25.txt6.89 KBGinovski
#23 allow_redirect_between-2833457-23.patch10.09 KBGinovski
#23 interdiff-2833457-21-23.txt1.95 KBGinovski
#21 allow_redirect_between-2833457-21.patch10.01 KBGinovski
#21 interdiff-2833457-19-21.txt2.87 KBGinovski
#19 allow_redirect_between-2833457-19.patch9.09 KBGinovski
#19 interdiff-2833457-14-19.txt1.14 KBGinovski
#14 allow_redirect_between-2833457-14.patch8.98 KBGinovski
#12 allow_redirect_between_2833457-12.patch16.14 KBGinovski
#12 interdiff-2833457-11-12.txt4.9 KBGinovski
#11 allow_redirect_between-2833457-11.patch8.2 KBGinovski
#8 form-domain-redirects.png22.35 KBGinovski
#8 allow_redirect_between-2833457-8.patch7.45 KBGinovski
#8 interdiff-2833457-5-8.txt4.89 KBGinovski
#5 allow_redirect_between-2833457-4.patch7.09 KBGinovski
#5 interdiff-2833457-2-4.txt765 bytesGinovski
#2 redirect-form.png37.46 KBGinovski
#2 allow_redirect_between-2833457-2.patch6.34 KBGinovski
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Ginovski created an issue. See original summary.

Ginovski’s picture

Initial patch, added a form, some configuration for the redirects in install folder, and extended the function
RedirectRequestSubscriber::onKernelRequestCheckRedirect to check for domains redirect.

Status: Needs review » Needs work

The last submitted patch, 2: allow_redirect_between-2833457-2.patch, failed testing.

The last submitted patch, 2: allow_redirect_between-2833457-2.patch, failed testing.

Ginovski’s picture

Status: Needs review » Needs work

The last submitted patch, 5: allow_redirect_between-2833457-4.patch, failed testing.

The last submitted patch, 5: allow_redirect_between-2833457-4.patch, failed testing.

Ginovski’s picture

1.Fixed schema
2. Added table in the form
3. Added ajax button for adding another redirect row (need to fix this)
4. Configured submitForm (need to fix this aswell, values from the text fields are not submitted).

Screenshot of the current form:

Status: Needs review » Needs work

The last submitted patch, 8: allow_redirect_between-2833457-8.patch, failed testing.

The last submitted patch, 8: allow_redirect_between-2833457-8.patch, failed testing.

Ginovski’s picture

Ginovski’s picture

Some form and schema configuration, functioning good now.

Status: Needs review » Needs work

The last submitted patch, 12: allow_redirect_between_2833457-12.patch, failed testing.

Ginovski’s picture

Reroll and added test coverage for the form functionality.

Status: Needs review » Needs work

The last submitted patch, 14: allow_redirect_between-2833457-14.patch, failed testing.

Ginovski’s picture

Status: Needs work » Needs review

The last submitted patch, 11: allow_redirect_between-2833457-11.patch, failed testing.

The last submitted patch, 11: allow_redirect_between-2833457-11.patch, failed testing.

Ginovski’s picture

Status: Needs review » Needs work

The last submitted patch, 19: allow_redirect_between-2833457-19.patch, failed testing.

Ginovski’s picture

Fixed the RedirectRequestSubscriberTest with adding a domain configration.

Status: Needs review » Needs work

The last submitted patch, 21: allow_redirect_between-2833457-21.patch, failed testing.

Ginovski’s picture

Adding an if to fix the foreach.

Berdir’s picture

Status: Needs review » Needs work
  1. +++ b/config/install/redirect.domain.yml
    @@ -0,0 +1,4 @@
    +domain_redirects:
    +  -
    +    from: abc.example.org
    +    to: www.example.org/abc
    

    this needs to go, default is an empty list.

  2. +++ b/redirect.routing.yml
    @@ -56,3 +56,11 @@ redirect.settings:
    +    _title: 'Domain redirect'
    

    Domain redirectS

  3. +++ b/src/EventSubscriber/RedirectRequestSubscriber.php
    @@ -123,6 +131,34 @@ class RedirectRequestSubscriber implements EventSubscriberInterface {
     
    +    // Redirect between domains configuration.
    +    if ($this->domainConfig) {
    

    this first if is always true as you assign it in the constructor.

  4. +++ b/src/EventSubscriber/RedirectRequestSubscriber.php
    @@ -123,6 +131,34 @@ class RedirectRequestSubscriber implements EventSubscriberInterface {
    +        $from_domain = $request->getUri();
    

    this matches on the full path for the domain.

    What you usually want to do with something like this is a redirect of anything that uses foo.org/bla to bar.org/bla. So only match on the domain and keep the path when redirecting.

    Matching a certain path is probably still useful, especially if we support wildcards. Lets split this up into a separate "Path" field, so you could have a configuration like this:

    From: example.org, Path: /blog/*, To: blog.example.org

  5. +++ b/src/EventSubscriber/RedirectRequestSubscriber.php
    @@ -123,6 +131,34 @@ class RedirectRequestSubscriber implements EventSubscriberInterface {
    +        // Removes HTTP prefix.
    +        if (strpos($from_domain, 'http://') !== FALSE) {
    +          $from_domain = substr($from_domain, 7);
    +        }
    +        if (strpos($from_domain, 'https://') !== FALSE) {
    +          $from_domain = substr($from_domain, 8);
    +        }
    

    if it contains either http:// or https:// then we should treat it to only match if you are actually using that protocol, so you need to set a flag in those ifs or check the protocol of the request.

    For example, if you have from: http://example.org and to: https://example.org, then you need to redirect to https but if you're on https then it must not match.

    That said, I'm not sure we should actually support that. Usually having this kind of configuration in Drupal is very annoying on local/test environments when you don't have https.

    So maybe we should validate that it does *not* contain a protocol in the UI and it ourself here for he target domain.

  6. +++ b/src/Form/RedirectDomainForm.php
    @@ -0,0 +1,158 @@
    +
    +class RedirectDomainForm extends ConfigFormBase {
    

    missing docblock.

  7. +++ b/src/Form/RedirectDomainForm.php
    @@ -0,0 +1,158 @@
    +   */
    +  public function __construct(ConfigFactoryInterface $config_factory) {
    +    parent::__construct($config_factory);
    +  }
    +
    +  /**
    +   * {@inheritdoc}
    +   */
    +  public static function create(ContainerInterface $container) {
    +    return new static(
    +      $container->get('config.factory')
    +    );
    

    this is the same as the parent

  8. +++ b/src/Form/RedirectDomainForm.php
    @@ -0,0 +1,158 @@
    +    if ($domain_redirects = \Drupal::config('redirect.domain')->get('domain_redirects')) {
    

    use $this->config.

  9. +++ b/src/Form/RedirectDomainForm.php
    @@ -0,0 +1,158 @@
    +    $form['redirects'] += $rows;
    

    you could above just do $form['redirects'][] then you don't need this.

  10. +++ b/src/Form/RedirectDomainForm.php
    @@ -0,0 +1,158 @@
    +    $form['submit'] = [
    +      '#type' => 'submit',
    +      '#value' => $this->t('Save'),
    +    ];
    

    add the primary button flag so this one is highlighted

  11. +++ b/src/Form/RedirectDomainForm.php
    @@ -0,0 +1,158 @@
    +    $form_state->set('maximum_domains', ($form_state->get('maximum_domains') ?:1) + 1);
    

    you could initialize maximum_domains in buildForm() to 1 if not set, then you don't need this special case.

  12. +++ b/src/Form/RedirectDomainForm.php
    @@ -0,0 +1,158 @@
    +   * {@inheritdoc}
    +   */
    +  public function validateForm(array &$form, FormStateInterface $form_state) {
    +    parent::validateForm($form, $form_state);
    +    // @todo Validate domain redirects.
    

    as discussed above

  13. +++ b/src/Form/RedirectDomainForm.php
    @@ -0,0 +1,158 @@
    +    $domain_redirects = [];
    +    $domain_config = \Drupal::configFactory()->getEditable('redirect.domain');
    

    again, $this->config(), that's why getEditableConfigNames() exists.

  14. +++ b/src/Form/RedirectDomainForm.php
    @@ -0,0 +1,158 @@
    +    if ($redirects = $form_state->getValues()['redirects']) {
    

    getValue('redirects')

Ginovski’s picture

Addressed changes from #24 comment.
Except for 3. (RedirectRequestSubscriberTest fail locally if I don't have this if) - added screenshot

Status: Needs review » Needs work

The last submitted patch, 25: allow_redirect_between-2833457-25.patch, failed testing.

Ginovski’s picture

Berdir’s picture

Status: Needs review » Needs work
  1. +++ b/src/Form/RedirectDomainForm.php
    @@ -45,6 +32,9 @@ class RedirectDomainForm extends ConfigFormBase {
    +    if ($form_state->get('maximum_domains') === NULL) {
    

    use has()

  2. +++ b/src/Form/RedirectDomainForm.php
    @@ -131,7 +121,13 @@ class RedirectDomainForm extends ConfigFormBase {
    +        if (strpos($redirect['from'], 'http://') !==FALSE || strpos($redirect['from'], 'https://') !== FALSE || strpos($redirect['to'], 'http://') !==FALSE || strpos($redirect['to'], 'https://') !== FALSE) {
    

    coding style. Also, you can simply look for '://'

  3. +++ b/src/Form/RedirectDomainForm.php
    @@ -139,10 +135,10 @@ class RedirectDomainForm extends ConfigFormBase {
         $domain_redirects = [];
    -    $domain_config = \Drupal::configFactory()->getEditable('redirect.domain');
    +    $domain_config = $this->configFactory()->getEditable('redirect.domain');
    

    not what I said, $this->config()

4 isn't addressed yet I think, splitting up domain and path + wildcard support.

Ginovski’s picture

1. Using has() instead of get() for the form_state
2. Fixed the if for the http:// check, now checking for ://
3. Changed Drupal::configFactory()->getEditable('redirect.domain') to $this->config('redirect.domain').
4. Added wildcard support (not sure if this I did this right)
Example for the wildcard usage:
Case 1:
Assuming there is a redirect domain in the configuration (foo.org/bar/* -> bar.foo.org)
When requesting foo.org/bar/post, it redirects to bar.foo.org/post
Case 2:
Assuming there is a redirect domain in the configuration (foo.org/first/* -> foo.org/first/second)
When requesting foo.org/first/third, it redirects to foo.org/first/second/third

Berdir’s picture

Status: Needs review » Needs work
  1. +++ b/src/EventSubscriber/RedirectRequestSubscriber.php
    @@ -143,6 +145,16 @@ class RedirectRequestSubscriber implements EventSubscriberInterface {
    +          // Check for a wildcard in the domains.
    +          if (strpos($domain['from'], '*')) {
    +            $replace_path = substr($domain['from'], strpos($domain['from'], '/'));
    +            $domain_base_uri = substr($domain['from'], 0, -2);
    +            if (strpos($from_domain, $domain_base_uri) !== FALSE) {
    

    there is a path matcher service that we might be able to use. Maybe. If not then we'll figure out something. Also, doesn't the break above skip this then?

    what we need todo is first get a list of matches for the domain and then apply the path rules.

    As discussed, we can't do web tests easily (only for path, not domain).

    but we should be able to write unit tests for this easily. Check the existing unit test and then lets define a number of scenarios that we want to support.

  2. +++ b/src/Form/RedirectDomainForm.php
    @@ -123,7 +123,7 @@ class RedirectDomainForm extends ConfigFormBase {
           foreach ($redirects as $redirect) {
    -        if (strpos($redirect['from'], 'http://') !==FALSE || strpos($redirect['from'], 'https://') !== FALSE || strpos($redirect['to'], 'http://') !==FALSE || strpos($redirect['to'], 'https://') !== FALSE) {
    +        if (strpos($redirect['from'], '://') !== FALSE || strpos($redirect['to'], '://') !== FALSE) {
               $form_state->setErrorByName('redirects', t('No protocol should be included in the redirect domain.'));
    

    the point of this change is that you don't need to do it twice ;)

Ginovski’s picture

1. Changed the schema structure of the domain redirects.
2. Changed the form and redirect function accordingly.
3. Added unit test with 1 basic redirect case:
Configuration: 'foo.com/*' => 'bar.com'
Test: Request to foo.com/example, redirects to bar.com/example.

Screenshot of the form:

Berdir’s picture

Status: Needs review » Needs work

* To Domain then isn't just the domain, its' the whole uri. Maybe we can call that Destination instead.
*

  1. +++ b/src/EventSubscriber/RedirectRequestSubscriber.php
    @@ -96,6 +103,7 @@ class RedirectRequestSubscriber implements EventSubscriberInterface {
         $this->languageManager = $language_manager;
         $this->config = $config->get('redirect.settings');
    +    $this->domainConfig = $config->get('redirect.domain');
    

    I've been thinking about this config and the separation of code from this and normal redirects.

    I see 3 options for the config:

    1. Existing settings file, then we only have to load one on each request
    2. Separate config file, current patch
    3. Separate, optional module for this feature.

    For 1 and 2, we additionally have 3 options for where this code lives:

    A. Same event subscriber, inlined (current patch)
    B. A separate event method
    C. A completely separate event subscriber (if we pick 3 above, then we need C here as it is a separate module anyway).

    In #2704213: "Redirect from non-canonical URLs to the canonical URLs" not working with language code in url, we moved a lot methods to a separate service, as that will possibly eventually move to core.

    I think we should at least move the logic to a separate method, that might also make it easier to test.

    And I'm actually considering to move it into th same config file but not sure about that yet. Wait with that for now.

  2. +++ b/src/EventSubscriber/RedirectRequestSubscriber.php
    @@ -123,6 +131,38 @@ class RedirectRequestSubscriber implements EventSubscriberInterface {
    +    // Redirect between domains configuration.
    +    if ($this->domainConfig) {
    +      $domains = $this->domainConfig->get('domain_redirects');
    

    still want to get rid of that if condition. And if we go with a separate method, then that should just work.

  3. +++ b/src/EventSubscriber/RedirectRequestSubscriber.php
    @@ -123,6 +131,38 @@ class RedirectRequestSubscriber implements EventSubscriberInterface {
    +            // Check for a wildcard in the domains.
    +            else if (strpos($item['sub_path'], '*') !== FALSE) {
    +              $sub_path = substr($item['sub_path'], 0, -1);
    +              if (strpos($path, $sub_path) !== FALSE) {
    +                $to_domain = $item['to_domain'] . '/' . substr($path, strlen($sub_path));
    +                break;
    +              }
    +            }
    

    did you try to use the PathMatcher service? I'd expect that to be a lot more flexible, someone might write something like /foo/*/bar

  4. +++ b/src/EventSubscriber/RedirectRequestSubscriber.php
    @@ -123,6 +131,38 @@ class RedirectRequestSubscriber implements EventSubscriberInterface {
    +          if ($to_domain) {
    

    as in the UI, I'd rename this to destination in config and variables.

Ginovski’s picture

1.Changed to_domain -> destination
2. Created new domain redirect request subscriber and test
3. Using path matcher for redirecting with wildcard.

Status: Needs review » Needs work

The last submitted patch, 33: domain-redirect-2833457-33.patch, failed testing.

Berdir’s picture

  1. +++ b/src/EventSubscriber/DomainRedirectRequestSubscriber.php
    @@ -0,0 +1,152 @@
    +    $this->domainConfig = $config->get('redirect.domain');
    

    $config_factory

  2. +++ b/src/EventSubscriber/DomainRedirectRequestSubscriber.php
    @@ -0,0 +1,152 @@
    +    $this->checker = $checker;
    

    redirectChecker, $redirect_checker.

  3. +++ b/src/EventSubscriber/DomainRedirectRequestSubscriber.php
    @@ -0,0 +1,152 @@
    +    $this->context = $context;
    

    requestContext, $request_contet.

  4. +++ b/src/EventSubscriber/DomainRedirectRequestSubscriber.php
    @@ -0,0 +1,152 @@
    +    // Get a clone of the request. During inbound processing the request
    +    // can be altered. Allowing this here can lead to unexpected behavior.
    +    // For example the path_processor.files inbound processor provided by
    +    // the system module alters both the path and the request; only the
    +    // changes to the request will be propagated, while the change to the
    +    // path will be lost.
    +    $request = clone $event->getRequest();
    

    this doesn't matter for us, we're not doing inbound path processing.

  5. +++ b/src/EventSubscriber/DomainRedirectRequestSubscriber.php
    @@ -0,0 +1,152 @@
    +          if ($path === $item['sub_path']) {
    +            $destination = $item['destination'];
    +            break;
    +          }
    +          // Check for a wildcard in the domains.
    +          else if (strpos($item['sub_path'], '*') !== FALSE) {
    +            if ($this->pathMatcher->matchPath($path, $item['sub_path'])) {
    +              $destination = $item['destination'];
    +              break;
    +            }
    +          }
    

    pathmatcher can do both, so you don't have to do your own check.

    getRequestUri includes the query string, pretty sure what we want here is getPathInfo().

  6. +++ b/src/EventSubscriber/DomainRedirectRequestSubscriber.php
    @@ -0,0 +1,152 @@
    +    // We can only check access for routed URLs.
    +    if (!$url->isRouted() || $this->checker->canRedirect($request, $url->getRouteName())) {
    +      // Add the 'rendered' cache tag, so that we can invalidate all responses
    +      // when settings are changed.
    +      $response = new TrustedRedirectResponse($url->toString(), 301);
    +      $response->addCacheableDependency(CacheableMetadata::createFromRenderArray([])->addCacheTags(['rendered']));
    +      $event->setResponse($response);
    +    }
    

    I don't think those redirects are ever routed, so we can possibly skip this.

    also not sure what the context line does above, we don't seem to use that here?

  7. +++ b/src/Tests/GlobalRedirectTest.php
    @@ -262,4 +262,40 @@ class GlobalRedirectTest extends WebTestBase {
    +  /**
    +   * Tests domain redirect.
    +   */
    +  public function testDomainRedirect() {
    +    $user = $this->drupalCreateUser([
    +      'administer site configuration',
    +      'access administration pages',
    +      'administer redirects'
    +    ]);
    +    $this->drupalLogin($user);
    

    lets make a separate test for this.

Berdir’s picture

Also, still thinking about making this a separate module like redirect_404, now that everything is in separate files. Lets discuss this.

A few more variations on the unit test would also be good, lets have a look at using a data provider for this.

Ginovski’s picture

1. Renamed config -> config_factory, checker -> redirect_checker
2. Removed request_context since it wasn't used.
3. Removed comment for the request inbound path processing.
4. Using path matcher for the redirect check.
5. Using pathInfo instead of requestUri
6. Skipped the redirects in setResponse()
7. Made a separate test for the UI
8. Moved the whole domain redirect to a submodule redirect_domain
9. Added services and everything accordingly.
10. Added data provider in the unit tests and another case for simple redirect.

Berdir’s picture

Status: Needs review » Needs work
  1. +++ b/modules/redirect_domain/tests/src/Unit/DomainRedirectRequestSubscriberTest.php
    @@ -0,0 +1,136 @@
    +    $checker = $this->getMockBuilder('Drupal\redirect\RedirectChecker')
    

    Can use RedirectChecker::class, same for other class/interface references.

  2. +++ b/modules/redirect_domain/tests/src/Unit/DomainRedirectRequestSubscriberTest.php
    @@ -0,0 +1,136 @@
    +    $checker->expects($this->any())
    +      ->method('isLoop')
    +      ->will($this->returnValue(FALSE));
    

    not used here I think? Wondering if we actually should, though, but we can possibly implement that later. (Would be pretty easy to configure an endless redirect).

  3. +++ b/modules/redirect_domain/tests/src/Unit/DomainRedirectRequestSubscriberTest.php
    @@ -0,0 +1,136 @@
    +    // Set up the configuration for the requested domain.
    +    $config_factory = $this->getConfigFactoryStub($data);
    

    that's not really how a data provider is meant to work.

    I'd keep the configuration static, what the data provider should provide is the incoming URL and the expected destination, including a NULL if it should not redirect. Then we test various different requests against a given configuration.

  4. +++ b/modules/redirect_domain/tests/src/Unit/DomainRedirectRequestSubscriberTest.php
    @@ -0,0 +1,136 @@
    +    // Set up the configuration for the requested domain.
    +    $config_factory = $this->getConfigFactoryStub($data);
    +    $config_factory_stub = $this->getConfigFactoryStub(
    +      [
    +        'system.site' => [
    +          'page.front' => '/',
    +        ],
    

    Those two things can combined into one call.

  5. +++ b/modules/redirect_domain/tests/src/Unit/DomainRedirectRequestSubscriberTest.php
    @@ -0,0 +1,136 @@
    +    // Run the main redirect method with wildcard in the middle.
    +    $event = $this->getGetResponseEventStub('http://example.com/foo/test/bar', http_build_query([]));
    +
    +    $subscriber->onKernelRequestCheckDomainRedirect($event);
    +
    +    $this->assertTrue($event->getResponse() instanceof RedirectResponse);
    +    $response = $event->getResponse();
    +    // Make sure that the response is properly redirected.
    +    $this->assertEquals('example.com/bar/foo', $response->getTargetUrl());
    +    $this->assertEquals(302, $response->getStatusCode());
    

    this snippet is then no longer repeated, instead you provide $request and $destination as arguments to your method. See \Drupal\KernelTests\Core\Cache\CacheCollectorTest::testCacheCollector() for an example.

  6. +++ b/modules/redirect_domain/tests/src/Unit/DomainRedirectRequestSubscriberTest.php
    @@ -0,0 +1,136 @@
    +   * Gets response event object.
    +   *
    +   * @param $path_info
    +   * @param $query_string
    +   *
    +   * @return GetResponseEvent
    +   */
    +  protected function getGetResponseEventStub($path_info, $query_string) {
    

    probably copied, but we should fix the docs still on the new code.

  7. +++ b/modules/redirect_domain/tests/src/Unit/DomainRedirectRequestSubscriberTest.php
    @@ -0,0 +1,136 @@
    +  public function providerDomains() {
    

    no documentation.

Ginovski’s picture

1. Addressed changes from #38 comment.
2. Also added the protocol to the destination, not sure if I did this correct

$response = new TrustedRedirectResponse($protocol . $destination);
Berdir’s picture

Status: Needs review » Needs work
  1. +++ b/modules/redirect_domain/tests/src/Unit/DomainRedirectRequestSubscriberTest.php
    @@ -24,113 +27,106 @@
    +    else {
    +      $this->assertNull($response_url);
    +    }
    

    That's asserting the wrong thing, you need to check if you have a response, not that the response_url is null, you know that already.

  2. +++ b/modules/redirect_domain/tests/src/Unit/DomainRedirectRequestSubscriberTest.php
    @@ -24,113 +27,106 @@
    +    $datasets[] = ['http://foo.com/example', 'http://bar.com/example'];
    +    $datasets[] = ['http://example.com/foo/test/bar', 'http://example.com/bar/foo'];
    +    $datasets[] = ['http://simpleexample.com/redirect', 'http://redirected.com/redirect'];
    +    $datasets[] = ['http://nonexisting.com', NULL];
    

    We have no example yet for..

    * correct domain but wrong path
    * A domain with multiple paths, e.g. first a fixed path and then a wildcard, you could add a fixed redirect to foo:com, if it matches that, it should use that destination and otherwise the wildcard.

Ginovski’s picture

Status: Needs work » Needs review

Edit: #42 is the correct comment.

Ginovski’s picture

Added the two cases and fixed assertion.
There is 1 thing though:
If the user writes first the wildcard in the configuration, then the fixed,
the wildcard will pick up the redirect before the fixed.
So not sure if it should check for the fixed first, and then for the wildcards? (that was in some previous patch with the manual check instead of pathmatcher)

Berdir’s picture

Status: Needs review » Needs work

As discussed:

* move yml file
* ensure leading / in form
* add help text

Ginovski’s picture

1. Moved yml file
2. Enforced a leading '/' in the subpath and added test assertion
3. Added hook_help.

Ginovski’s picture

Adding the sorting domain redirects as a related issue.

Ginovski’s picture

Added help text for the domain redirect.

Screenshot of the form:

Ginovski’s picture

Fixed the subpath and added 1 more test assertion.

  • Berdir committed 8b29d54 on 8.x-1.x authored by Ginovski
    Issue #2833457 by Ginovski: Allow redirect between domains
    
Berdir’s picture

Status: Needs review » Fixed

Yay, finally :)

Committed.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

ABaier’s picture

Just a quick question:
Would wildcards in the middle of a path work? I have to fix some strange patterns of an old site.

Example of a possible sub path:
/node/*/videos

Another thing:
If I had a path with more than one arguments, would one wildcard solve it or do I have to set one for each position?

Example:
/videos/335/some-alias
Sub path:
/videos/* OR /videos/*/*

Nginx should not be a problem, I hope.

Thanks for any hints!

ABaier’s picture

Forget my questions in #51 … everything is working as expected, but I missed out that our server redirects the whole traffic from example.com to www.example.com, so my redirects with example.com in the "from domain" field did not catch. Maybe it would be a good idea to mention this in the example configuration above the form.

arakwar’s picture

I'm not sure I understood the documentation and this thread properly, but my assumption is that a config of :
FROM : example.com
SUB PATH : /*
DESTINATION : foo.com/en

should redirect http://example.com/product/first to http://foo.com/en/product/first

Am I right ? If so, it doesn't seems to work... Anything I throw at the site on example.com once the configuration is set as noted earlier will just land at http://foo.com/en

I could just set an entry for each redirection, but I have about 7 or 8 domains to redirect, and my marketing team misprinted some URL. It would be nice to be able to redirect entire domain to the new base domain, then have the base domain manage those wrong URL.

Most people would recommend at this point to manage it in the .htaccess file, but with our multisite configuration that would mean more than thousand rules... I need to figure out a way to have Drupal to manage it :( The Redirect module with the domain redirect feature seemed the best way, if it could work as I assumed it.

If it's not the plan, what would be a good procedure ? maybe adding a tag in the Destination URL to let know that we want the subpath to be repeated there ?

bkosborne’s picture

Looks like that type of redirect is not supported, but there's work being done in #2904056: Option to keep original path on domain redirect to fix that.