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

Incorrect server request url on server side, when deployed on ccv2 #11016

Closed
dunqan opened this issue Feb 5, 2021 · 5 comments · Fixed by #11056
Closed

Incorrect server request url on server side, when deployed on ccv2 #11016

dunqan opened this issue Feb 5, 2021 · 5 comments · Fixed by #11056
Assignees
Labels
bug Something isn't working P2-Snow-High
Milestone

Comments

@dunqan
Copy link
Contributor

dunqan commented Feb 5, 2021

This mostly affects Automatic Multi-Site Configuration that in many cases won't be able to resolve proper base site.

How to reproduce:

  1. Add this snippet to the app.module.ts
  constructor(
    @Optional()
    @Inject(SERVER_REQUEST_URL)
    protected serverRequestUrl?: string
  ) {
    console.log('serverRequestUrl', serverRequestUrl);
  }
  1. Check SSR logs (serverRequestUrl will contain local address, instead of request
@dunqan dunqan added the bug Something isn't working label Feb 5, 2021
@Platonn
Copy link
Contributor

Platonn commented Feb 5, 2021

Blocks #10903

@dunqan dunqan self-assigned this Feb 5, 2021
@dunqan dunqan added this to Newly Added in Spartacus Bug Board [can close board?] via automation Feb 5, 2021
@dunqan dunqan moved this from Newly Added to In Progress in Spartacus Bug Board [can close board?] Feb 5, 2021
@dunqan
Copy link
Contributor Author

dunqan commented Feb 8, 2021

We are working on a hotfix that will be included in the next patch release.
Before it will happen, here is a workaround:

  1. Copy this code snippet, preferably to separate file alongside app.module.ts app.server.module.ts
import { StaticProvider } from '@angular/core';
import { REQUEST } from '@nguniversal/express-engine/tokens';
import { Request } from 'express';
import { SERVER_REQUEST_ORIGIN, SERVER_REQUEST_URL } from '@spartacus/core';

export const fixServerRequestProviders: StaticProvider[] = [
  {
    provide: SERVER_REQUEST_URL,
    useFactory: getRequestUrl,
    deps: [REQUEST],
  },
  {
    provide: SERVER_REQUEST_ORIGIN,
    useFactory: getRequestOrigin,
    deps: [REQUEST],
  },
];

function getRequestUrl(req: Request): string {
  return getRequestOrigin(req) + req.originalUrl;
}

function getRequestOrigin(req: Request): string {
  // If express is resolving and trusting X-Forwarded-Host, we want to take it
  // into an account to properly generate request origin.
  const trustProxyFn = req.app.get('trust proxy fn');
  let forwardedHost = req.get('X-Forwarded-Host');
  if (forwardedHost && trustProxyFn(req.connection.remoteAddress, 0)) {
    if (forwardedHost.indexOf(',') !== -1) {
      // Note: X-Forwarded-Host is normally only ever a
      //       single value, but this is to be safe.
      forwardedHost = forwardedHost
        .substring(0, forwardedHost.indexOf(','))
        .trimRight();
    }
    return req.protocol + '://' + forwardedHost;
  } else {
    return req.protocol + '://' + req.get('host');
  }
}
  1. Add fixServerRequestProviders to providers array in app.module.ts app.server.module.ts
providers: [
  // ...
  ...fixServerRequestProviders
],
  1. Enable trust proxy option in server.ts file:
  server.set('trust proxy', 'loopback');

(below const server = express(); line)

dunqan added a commit that referenced this issue Feb 9, 2021
…proxy configuration (#11056)

This fix requies additional change for existing `server.ts` implementation:
 server.set('trust proxy', 'loopback')

Closes #11016
dunqan added a commit that referenced this issue Feb 9, 2021
…proxy configuration (#11058)

Backport of #11056 for 3.0
Closes #11016

This fix requies additional change for existing `server.ts` implementation:
server.set('trust proxy', 'loopback')
@Platonn
Copy link
Contributor

Platonn commented Feb 24, 2021

Edited above workaround. Changed app.module.ts to app.server.module.ts. Otherwise in CSR it throws an error:

 NullInjectorError: No provider for InjectionToken REQUEST! ; Zone: <root> ; Task: Promise.then ; Value: NullInjectorError: R3InjectorError(AppModule)[ApplicationModule -> ApplicationRef -> ApplicationInitStatus -> InjectionToken Application Initializer -> [object Object] -> InjectionToken ConfigInitializer -> [object Object] -> OccConfigLoaderService -> InjectionToken SERVER_REQUEST_URL -> InjectionToken REQUEST -> InjectionToken REQUEST -> InjectionToken REQUEST]: 
  NullInjectorError: No provider for InjectionToken REQUEST!

The SERVER_REQUEST_ORIGIN and SERVER_REQUEST_URL should not be provided in CSR by design.

@Platonn
Copy link
Contributor

Platonn commented Apr 19, 2021

Edited the above workaround with regards to the latest bugfix 37e8926

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working P2-Snow-High
Projects
No open projects
4 participants