Skip to content
This repository has been archived by the owner on Nov 1, 2023. It is now read-only.

Add support for canonical host #136

Merged
merged 2 commits into from
Jul 29, 2019
Merged

Add support for canonical host #136

merged 2 commits into from
Jul 29, 2019

Conversation

root-io
Copy link
Contributor

@root-io root-io commented Jun 25, 2019

Add support for canonical host (e.g for SEO purpose)

I was inspired by https://github.com/keepworks/heroku-buildpack-static but simplify the nginx config.

Resolve #60 #110 #135

@gpspake
Copy link

gpspake commented Jun 25, 2019

First of all, thanks for the PR; I think this is a common use case that isn't currently easy to handle
I'm testing this out and it's not behaving like I'd expect but maybe I don't understand.

I'd like to 301 https://www.devmemphis.org to https://devmemphis.org
right now, both are pointed at my app and working:

image

First, I've added the updated buildpack:

$ heroku buildpacks
=== devmemphis Buildpack URLs
1. heroku/nodejs
2. https://github.com/root-io/heroku-buildpack-static.git

Then I added canonical_host to static.json here as described in the readme changes (I've tried setting it to both "devmemphis.org" and "www.devmemphis.org")

{
  "root": "public/",
  "https_only": true,
  "error_page": "404",
  "canonical_host": "devmemphis.org",
  "headers": {
    "/**.js": {
      "Cache-Control": "public, max-age=0, must-revalidate"
    }
  }
}

My expectation was that it would redirect but it doesn't seem to be working. Am I missing something? 🤔

@gpspake
Copy link

gpspake commented Jun 25, 2019

Just to update, I noticed I was pointing to the wrong branch before but I've updated it and I still have the same issue:

$ heroku buildpacks                            
=== devmemphis Buildpack URLs                  
1. heroku/nodejs                                       
2. https://github.com/root-io/heroku-buildpack-static.git#canonical-host 

@root-io
Copy link
Contributor Author

root-io commented Jun 25, 2019

I have the same config you have, with the buildpack https://github.com/root-io/heroku-buildpack-static.git#canonical-host and it works.

Did you redeploy your app once you updated the buildpack ? Can you try with another browser, maybe there is some kind of cache, a conflic with a plugin like HTTPS everywhere etc

@gpspake
Copy link

gpspake commented Jun 25, 2019

@root-io You're right. It was just a matter of clearing the cache in my browser. I can confirm that this works for redirecting both www to non-www and vice versa. Props on adding this; thanks a lot!

@root-io
Copy link
Contributor Author

root-io commented Jun 25, 2019

Awesome, thanks for your feedback ! ;)

Copy link

@mhaylock mhaylock left a comment

Choose a reason for hiding this comment

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

nginx configuration is not a strength of mine, but I wonder whether there is a way to make sure the correct scheme is used even when the Heroku router sends all requests over HTTP rather than HTTPS?

scripts/config/templates/nginx.conf.erb Outdated Show resolved Hide resolved
@Aupajo
Copy link

Aupajo commented Jul 19, 2019

We're using this branch and would love to see this merged.

For anyone working at Heroku, the use-case for us is we're deploying a Single Page Application written in React to Heroku. The static buildpack simplifies delivery and reduces performance and security concerns. However, we have two subdomains that we want to handle with one application, and would like to redirect from the secondary host to the canonical host. Without this change merged, we have to choose between picking a buildpack fork that may not be kept up-to-date, adding the complexity and risks of adding a custom webserver, or writing a .profile/.profile.d hook to modify the ERB template. None of these is a great option.

Is it not a common use-case for us to have one application respond to more than one domain (e.g. a www domain to an apex domain)? I can imagine this helping other people deploying SPAs.

@stephanebruckert
Copy link

We need this too and would love to see it merged. Using https://github.com/keepworks/heroku-buildpack-static for now.

@joshwlewis
Copy link
Member

Thanks y'all. Bringing this in.

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

Successfully merging this pull request may close these issues.

URL Canonicalization
6 participants