reactioncommerce/example-storefront

Sitemap Handler produces urls with undefined protocol

janus-reith opened this issue · 2 comments

Type: minor

Describe the bug
The sitemap handler uses req.protocol to determine wether to use http or https for the links it returns:

const shopUrl = `${req.protocol}://${req.headers.host}`;

req.protocol is an express feature not present in nextsjs api routes.
All it does is return req.connection.encrypted ? "https" : "http"

To Reproduce
Steps to reproduce the behavior:

  1. Set up a Sitemap in the api
  2. Check /sitemap.xml on your storefront, urls start with undefined://

Expected behavior
The urls in the sitemap contain the protocol that was used when requesting the sitemap.

Additional context
I tried to easily fix this by adding req.connection.encrypted ? "https" : "http" in the place of req.protocol, but the result on production deployments was that it always returns http - Could be, because the Reverse Proxy is handling SSL Termiantion, so nextjs is in fact not using https internally. This would mean that the same issue should've been present in older releases using express, but I currently have no production deployment for that at hand to verify it.
Maybe expess also does other thing to determine the requested url, and to my surprise I found no quick way in the next api routes to retrieve it properly.

Solution
I think the original approach of building the shop url by parsing req.protocol and req.headers.host separetely to get the url isn't really necessary anyway, and we could just resort to using CANONICAL_URL for this instead.

@willopez Maybe we could merge a quick fix prior to the next release? - I will prepare something now.

Sounds good