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

fix: forward host header to dynamic source endpoint #280

Merged

Conversation

chenyuncai
Copy link
Contributor

Try to get right host in danymic source eventhandler and give right right urls back.

It's useful in some saas platform with custom domain support, The sitemap need to be different for each Tenant

@chenyuncai
Copy link
Contributor Author

Hi @harlan-zw , is this PR useful or anything I need change?

@harlan-zw
Copy link
Collaborator

harlan-zw commented May 14, 2024

Hi @chenyuncai, thank you for the PR 🙌!

There is one thing I need to investigate before merging this which is blocking it. I believe there is a way to call fetch where it will automatically forward these headers. I think it may be the event.fetch or something.

We should use as it will fix the problem for all use cases not just the host.

If it doesn't work like this then we should be forwarded relevant headers.

@chenyuncai
Copy link
Contributor Author

Yes, that's right, maybe we should send all headers to self event handler, and only safe headers to third-party sources.

Thanks for your response.

@harlan-zw
Copy link
Collaborator

That also does seem reasonable 🤔 I'll have a think during the day and revisit this tonight

@harlan-zw harlan-zw changed the title fix: send real host to dynamic source endpoint so we can get right sitemap urls in Multi-Tenant System fix: forward host header to dynamic source endpoint May 22, 2024
@harlan-zw harlan-zw merged commit 85456e2 into nuxt-modules:main May 22, 2024
1 check passed
@harlan-zw
Copy link
Collaborator

Thanks for the PR, I've merged this into 5.2.0 with a minor fix to avoid empty Host headers being passed.

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.

3 participants