Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CurrentVisitorUrl rule does not take urlmap into account #22007

Closed
wezell opened this issue Apr 11, 2022 · 5 comments · Fixed by #22142
Closed

CurrentVisitorUrl rule does not take urlmap into account #22007

wezell opened this issue Apr 11, 2022 · 5 comments · Fixed by #22142

Comments

@wezell
Copy link
Contributor

wezell commented Apr 11, 2022

Trying to use rule to do redirects on our doc site and it is not correctly capturing the url. This basically makes rules useless for site redirects on sites that use urlmaps

The rule is trying to redirect a user from https://dotcms.com to https://www.dotcms.com but the issue is that it does not take URLMaps into account when rewriting the url. This means that someone visiting a urlmapped page

https://dotcms.com/docs/latest/markdown-syntax

ends up getting redirected to the urlmap detail page

https://www.dotcms.com/documentation/index

Screen Shot 2022-04-11 at 5 01 41 PM

@wezell
Copy link
Contributor Author

wezell commented May 3, 2022

PR: #22142

@fabrizzio-dotCMS fabrizzio-dotCMS self-assigned this May 13, 2022
@fabrizzio-dotCMS
Copy link
Contributor

fabrizzio-dotCMS commented May 16, 2022

Here's what I did to verify this PR

  • I added the following entry to my /etc/hosts. 127.0.0.1 dotcms.com
  • then I created a dummy page like /documents/rest-endpoint-documentation on my local instance
  • then I added a Rule almost identical to the one listed in the first comment above Request Header Host is dotcms.com:8080
  • And then fired the rule putting this in my browser http://dotcms.com:8080/documentation/rest-endpoint-documentation
  • and then verified the redirection effectively takes the user to https://www.dotcms.com/documentation/rest-endpoint-documentation

As far as I can tell the PR #22142 is good to go as it is and can be merged

@wezell
Copy link
Contributor Author

wezell commented May 17, 2022

So to test using blogs

  1. Create a vanity url that forwards from an existing blog to the homepage
  2. test to make sure that works (you can see the vanityurl header)
  3. add a rule that checks current url for the same url as the vanity - have it add a header
  4. make sure both the rule and the vanity are firing.

@fabrizzio-dotCMS
Copy link
Contributor

I added a Rule configured to intercept everything under blogs /blog/post/(.*) and set a header
Also created a VanityURL that intercepts the specific URL of a blog post and redirects to the blog's index.
I was able to verify that both the vanity and the Rule work together in conjunction As the redirection takes place correctly and the header is also set in the response

nollymar pushed a commit that referenced this issue May 18, 2022
Co-authored-by: nollymar <[email protected]>
Co-authored-by: fabrizzio-dotCMS <[email protected]>
@nollymar nollymar linked a pull request May 18, 2022 that will close this issue
dsilvam pushed a commit that referenced this issue Jun 1, 2022
* #22131 fix show-or-hide-ViewStatistics-button-based-on-dotmarketing-property (#22201)

* #22073 adding test for AWSS3Publisher (#22200)

* #21060 Create the XStreamFactory (#22187)

* #21060 Create the XStreamFactory

* Add encoding

* Add Encoding

* Add test in MainSuite

* Add test in MainSuite

* remove unnecessary change

* #21626 Container Resource (#21455)

* #2142 adding first draft

* #21432 optimizing the get link

* #21432 optimizing the get link2

* #21432 adding postman test

* #21432 adding template test

* #21432 first draft container

* adding unpublish

* #21432 unpublish container

* #21432 added save new container endpoint

* #21432 adding container bring back + curl test

* #21432 adding container bring back + curl test

* #21432 adding the methods for all containers operations

* #21432 adding first curl test for containers

* #21432 adding container publish/unpublish

* #21432 more container enhancements

* #21432 adding publish and unpublish for file container

* #21432 adding the archive functionality

* #21432 adding changes for retrieving archive file container

* #21432 adding the unarchive function

* #21432 added delete container

* #21432 clean up

* #21432 test and final changes to the container resource

* #21432 adding some fixes to the container curl test

* #21432 adding more curl test per feedback

* #21432 fixing curl test

* #21432 fixing curl test part 2

* #21432 finally fixed the curl test for containers

* #21432 fixing the curl test

* #21432 changes over the curl test

* #21432 adding a change to remove the container/test folder is exists and run the test from fresh

* #21432 adding more curl test

* #21432 adding PR feedback

* Targeting master as branch

* Validatee if exists a Vanity URL Contet Type with a site Custom field (#22232)

* #22186 The minSelections was modified to avoid refreshing screen without selecting a country (#22243)

Co-authored-by: nollymar <[email protected]>

* #22007 fixes rewrites when its a urlmap (#22142)

Co-authored-by: nollymar <[email protected]>
Co-authored-by: fabrizzio-dotCMS <[email protected]>

* Removing Task220404RemoveCalendarReminderTest

* #22250 Moving unique oer site to field variable

* #21060 Using XStreamFactory everywhere (#22192)

* #21060 Create the XStreamFactory

* Add encoding

* Add Encoding

* #21060 Using XStreamFactory everywhere

* Just testing

* Testing

* Testing

* testing

* Testing

* Testing

* Testing

* Testing

* Testing

* Testing

* Testing

* Testing

* Testing

* Testing

* Testing

* Testing

* Testing

* Fxing test

* Fixing test

* Removing ;

* refactoring

* Fixing test

* Revert "Fixing test"

This reverts commit 1541ff7.

* Revert "Revert "Fixing test""

This reverts commit 1bcee19.

* Revert "Merge remote-tracking branch 'origin/master' into issue-22250-Move-Unique-per-site-to-a-field-variable"

This reverts commit 82edb94, reversing
changes made to 1541ff7.

Co-authored-by: alfredo-dotcms <[email protected]>
Co-authored-by: Fabrizzio Araya <[email protected]>
Co-authored-by: Jonathan <[email protected]>
Co-authored-by: nollymar <[email protected]>
Co-authored-by: Nollymar Longa <[email protected]>
Co-authored-by: Will Ezell <[email protected]>
Co-authored-by: fabrizzio-dotCMS <[email protected]>
Co-authored-by: Freddy Montes <[email protected]>
oidacra pushed a commit that referenced this issue Jun 13, 2022
* #22131 fix show-or-hide-ViewStatistics-button-based-on-dotmarketing-property (#22201)

* #22073 adding test for AWSS3Publisher (#22200)

* #21060 Create the XStreamFactory (#22187)

* #21060 Create the XStreamFactory

* Add encoding

* Add Encoding

* Add test in MainSuite

* Add test in MainSuite

* remove unnecessary change

* #21626 Container Resource (#21455)

* #2142 adding first draft

* #21432 optimizing the get link

* #21432 optimizing the get link2

* #21432 adding postman test

* #21432 adding template test

* #21432 first draft container

* adding unpublish

* #21432 unpublish container

* #21432 added save new container endpoint

* #21432 adding container bring back + curl test

* #21432 adding container bring back + curl test

* #21432 adding the methods for all containers operations

* #21432 adding first curl test for containers

* #21432 adding container publish/unpublish

* #21432 more container enhancements

* #21432 adding publish and unpublish for file container

* #21432 adding the archive functionality

* #21432 adding changes for retrieving archive file container

* #21432 adding the unarchive function

* #21432 added delete container

* #21432 clean up

* #21432 test and final changes to the container resource

* #21432 adding some fixes to the container curl test

* #21432 adding more curl test per feedback

* #21432 fixing curl test

* #21432 fixing curl test part 2

* #21432 finally fixed the curl test for containers

* #21432 fixing the curl test

* #21432 changes over the curl test

* #21432 adding a change to remove the container/test folder is exists and run the test from fresh

* #21432 adding more curl test

* #21432 adding PR feedback

* Targeting master as branch

* Validatee if exists a Vanity URL Contet Type with a site Custom field (#22232)

* #22186 The minSelections was modified to avoid refreshing screen without selecting a country (#22243)

Co-authored-by: nollymar <[email protected]>

* #22007 fixes rewrites when its a urlmap (#22142)

Co-authored-by: nollymar <[email protected]>
Co-authored-by: fabrizzio-dotCMS <[email protected]>

* Removing Task220404RemoveCalendarReminderTest

* #22250 Moving unique oer site to field variable

* #21060 Using XStreamFactory everywhere (#22192)

* #21060 Create the XStreamFactory

* Add encoding

* Add Encoding

* #21060 Using XStreamFactory everywhere

* Just testing

* Testing

* Testing

* testing

* Testing

* Testing

* Testing

* Testing

* Testing

* Testing

* Testing

* Testing

* Testing

* Testing

* Testing

* Testing

* Testing

* Fxing test

* Fixing test

* Removing ;

* refactoring

* Fixing test

* Revert "Fixing test"

This reverts commit 1541ff7.

* Revert "Revert "Fixing test""

This reverts commit 1bcee19.

* Revert "Merge remote-tracking branch 'origin/master' into issue-22250-Move-Unique-per-site-to-a-field-variable"

This reverts commit 82edb94, reversing
changes made to 1541ff7.

Co-authored-by: alfredo-dotcms <[email protected]>
Co-authored-by: Fabrizzio Araya <[email protected]>
Co-authored-by: Jonathan <[email protected]>
Co-authored-by: nollymar <[email protected]>
Co-authored-by: Nollymar Longa <[email protected]>
Co-authored-by: Will Ezell <[email protected]>
Co-authored-by: fabrizzio-dotCMS <[email protected]>
Co-authored-by: Freddy Montes <[email protected]>
@bryanboza
Copy link
Contributor

Fixed, tested on release-22.-7 // docker // FF

@wezell wezell closed this as completed Aug 2, 2022
@erickgonzalez erickgonzalez added LTS: Needs Backport Ticket that will be added to LTS Next LTS Release and removed LTS: Needs Backport Ticket that will be added to LTS labels Nov 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants