Skip to content

docs: use regex for Vite server.cors to support additional hostnames#313

Merged
mandrasch merged 13 commits into
mainfrom
mandrasch-vite-server-cors-regex
Feb 5, 2025
Merged

docs: use regex for Vite server.cors to support additional hostnames#313
mandrasch merged 13 commits into
mainfrom
mandrasch-vite-server-cors-regex

Conversation

@mandrasch
Copy link
Copy Markdown
Collaborator

@mandrasch mandrasch commented Feb 4, 2025

The Issue

Article: https://ddev.com/blog/working-with-vite-in-ddev/

How This PR Solves The Issue

Update article to use regex:

 cors: {
      origin: /https?:\/\/([A-Za-z0-9\-\.]+)?(\.ddev\.site)(?::\d+)?$/,
    },

Supports requests from additional hostnames (and non-standard HTTPs ports for DDEV usage), assumes using .ddev.site as local project domain:

https://my-project.ddev.site
https://my-project.ddev.site:8888

Automagic regex by Stas, when you have a custom project_tld:

cors: {
  origin: new RegExp(
    `https?:\/\/(${process.env.DDEV_HOSTNAME.split(",")
      .map((h) => h.replace("*", "[^.]+"))
      .join("|")})(?::\\d+)?$`
  )
}

Generates /https?:\/\/(test-vite.ddev.site|test-vite-second-page.ddev.site)(?::\d+)?$/ from process.env.DDEV_HOSTNAME (this can has values like test-vite.ddev.site, test-vite.ddev.site,test-vite-second-page.ddev.site or wildcards ⚠️ such as *.test- see additional hostnames)

Manual Testing Instructions

Demo repos with that config:

Automated Testing Overview

Related Issue Link(s)

Release/Deployment Notes

@mandrasch
Copy link
Copy Markdown
Collaborator Author

mandrasch commented Feb 4, 2025

Oh sorry, just noticed I created my branch on ddev.com (instead of my fork) - should I remove it and start again?

@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages Bot commented Feb 4, 2025

Deploying ddev-com-front-end with  Cloudflare Pages  Cloudflare Pages

Latest commit: 2bc3c9b
Status: ✅  Deploy successful!
Preview URL: https://aea65de4.ddev-com-front-end.pages.dev
Branch Preview URL: https://mandrasch-vite-server-cors-r.ddev-com-front-end.pages.dev

View logs

@stasadev
Copy link
Copy Markdown
Member

stasadev commented Feb 4, 2025

It's okay to continue here. (IIRC cloudflare pages are not deployed in forks.)

Please rebase:

image

@mandrasch mandrasch force-pushed the mandrasch-vite-server-cors-regex branch from 5cf4ccc to f5c603e Compare February 4, 2025 15:36
@mandrasch
Copy link
Copy Markdown
Collaborator Author

ready for review/merge ✅

cors: { origin: process.env.DDEV_PRIMARY_URL },
// Configure CORS for the devserver (security)
cors: {
origin: /https?:\/\/([A-Za-z0-9\-\.]+)?(localhost|\.site)(?::\d+)?$/,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use process.env.DDEV_TLD here?

People can use different project_tld instead of ddev.site.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to check https://stackoverflow.com/questions/494035/how-do-you-use-a-variable-in-a-regular-expression 🤔

(Might be maybe simpler to add a section with a note about custom tlds and that users need to change it)

Copy link
Copy Markdown
Member

@stasadev stasadev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Try this, it works for me not only for additional_hostnames, but also for:

ddev config --additional-fqdns=example.test

And it works for https://localhost:32803 and https://127.0.0.1:32803 from ddev describe

Comment thread src/content/blog/working-with-vite-in-ddev.md Outdated
Comment thread src/content/blog/working-with-vite-in-ddev.md Outdated
Comment thread src/content/blog/working-with-vite-in-ddev.md Outdated
@stasadev
Copy link
Copy Markdown
Member

stasadev commented Feb 4, 2025

Edit:

It didn't work for wildcards ddev config --additional-hostnames='*.test'

Updating a regex to make it work:

-origin: new RegExp(`https?:\/\/(${process.env.DDEV_HOSTNAME.split(',').join('|')}|localhost|127.0.0.1)(?::\\d+)?$`),
+origin: new RegExp(`https?:\/\/(${process.env.DDEV_HOSTNAME.split(',').map(h => h.replace('*', '[^.]+')).join('|')}|localhost|127.0.0.1)(?::\\d+)?$`),

I think this covers all possible cases in DDEV.

@tyler36
Copy link
Copy Markdown
Contributor

tyler36 commented Feb 4, 2025

Nice catch!

Could we get a test added for this too? (It feels like there have been alot of "additional host" issues over the last year.)

@mandrasch
Copy link
Copy Markdown
Collaborator Author

mandrasch commented Feb 5, 2025

  1. @stasadev Awesome regex snippet 👍 But I don't want to get users lost in complexity for simple use cases. My fear is also that if something with process.env.DDEV_HOSTNAME changes in future, all Vite project configs must be changed. I therefore added your regex as optional (see latest changes). I try to keep it as simple as possible since I currently hear some annoyed users for the recent Vite changes (since they do not have time to fiddle around with configs / just want to have a working config). Not the fault of DDEV, Vite released this breaking change in a minor version. Therefore my goal is to avoid future disruptions as much as possible ;)

Personally, I'd prefer the simple solution which can be manually adapted and is more readable.

    cors: {
      origin: /https?:\/\/([A-Za-z0-9\-\.]+)?(\.site)(?::\d+)?$/,
    },

And I don't really want to support problems with the more complicated RegEx - like unforeseen edge cases, etc. Takes more time to test it with and without (all possible) additional hostname configs.

Is this alright with you, Stas? In the end, your call of course. :)

  1. @stasadev @tyler36 I removed 127.0.0.1 and localhost from the origins since they will never be used. All requests will be made from the DDEV URL(s) - please correct me if this is not true?
    cors: {
      origin: /https?:\/\/([A-Za-z0-9\-\.]+)?(\.site)(?::\d+)?$/,
    },
cors: {
  origin: new RegExp(
    `https?:\/\/(${process.env.DDEV_HOSTNAME.split(",")
      .map((h) => h.replace("*", "[^.]+"))
      .join("|")})(?::\\d+)?$`
  )
}

Pushed my latest changes

// Configure CORS securely for the Vite dev server to allow requests
// from *.ddev.site domains, supports additional hostnames (via regex)
cors: {
origin: /https?:\/\/([A-Za-z0-9\-\.]+)?(\.site)(?::\d+)?$/,
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for writing the post 🙏

I think this regex should be

Suggested change
origin: /https?:\/\/([A-Za-z0-9\-\.]+)?(\.site)(?::\d+)?$/,
origin: /https?:\/\/([A-Za-z0-9\-\.]+)?(\.ddev\.site)(?::\d+)?$/,

Otherwise, an attacker can serve their malicious script on *.site and the regex in the PR will also that script to fetch the content.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @sapphi-red, thanks very much that you also review here 👍

(we had a discussion here vitejs/vite#19345, sapphi-red is vitejs core team member)

Oh, I wasn't aware that .site is also an official TLD. Thought it was only local.

I'll change it.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed ✅

Copy link
Copy Markdown
Member

@stasadev stasadev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I like the simplicity.

And I found another edge case 😅, it doesn't work with this:

ddev config --router-https-port 8443

See my suggestion.

This also requires an update in https://github.com/mandrasch/ddev-vite-simple-demo/blob/c6e97c815d7071f346a4af10b1641287d2666e82/index.php#L10-L11

(I used only this repo for my tests, didn't check another one.)

-<?php echo $_SERVER['DDEV_PRIMARY_URL']; ?>:5173
+<?php echo preg_replace('/:\d+$/', '', $_SERVER['DDEV_PRIMARY_URL']); ?>:5173

Comment thread src/content/blog/working-with-vite-in-ddev.md Outdated
Comment thread src/content/blog/working-with-vite-in-ddev.md Outdated
Comment thread src/content/blog/working-with-vite-in-ddev.md Outdated
@mandrasch
Copy link
Copy Markdown
Collaborator Author

mandrasch commented Feb 5, 2025

@stasadev okay, wow thanks - great that you catch this now, before we merged the whole thing 🙏

I'll updated all instances in the article and try to give a clear and readable comment so everyone knows what's happening. Maybe a bit verbose, but DDEV users will just strip the comments for their projects I assume.

 // Adjust Vites dev server for DDEV
  // https://vitejs.dev/config/server-options.html
  server: {
    // Respond to all network requests
    host: "0.0.0.0",
    port: 5173,
    strictPort: true,
    // Defines the origin of the generated asset URLs during development, this must be set to the
    // Vite dev server URL and selected port. In general, `process.env.DDEV_PRIMARY_URL` will give
    // us the primary URL of the DDEV project, e.g. "https://test-vite.ddev.site". But since DDEV
    // can be configured to use another port (via `router_https_port`), the output can also be
    // "https://test-vite.ddev.site:1234". Therefore we need to strip a port number like ":1234"
    // before adding Vites port to achieve the desired output of "https://test-vite.ddev.site:5173".
    origin: `${process.env.DDEV_PRIMARY_URL.replace(/:\d+$/, "")}:5173`,
    // Configure CORS securely for the Vite dev server to allow requests from *.ddev.site domains,
    // supports additional hostnames (via regex). If you use another `project_tld`, adjust this.
    cors: {
      origin: /https?:\/\/([A-Za-z0-9\-\.]+)?(\.ddev\.site)(?::\d+)?$/,
    },
  },

The demo is also updated (https://github.com/mandrasch/ddev-vite-simple-demo)

Hope that is it now, good to go from my point of view. 👍

Comment thread src/content/blog/working-with-vite-in-ddev.md Outdated
Comment thread src/content/blog/working-with-vite-in-ddev.md
mandrasch and others added 2 commits February 5, 2025 14:21
Comment thread src/content/blog/working-with-vite-in-ddev.md
Copy link
Copy Markdown
Member

@stasadev stasadev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, thank you!

@mandrasch mandrasch merged commit 3d05056 into main Feb 5, 2025
@mandrasch mandrasch deleted the mandrasch-vite-server-cors-regex branch February 5, 2025 13:49
@rfay
Copy link
Copy Markdown
Member

rfay commented Feb 5, 2025

Thanks all!

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.

5 participants