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

[5.x, 4.x]: bootstrap/router.php can not handle umlauts #14977

Closed
thomas-jaeger opened this issue May 11, 2024 · 12 comments · Fixed by #15115
Closed

[5.x, 4.x]: bootstrap/router.php can not handle umlauts #14977

thomas-jaeger opened this issue May 11, 2024 · 12 comments · Fixed by #15115
Labels

Comments

@thomas-jaeger
Copy link

What happened?

Description

If serving files in local directory, router.php does not serve files with umlauts.

Steps to reproduce

  1. Create an asset with umlauts, for example hellö.png
  2. router.php tries to serve a file named hell%C3%96.png

Expected behavior

File should be served from file system

Actual behavior

File is not found in the file system

Fix (use urldecode):

4,6c4,6
< $parts = parse_url($_SERVER['REQUEST_URI'] ?? '');
< if (!empty($parts['path'])) {
<     if (realpath($_SERVER['DOCUMENT_ROOT'] . DIRECTORY_SEPARATOR . $parts['path'])) {
---
> $path = parse_url($_SERVER['REQUEST_URI'] ?? '', PHP_URL_PATH);
> if (!empty($path)) {
>     if (realpath($_SERVER['DOCUMENT_ROOT'] . DIRECTORY_SEPARATOR . urldecode($path))) {

Craft CMS version

4.x, 5.x

PHP version

8.3

Operating system and version

No response

Database type and version

No response

Image driver and version

No response

Installed plugins and versions

@brandonkelly
Copy link
Member

Thanks for reporting that, and for the suggested fix!

@brandonkelly
Copy link
Member

Craft 4.9.3 and 5.1.3 are out with that fix. Thanks again.

@boboldehampsink
Copy link
Contributor

boboldehampsink commented May 15, 2024

@brandonkelly not sure if it has anything to do with this, but since this update I have multiple projects that hang after craft serve:

Screenshot 2024-05-15 at 08 44 11

This has been running for the past 30 minutes and not proceeded - on at least 3 projects of mine (all craft 5 latest)

@brandonkelly
Copy link
Member

@boboldehampsink If you revert the change does it fix it?

@boboldehampsink
Copy link
Contributor

@brandonkelly nope it did not. I went on and reverted some other changes, and found that 81d20bb is the problem. Once I reverted that, everything started working again...

@brandonkelly
Copy link
Member

Can you set a breakpoint in that function? How big is $paths?

@boboldehampsink
Copy link
Contributor

@brandonkelly

Acceptance Tests (0) -------------------------------------------------------------------------------------------------------
Modules: \craft\test\Craft, WebDriver, \Helper\Acceptance
----------------------------------------------------------------------------------------------------------------------------

  [RunProcess] Starting chromedriver --url-base=/wd/hub
  [RunProcess] Starting ./craft serve --port=$PORT --interactive=0

  Suite done, restoring $_SERVER to original
----------------------------------------------------------------------------------------------------------------------------
  [RunProcess] Stopping ./craft serve --port=$PORT --interactive=0
Psy Shell v0.12.3 (PHP 8.3.5 — cli) by Justin Hileman
Execution PAUSED All commands will be saved to tests/_output/.pause.log
Use $this-> to access current object
>> $paths
=>  [
      "dateModified",
    ]

>> q
 INFO  Goodbye.
Psy Shell v0.12.3 (PHP 8.3.5 — cli) by Justin Hileman
Execution PAUSED All commands will be saved to tests/_output/.pause.log
Use $this-> to access current object
>> $paths
=>  [
      "dateModified",
    ]

>> q
 INFO  Goodbye.

So let me make it clear it only happens in test. And testing happens to remove the whole project config (don't know why?):

src/test/Craft.php is loaded as helper and does this:

/**
     * @inheritdoc
     */
    public function _beforeSuite($settings = []): void
    {
        parent::_beforeSuite($settings);

        if ($this->_getConfig('fullMock') !== true) {
            $this->setupDb();
        }

        TestSetup::removeProjectConfigFolders(CRAFT_CONFIG_PATH . DIRECTORY_SEPARATOR . 'project');
    }

    /**
     * @inheritdoc
     */
    public function _afterSuite(): void
    {
        parent::_afterSuite();

        TestSetup::removeProjectConfigFolders(CRAFT_CONFIG_PATH . DIRECTORY_SEPARATOR . 'project');
    }

@boboldehampsink
Copy link
Contributor

@brandonkelly want me to create a new issue for this?

@brandonkelly
Copy link
Member

@boboldehampsink Yes please. And can you include a stack trace that shows how craft\services\ProjectConfig::removeInternalConfigValuesByPaths() is getting hit in the first place?

@boboldehampsink
Copy link
Contributor

FWIW about 81d20bb - craft\services\ProjectConfig::removeInternalConfigValuesByPaths() is already always being called from within a transaction. Removing the transaction block in your commit fixes it as well. Will submit PR.

@brandonkelly
Copy link
Member

@boboldehampsink Weird, that would be far from the only place we use nested transactions. Thanks for digging in and sending the PR, though!

@brandonkelly
Copy link
Member

Craft 5.1.8 is out with that change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants