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

includeDynamic on multiple sites with a subfolder (site.com/myLanguage) may be broken #562

Closed
Romanavr opened this issue Sep 25, 2023 · 13 comments
Assignees
Labels
enhancement New feature or request

Comments

@Romanavr
Copy link

Romanavr commented Sep 25, 2023

I ran into a weird issue after switching from getTemplate to includeDynamic tag.

When using getTemplate everything was fine, but after switching to includeDynamic, all the includes just fall off with a 404 error.
So I decided to check what the problem, because for other sites it worked well.

After some research, I realized that the included URL was prefixed with a currentSite prefix:
So instead of site.com/index.php?action=blitz... it was site.com/en/index.php?action=blitz... And as I have only one index.php in the web folder(without any prefixes), all dynamic requests turn into broken.
craft_folder/web/index.php
And I don't have
craft_folder/web/en/index.php

I understand that I can create, for example, an en folder in the web directory and add index.php there, but isn't there a more elegant solution. Is it possible to add some option in config/somewhere else to not include the current site URL and use root/baseUrl?

Or what can I do to fix this without creating any folders with another index.php?

@Romanavr Romanavr added the enhancement New feature or request label Sep 25, 2023
@bencroker
Copy link
Collaborator

Does your web server run nginx or Apache, and can you please show me the location block or rewrite rule used, respectively?

@Romanavr
Copy link
Author

Romanavr commented Sep 25, 2023

Does your web server run nginx or Apache, and can you please show me the location block or rewrite rule used, respectively?

Thanks for the reply @bencroker

Nginx. You can easily reproduce it using the craftcms/nginx docker image. But here's an example from staging server:

...
    # Cache path
    set $cache_path false;
    if ($request_method = GET) {
        set $cache_path /cache/blitz/$host/$lang/$uri/$args/index.html;
    }
    if ($args ~ "token=") {
        set $cache_path false;
    }
    
    #- Send would-be 404 requests to the cache path or Craft
    location / {
        try_files $cache_path $uri $uri/ /index.php?$query_string;
    }
...

@bencroker
Copy link
Collaborator

I would think that the final rule, /index.php?$query_string, would send the request to the base URL. At least that is the behaviour I am seeing locally.

@Romanavr
Copy link
Author

Romanavr commented Sep 25, 2023

I would think that the final rule, /index.php?$query_string, would send the request to the base URL. At least that is the behaviour I am seeing locally.

Are you trying to reach the URL with the subfolder at the end?
E.g. http://127.0.0.1:8090/en/index.php?action=blitz%2Finclude%2Fdynamic&index=4098772381

изображение

I see no reason why it should fall to the "/" location.
Because server requests /en/index.php while I don't have en folder the web directory.

Can't you just use the controller URL(without index.php) like you do for getTemplate? I believe this should solve the problem
E.g. this works perfectly http://127.0.0.1:8027/en/actions/blitz/include/dynamic?index=XXXXXXXXX

@bencroker
Copy link
Collaborator

bencroker commented Sep 26, 2023

This is due to how try_files works in the nginx config. It tries to find a file based on the listed paths in a left-to-right order, so should continue down the list until it finds it at /index.php?$query_string.

try_files $cache_path $uri $uri/ /index.php?$query_string;

Can you please confirm that you’re using the try_files statement above?

Can't you just use the controller URL(without index.php) like you do for getTemplate? I believe this should solve the problem

This would work but also breaks other parts of the system.

@Romanavr
Copy link
Author

Romanavr commented Sep 26, 2023

This is due to how try_files works in the nginx config. It tries to find a file based on the listed paths in a left-to-right order, so should continue down the list until it finds it at /index.php?$query_string.

try_files $cache_path $uri $uri/ /index.php?$query_string;

Can you please confirm that you’re using the try_files statement above?

Can't you just use the controller URL(without index.php) like you do for getTemplate? I believe this should solve the problem

This would work but also breaks other parts of the system.

Yes, I confirm that I use this statement.

This is due to how try_files works in the nginx config. It tries to find a file based on the listed paths in a left-to-right order, so should continue down the list until it finds it at /index.php?$query_string.

I understand it. I may be wrong, but as far as I know, the request for /en/index.php does not match the "location/" conditions, since in this case we are requesting the exact file in a specific folder, and since there is no such route. it will be 404.

https://putyourlightson.com/index.php 200
https://putyourlightson.com/en/index.php 404

This would work but also breaks other parts of the system.

Okay, then it looks like I can only add another location rule for this case so that I can catch the location of /subfolder/index.php. Something like this?

    location ~ ^/(.*)/index.php {
        try_files $uri $uri/ /index.php?$query_string;
    }

@bencroker
Copy link
Collaborator

One of the location blocks in your nginx config file must be taking precedence. The prefix location / matches all requests but only as a last resort if no other matches are found.

You shouldn’t need to be doing this, though, and I plan on fixing it at the code level. It may be as simple as 4151114 but I’ll need to do some further testing. In the meantime, can you please see if that change solves the issue for you?

@Romanavr
Copy link
Author

Romanavr commented Sep 28, 2023

One of the location blocks in your nginx config file must be taking precedence. The prefix location / matches all requests but only as a last resort if no other matches are found.

You shouldn’t need to be doing this, though, and I plan on fixing it at the code level. It may be as simple as 4151114 but I’ll need to do some further testing. In the meantime, can you please see if that change solves the issue for you?

Thanks Ben.

Yes, this change solves the problem for me:
Before change -> http://127.0.0.1:80XX/en/index.php?action=blitz%2Finclude%2Fdynamic&index=XXXXXXXX 404
After change -> http://127.0.0.1:80XX/en/?action=blitz%2Finclude%2Fdynamic&index=XXXXXXXX 200

@bencroker
Copy link
Collaborator

bencroker commented Sep 28, 2023

That’s good to hear, thanks for testing! I’ll do some more testing on my end and, if all goes well, will add this to the next release.

@abigailjxn
Copy link

Chiming in to just confirm that I ran into this too and reverting to 4.5.0 resolved my errors. Thanks for the incoming fix! 🙏

@bencroker
Copy link
Collaborator

bencroker commented Oct 3, 2023

Thanks for that @abigailjxn, I'm working on a fix to resolve this as well as the issue that was originally solved, without inadvertently introducing a regression bug.

@bencroker bencroker self-assigned this Oct 10, 2023
@bencroker
Copy link
Collaborator

bencroker commented Oct 10, 2023

This should be fixed in 340264d.

You can test this by running composer require "putyourlightson/craft-blitz:dev-develop as 4.6.0".

@bencroker
Copy link
Collaborator

bencroker commented Oct 17, 2023

Released in 4.6.0.

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

No branches or pull requests

3 participants