fix: improve get headers from request #2190 (#2271)

* fix: improve request header handling * chore: fix test * chore: apply suggestion
verdaccio · May 26, 2021 · 38ca095 · 38ca095
1 parent b5ee703
commit 38ca095
Show file tree

Hide file tree

Showing 5 changed files with 27 additions and 11 deletions.
diff --git a/src/api/endpoint/api/whoami.ts b/src/api/endpoint/api/whoami.ts
@@ -3,7 +3,7 @@ import { $RequestExtend, $NextFunctionVer } from '../../../../types';
 
 export default function (route: Router): void {
   route.get('/whoami', (req: $RequestExtend, res: Response, next: $NextFunctionVer): void => {
-    if (req.headers.referer === 'whoami') {
+    if (req.get('referer') === 'whoami') {
       next({ username: req.remote_user.name });
     } else {
       next('route');

diff --git a/src/api/middleware.ts b/src/api/middleware.ts
@@ -110,7 +110,7 @@ export function validatePackage(req: $RequestExtend, res: $ResponseExtend, next:
 export function media(expect: string | null): any {
   return function (req: $RequestExtend, res: $ResponseExtend, next: $NextFunctionVer): void {
     if (req.headers[HEADER_TYPE.CONTENT_TYPE] !== expect) {
-      next(ErrorCode.getCode(HTTP_STATUS.UNSUPPORTED_MEDIA, 'wrong content-type, expect: ' + expect + ', got: ' + req.headers[HEADER_TYPE.CONTENT_TYPE]));
+      next(ErrorCode.getCode(HTTP_STATUS.UNSUPPORTED_MEDIA, 'wrong content-type, expect: ' + expect + ', got: ' + req.get(HEADER_TYPE.CONTENT_TYPE)));
     } else {
       next();
     }
@@ -134,7 +134,7 @@ export function expectJson(req: $RequestExtend, res: $ResponseExtend, next: $Nex
 
 export function antiLoop(config: Config): Function {
   return function (req: $RequestExtend, res: $ResponseExtend, next: $NextFunctionVer): void {
-    if (req.headers.via != null) {
+    if (req?.headers?.via != null) {
       const arr = req.headers.via.split(',');
 
       for (let i = 0; i < arr.length; i++) {
@@ -232,7 +232,7 @@ export function log(config: Config) {
       req.headers.authorization = '<Classified>';
     }
 
-    const _cookie = req.headers.cookie;
+    const _cookie = req.get('cookie');
     if (_.isNil(_cookie) === false) {
       req.headers.cookie = '<Classified>';
     }
@@ -277,7 +277,7 @@ export function log(config: Config) {
       }
       logHasBeenCalled = true;
 
-      const forwardedFor = req.headers['x-forwarded-for'];
+      const forwardedFor = req.get('x-forwarded-for');
       const remoteAddress = req.connection.remoteAddress;
       const remoteIP = forwardedFor ? `${forwardedFor} via ${remoteAddress}` : remoteAddress;
       let message;

diff --git a/src/lib/up-storage.ts b/src/lib/up-storage.ts
@@ -509,7 +509,8 @@ class ProxyStorage implements IProxy {
       uri: options.req.url,
       req: options.req,
       headers: {
-        referer: options.req.headers.referer,
+        // query for search
+        referer: options.req.get('referer'),
       },
     });
 
@@ -570,14 +571,14 @@ class ProxyStorage implements IProxy {
       // FIXME: proxy logic is odd, something is wrong here.
       // @ts-ignore
       if (!this.proxy) {
-        headers['X-Forwarded-For'] = (req.headers['x-forwarded-for'] ? req.headers['x-forwarded-for'] + ', ' : '') + req.connection.remoteAddress;
+        headers['x-forwarded-for'] = (req.get('x-forwarded-for') ? req.get('x-forwarded-for') + ', ' : '') + req.connection.remoteAddress;
       }
     }
 
     // always attach Via header to avoid loops, even if we're not proxying
-    headers['Via'] = req && req.headers['via'] ? req.headers['via'] + ', ' : '';
+    headers['via'] = req && req.get('via') ? req.get('via') + ', ' : '';
 
-    headers['Via'] += '1.1 ' + this.server_id + ' (Verdaccio)';
+    headers['via'] += '1.1 ' + this.server_id + ' (Verdaccio)';
   }
 
   /**

diff --git a/src/lib/utils.ts b/src/lib/utils.ts
@@ -156,7 +156,7 @@ const memoizedgetPublicUrl = memoizee(getPublicUrl);
  * @return {String} a parsed url
  */
 export function getLocalRegistryTarballUri(uri: string, pkgName: string, req: Request, urlPrefix: string | void): string {
-  const currentHost = req.headers.host;
+  const currentHost = req.get('host');
 
   if (!currentHost) {
     return uri;
@@ -646,7 +646,7 @@ export function getPublicUrl(url_prefix: string = '', req): string {
       throw new Error('invalid host');
     }
     const protoHeader = process.env.VERDACCIO_FORWARDED_PROTO ?? HEADERS.FORWARDED_PROTO;
-    const protocol = getWebProtocol(req.get(protoHeader), req.protocol);
+    const protocol = getWebProtocol(req.get(protoHeader.toLowerCase()), req.protocol);
     const combinedUrl = combineBaseUrl(protocol, host, url_prefix);
     debug('public url by request %o', combinedUrl);
     return combinedUrl;

diff --git a/test/unit/modules/utils/utils.spec.ts b/test/unit/modules/utils/utils.spec.ts
@@ -887,5 +887,20 @@ describe('Utilities', () => {
       expect(getPublicUrl(undefined, req)).toEqual('http://some.com/');
       delete process.env.VERDACCIO_FORWARDED_PROTO;
     });
+
+    test('with the VERDACCIO_FORWARDED_PROTO undefined', () => {
+      process.env.VERDACCIO_FORWARDED_PROTO = undefined;
+      const req = httpMocks.createRequest({
+        method: 'GET',
+        headers: {
+          host: 'some.com',
+          [HEADERS.FORWARDED_PROTO]: 'https',
+        },
+        url: '/',
+      });
+
+      expect(getPublicUrl('/test/', req)).toEqual('http://some.com/test/');
+      delete process.env.VERDACCIO_FORWARDED_PROTO;
+    });
   });
 });