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

docs: Update DDEV usage - set .ddev.site explicitly for server.cors.origin, add support for custom https_router_port for server.origin #99

Merged
merged 2 commits into from
Feb 7, 2025

Conversation

mandrasch
Copy link
Contributor

@mandrasch mandrasch commented Feb 5, 2025

Description

Changes:

  1. use \.ddev\.site explicitly for regex for server.cors.origin, as proposed by vitejs core maintainer - otherwise the attack surface is still open via sites with .site TLD

  2. support custom router-https-port (test via ddev config --router-https-port 8443 && ddev restart - process.env.DDEV_PRIMARY_URL will then output "https://my-project.ddev.site:8443". Therefore we need to strip out 8443 first, before setting the server.origin setting. This is also relevant because DDEV auto-selects a router port if 80/443 is already occupied since last year (https://ddev.com/blog/release-v1235-auto-port-assignment/). This needs a tiny change in config/vite.php as well.

  3. add info about official article about Vite on ddev.com (https://ddev.com/blog/working-with-vite-in-ddev/) where users find out more information and where DDEV maintainers try to keep all further infos up-to-date.

Hope this will be it, fingers crossed. Thanks very much for providing this plugin!

(Demo repo with this config is available here: https://github.com/mandrasch/ddev-craftcms-vite)

Related issues

@mandrasch mandrasch requested a review from khalwat as a code owner February 5, 2025 17:44
@mandrasch mandrasch marked this pull request as draft February 5, 2025 18:07
@mandrasch
Copy link
Contributor Author

mandrasch commented Feb 5, 2025

Okay, one question is remaining. For using ddev config --router-https-port 8443 && ddev restart,
config/vite.php might need to be changed from

        'devServerPublic' => App::env('PRIMARY_SITE_URL') . ':3000',

to

        'devServerPublic' => preg_replace('/:\d+$/', '', App::env('PRIMARY_SITE_URL')) . ':3000',

to also support primary URLs like https://my-project.ddev.site:8443. I'll need to test this.

@mandrasch mandrasch changed the title docs: Update DDEV usage - set .ddev.site explicitly, add support for custom https_router_port docs: Update DDEV usage - set .ddev.site explicitly for server.cors.origin, add support for custom https_router_port for server.origin Feb 5, 2025
@mandrasch
Copy link
Contributor Author

Okay, one question is remaining. For using ddev config --router-https-port 8443 && ddev restart, config/vite.php might need to be changed from

        'devServerPublic' => App::env('PRIMARY_SITE_URL') . ':3000',

to

        'devServerPublic' => preg_replace('/:\d+$/', '', App::env('PRIMARY_SITE_URL')) . ':3000',

to also support primary URLs like https://my-project.ddev.site:8443. I'll need to test this.

Just tested it, it is indeed needed to strip away the custom https port via

        'devServerPublic' => preg_replace('/:\d+$/', '', App::env('PRIMARY_SITE_URL')) . ':3000',

Updated the PR, ready for review ✅

@mandrasch mandrasch marked this pull request as ready for review February 5, 2025 20:58
@khalwat
Copy link
Contributor

khalwat commented Feb 7, 2025

Thanks again, appreciate it.

@khalwat khalwat merged commit 9073262 into nystudio107:develop-v5 Feb 7, 2025
2 checks passed
khalwat added a commit that referenced this pull request Feb 7, 2025
…rigin, add support for custom https_router_port for server.origin ([#99](#99))
khalwat added a commit that referenced this pull request Feb 7, 2025
…rigin, add support for custom https_router_port for server.origin ([#99](#99))
@mandrasch mandrasch deleted the patch-2 branch February 9, 2025 07:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants