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

Node Proxy: query string values aren't transferred properly #140

Closed
coffee-lover16 opened this issue Jan 23, 2019 · 8 comments
Closed

Node Proxy: query string values aren't transferred properly #140

coffee-lover16 opened this issue Jan 23, 2019 · 8 comments
Assignees

Comments

@coffee-lover16
Copy link

Description

The node proxy with sitecore-jss-proxy package does not transfer querystring values properly. As result it returns 404 error to client when query string value wasn't passed properly.

Expected behavior

The node proxy should pass query string values and shouldn't return 404 error if it isn't page

Steps To Reproduce

  1. Use the headless SSR via sitecore-jss-proxy
  2. Go to the sitecore admin and create a campaign
  3. It will provide the campaign link (like sc_camp=1D53C9EF8A9D413BE7B254EB4C8A6E3E)
  4. Try to do a request via browser like {DOMAIN}/?sc_camp={CAMP}
  5. It throws the 404 error

Your Environment

  • Sitecore Version: 9.0.2
  • JSS Version: 11.0.2
  • Browser Name and version: Chrome
  • Operating System and version (desktop or mobile): client: Win 10, Node proxy: Win 10, CentOS 7, Sitecore host: Win 10, Windows 2016
@coffee-lover16
Copy link
Author

There is workaround for packages -> sitecore-jss-proxy -> src -> index.ts

Just replace line in the rewriteRequestPath method:

const routeParams = parseRouteUrl(decodedReqPath);

to:

const routeParams = parseRouteUrl(finalReqPath);

Be careful as it may corrupt the method with same name in app

@jplwood
Copy link

jplwood commented Aug 9, 2019

@aweber1 Any updates on this or anyone know how this changes in 9.1? Still seeing query strings breaking routing in the node-jss-proxy.

@aweber1
Copy link
Contributor

aweber1 commented Aug 9, 2019

Two new workarounds, neither require forking or patching.

Option 1:

Assuming you've used the JSS sample as a basis, in server/server.js in your JSS app:

  • Add new import
import { URL } from 'url'; // standard node lib, only available on server
  • Modify the parseRouteUrl function
// parse the `url` value provided by the SSR proxy
const parsedUrl = new URL(url, 'http://anyvalue.jss');
routePatterns.some((pattern) => {
  // Pass `parsedUrl.pathname` to `matchPath`. This ensures that the matched
  // params returned by the router won't contain any querystring value found in `url`.
  const match = matchPath(parsedUrl.pathname, { path: pattern });
  if (match && match.params) {
    result = match.params;
    return true;
  }

  return false;
});

Option 2:

Modify config.js for the node ssr proxy sample app.

NOTE: the code below is not replacement for the config.js file, you need to merge the code into the existing config.js file.

// Resolve `layoutServiceRoute` and `apiHost` outside of the `config` object
// so that we can use the values when determining whether or not to rewrite
// the proxy request path.
const layoutServiceRoute = process.env.SITECORE_LAYOUT_SERVICE_ROUTE || '/sitecore/api/layout/render/jss';
const apiHost = process.env.SITECORE_API_HOST || '';

// New function to remove erroneous querystring value(s) from the `item` parameter
// that is passed to the Layout Service endpoint.
const fixProxyRequestPath = (requestPath) => {
  // We only need to fix layout service requests, ignore anything else.
  if (requestPath.toLowerCase().indexOf(layoutServiceRoute.toLowerCase()) === -1) {
    return requestPath;
  }

  const parsedUrl = new URL(requestPath, apiHost);
  if (!parsedUrl.search || !parsedUrl.searchParams.has('item')) {
    return requestPath;
  }

  const itemPath = decodeURIComponent(parsedUrl.searchParams.get('item'));
  const qsIndex = itemPath.indexOf('?');
  if (qsIndex === -1) {
    return requestPath;
  }

  const correctedItemPath = itemPath.slice(0, qsIndex);
  parsedUrl.searchParams.set('item', correctedItemPath);
  return `${parsedUrl.pathname}${parsedUrl.search}`;
};

// In the `config` object, need to modify the `layoutServiceRoute` and `apiHost` properties to use
// the resolved values above.
// Also need to add a line to the end of `config.proxyOptions.onProxyReq` to call the
// `fixProxyRequestPath` function. It will adjust the path for layout service requests if needed.
const config = {
    // keep existing properties
    // ...
    
    // modify `config.layoutServiceRoute` to use the resolved `layoutServiceRoute` value from above
    layoutServiceHost,
    // modify `config.apiHost` to use the resolved `apiHost` value from above
    apiHost,
    
    proxyOptions: {
        // keep existing properties
        
        onProxyReq: (proxyReq, req, res) => {
          // keep existing code
          
          // fix/modify the proxy request path 
          proxyReq.path = fixProxyRequestPath(proxyReq.path);
        }
    }
    
    // keep existing properties
}

The underlying issue stems from this line in the JSS proxy:

const routeParams = parseRouteUrl(decodedReqPath);

Essentially, when the JSS proxy is rewriting a URL, for instance /about?sc_camp=1234, into a Layout Service URL it passes that URL into the parseRouteUrl function exposed by the server bundle.

That function attempts to invoke the app router to "match" the URL to a route pattern used in the app, and then return any route parameters back to the proxy. For instance, the url above /about?sc_camp=1234 would match on the /:sitecoreRoute* pattern defined in AppRoot.js, and the matching result object would be:

{
  sitecoreRoute: '/about?sc_camp=1234'
}

That sitecoreRoute value is then what is passed to the Layout Service endpoint for the item parameter. And therein is the issue. The item parameter should be a path to a Sitecore item or a guid, it shouldn't contain any querystring params.

So both of the workaround options above address the issue, but in different ways.

In option 1, we basically parse the URL provided by the JSS proxy, then use only the URL pathname when trying to match route patterns. The router can still do its pattern matching and extract route parameters as usual and we don't have to worry about querystring values leaking into the matched route parameters. This is the more elegant solution.

In option 2, we let everything happen as usual, but before the proxy request is made, we inspect the request and if it's a Layout Service request and the item parameter for the request has a querystring value, we just strip out that querystring value.

@jbreuer
Copy link
Contributor

jbreuer commented Sep 23, 2019

Not sure if it's related, but I have a custom route in Sitecore which isn't working on the JSS proxy.

This is the route:

var paymentInvoiceUrl = "invoice/pay/";

RouteTable.Routes.MapRoute(
    "PaymentInvoice",
    paymentInvoiceUrl + "{id}",
    new { scItemPath = paymentInvoiceUrl });

When I visit /invoice/pay/56e46c3c-19d5-458c-a1a5-58f7ba190e47 on CM or CD it works and I get correct page and I have the id in the route data. When I visit it on the JSS proxy I get a 404 message. Is MapRoute supported?

@jbreuer
Copy link
Contributor

jbreuer commented Sep 23, 2019

It now also works after adding to the pathRewriteExcludeRoutes in the config.js file:

/**
   * pathRewriteExcludeRoutes: A list of absolute paths
   * that are NOT app routes and should not attempt to render a route
   * using SSR. These route prefixes are directly proxied to the apiHost,
   * allowing the proxy to also proxy GraphQL requests, REST requests, etc.
   * Local static assets, Sitecore API paths, Sitecore asset paths, etc should be listed here.
   */
    pathRewriteExcludeRoutes: ['/dist', '/assets', '/sitecore/api', '/api', '/-/jssmedia', '/en/-/media', '/nl/-/media', '/-/media', '/sitemap.xml', '/robots.txt', '/invoice/pay/'],

@sc-illiakovalenko
Copy link
Contributor

sc-illiakovalenko commented Jun 26, 2020

@Eugenio161288 Fixed in commit

@jbreuer
Copy link
Contributor

jbreuer commented Jul 8, 2020

Would this fix also fix the following problem: #171 ?
Might be worth testing if the percentage symbol in query string also works.

@sc-illiakovalenko
Copy link
Contributor

@jbreuer No, it wasn't fixed. Currently, I'm working on it

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

No branches or pull requests

6 participants