Closed (outdated)
Project:
Token
Version:
8.x-1.x-dev
Component:
Code
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
19 Aug 2019 at 15:03 UTC
Updated:
23 Jan 2026 at 10:52 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
valthebaldComment #3
nicrodgersWe ran in to a related issue with createFromRequest. The problem with createFromRequest is it doesn't take in to account user access, so in our case it's returning a pathalias that the current user doesn't have access to. This patch also replaces the remaining createFromRequest usage with Url::fromRoute('').
Still needs tests, and need to investigate the failing tests. Also need to check the try/catch logic around these changes and update the comments accordingly.
Comment #4
ngkoutsaik commentedComment #5
ngkoutsaik commentedHi,
I reviewed the issue and the supplied patch by @nicrodgers.
I troubleshoot the failing tests. First the functional tests.
Url:createFromRequestdoes not throw an exception when the route does not exist. Instead, It returns a system/404 URL. That is why I extended the patch and addedThen the functional tests run.
Next on the kernel tests, speciffically the URLtests. I updated the first line from
expectedCurrentRequestUrlResultson UrlTest toUrl::fromUserInput(Request::createFromGlobals()->getPathInfo())->setAbsolute()->toString()]. That way the outputs match. I am not certain if this the optimal way to go about it.The test with the 'does-not-exist' URL works without any issues.
The last 2 are the tests with the malformed URL test. And this is where I got stuck. Because of
Url::fromRoute('<current>')the URL is never null and so a token is created. Additionally, I don't know why butfromRoutedoes not return system/404 during the kernel tests. It just returns HTTP://localhostIf I set the $url variable to null by hand the 2 last tests pass.
I am attaching my patch in hope that someone else might have an idea on how to proceed.
Comment #6
ngkoutsaik commentedComment #7
nicrodgersI went to re-roll the patch for 1.17 but don't think it's needed anymore, due to the refactoring of the code in the OOP conversion. I've done some manual testing and can't reproduce the issues we were experiencing in < 1.17 so am going to close this down. Feel free to reopen if I'm wrong!