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

[Bug]: Having a pretty URL, that matches the key of a document link leads to infinite redirects on multisite environments #16420

Open
alexej-d opened this issue Jan 4, 2024 · 13 comments · May be fixed by #16776

Comments

@alexej-d
Copy link

alexej-d commented Jan 4, 2024

Pimcore version

11.1.4

Steps to reproduce

Create a subsite. Create a link document with the key "test". Create a directory or a page and nest another page with the key "test" below it. Give the nested "test" document the pretty URL "/test".

Actual Behavior

I am caught in an infinite redirect loop.

Expected Behavior

Navigating to /test should show the page with the internal path /subsite/subfolderOrPage/test.

@alexej-d alexej-d added the Bug label Jan 4, 2024
@wisconaut
Copy link
Contributor

@alexej-d Thanks for reporting this issue. Does this also occur without subsites?

@alexej-d
Copy link
Author

alexej-d commented Jan 8, 2024

@wisconaut yeah, the same error occurs without sites. If the resolved page of type link with the key test links to /subsite/subfolderOrPage/test an infinite loop is started. If /test just links to a direct URL or another document, the link is resolved but shouldn't be (calling /test should open /subsite/subfolderOrPage/test not the link in /test).

@wisconaut
Copy link
Contributor

@alexej-d Thanks for clarifying. We will have a look at it then.

@alexej-d
Copy link
Author

BTW @wisconaut: I believe this line needs to be changed.

At the moment, it takes pretty URLs into account too late. Before matching by the system path (like now), the code needs to check if a document with the pretty URL exists first.

@kingjia90
Copy link
Contributor

kingjia90 commented Feb 26, 2024

Thank you for reporting and the PR!

Not sure if i did it right, maybe a few screenshot would help to visualize the tree

Do you also have the ⚠️ icon on the Pretty URL?
image

Is it possible that you missed the warning alert?
image

If a link /test exists and set your pretty url as /test it will be conflicting 🤔
The check if is there a pretty url indeed is a little later in the code, i am wondering if there was any known issue/limitation to led to this and the warning

// check for a pretty url inside a site

I see instead a problem that the pretty url for a multi-site page is absolute and not relative to the sub-site domain itself, that might be the "bug"

@alexej-d
Copy link
Author

@kingjia90 what I mean is that link documents should not be considered in the process of creating pretty URLs. Check the attached video. I should be able to create a correctly functioning pretty URL even if a document of the type link exists with the same key. Apart from that, I also think document folders and snippets are also considered when creating a pretty URL which is wrong as only page documents can have pretty URL…

Bildschirmaufnahme.2024-02-26.um.13.48.51.mp4

@kingjia90
Copy link
Contributor

Thank you for the videoscreenshot!

I see, there is an alert on 0:27, there is clearly something unsupported/unwanted on the table 😄

AFAIK it is/was always prioritizing the document full path over PrettyUrls here

$data = $this->db->fetchAssociative('SELECT id FROM documents WHERE `path` = BINARY :path AND `key` = BINARY :key', $params);
if (!empty($data['id'])) {
$this->assignVariablesToModel($data);
} else {
// try to find a page with a pretty URL (use the original $path)
$data = $this->db->fetchAssociative('SELECT id FROM documents_page WHERE prettyUrl = :prettyUrl', [
'prettyUrl' => $path,
]);

@alexej-d
Copy link
Author

@kingjia90 yeah, that alert should not have popped up as the duplicate URL is in fact just an internal path of a link document

@kingjia90
Copy link
Contributor

kingjia90 commented Feb 26, 2024

Link are not internal though, the right path is https://demo.pimcore.com/en/Find-and-Buy and in the video you were typing without /en but the error should appear with /en/, at least here doesn't find as /Find-and-Buy

@alexej-d
Copy link
Author

So my PR has been closed @kingjia90 – I take it you are working on this issue instead? :)

@kingjia90
Copy link
Contributor

Sorry for that, it was unintentional, as soon as i have deleted 11.1, it automatically closed all the open PRs based off that. Fixing these right now

@kingjia90
Copy link
Contributor

In your case, you have already deleted the branch though and i can't manually fix the base anymore, could you please recreate the branch and open a new PR? Thank you in advance

@alexej-d alexej-d linked a pull request Mar 13, 2024 that will close this issue
@alexej-d
Copy link
Author

@kingjia90 I have reopened my PR here #16776

@kingjia90 kingjia90 linked a pull request Mar 13, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants