feat: add support for multiple protocol on protocol header (#1014)

More context #695
verdaccio · Sep 25, 2018 · 40e2b10 · 40e2b10
1 parent c3edcbf
commit 40e2b10
Show file tree

Hide file tree

Showing 4 changed files with 54 additions and 10 deletions.
diff --git a/src/api/web/index.js b/src/api/web/index.js
@@ -7,7 +7,7 @@ import express from 'express';
 
 import * as Utils from '../../lib/utils';
 import Search from '../../lib/search';
-import {HTTP_STATUS, WEB_TITLE} from '../../lib/constants';
+import {HEADERS, HTTP_STATUS, WEB_TITLE} from '../../lib/constants';
 
 const {securityIframe} = require('../middleware');
 /* eslint new-cap:off */
@@ -56,7 +56,10 @@ module.exports = function(config, auth, storage) {
   });
 
   router.get('/', function(req, res) {
-    const base = Utils.combineBaseUrl(Utils.getWebProtocol(req), req.get('host'), config.url_prefix);
+    const base = Utils.combineBaseUrl(
+      Utils.getWebProtocol(req.get(HEADERS.FORWARDED_PROTO), req.protocol),
+      req.get('host'), config.url_prefix);
+
     let webPage = template
       .replace(/ToReplaceByVerdaccio/g, base)
       .replace(/ToReplaceByTitle/g, _.get(config, 'web.title') ? config.web.title : WEB_TITLE)

diff --git a/src/lib/constants.js b/src/lib/constants.js
@@ -14,6 +14,7 @@ export const csrPem = 'verdaccio-csr.pem';
 export const HEADERS = {
   JSON: 'application/json',
   CONTENT_TYPE: 'Content-type',
+  FORWARDED_PROTO: 'X-Forwarded-Proto',
   ETAG: 'ETag',
   JSON_CHARSET: 'application/json; charset=utf-8',
   OCTET_STREAM: 'application/octet-stream; charset=utf-8',
@@ -123,4 +124,4 @@ export const PACKAGE_ACCESS = {
 
 export const UPDATE_BANNER = {
   CHANGELOG_URL: 'https://github.com/verdaccio/verdaccio/releases/tag/'
-}
+}
diff --git a/src/lib/utils.js b/src/lib/utils.js
@@ -15,7 +15,7 @@ import {
   DEFAULT_PORT,
   DEFAULT_DOMAIN,
   DEFAULT_PROTOCOL,
-  CHARACTER_ENCODING
+  CHARACTER_ENCODING, HEADERS
 } from './constants';
 import {generateGravatarUrl} from '../utils/user';
 
@@ -193,7 +193,7 @@ export function getLocalRegistryTarballUri(
   }
   const tarballName = extractTarballFromUrl(uri);
   const domainRegistry = combineBaseUrl(
-    getWebProtocol(req),
+    getWebProtocol(req.get(HEADERS.FORWARDED_PROTO), req.protocol),
     req.headers.host,
     urlPrefix
   );
@@ -377,11 +377,14 @@ export function parseInterval(interval: any): number {
 
 /**
  * Detect running protocol (http or https)
- * @param {*} req
- * @return {String}
  */
-export function getWebProtocol(req: $Request): string {
-  return req.get('X-Forwarded-Proto') || req.protocol;
+export function getWebProtocol(headerProtocol: string | void, protocol: string): string {
+  if (typeof(headerProtocol) === 'string' && headerProtocol !== '') {
+    const commaIndex = headerProtocol.indexOf(',');
+    return commaIndex > 0 ? headerProtocol.substr(0, commaIndex) : headerProtocol;
+  }
+
+  return protocol;
 }
 
 export function getLatestVersion(pkgInfo: Package): string {

diff --git a/test/unit/api/utils.spec.js b/test/unit/api/utils.spec.js
@@ -7,7 +7,14 @@ import {
   validateName,
   convertDistRemoteToLocalTarballUrls,
   parseReadme,
-  addGravatarSupport, validatePackage, validateMetadata, DIST_TAGS, combineBaseUrl, getVersion, normalizeDistTags
+  addGravatarSupport,
+  validatePackage,
+  validateMetadata,
+  DIST_TAGS,
+  combineBaseUrl,
+  getVersion,
+  normalizeDistTags,
+  getWebProtocol
 } from '../../../src/lib/utils';
 import Logger, { setup } from '../../../src/lib/logger';
 import { readFile } from '../../functional/lib/test.utils';
@@ -39,6 +46,36 @@ describe('Utilities', () => {
   const cloneMetadata = (pkg = metadata) => Object.assign({}, pkg);
 
   describe('API utilities', () => {
+    describe('getWebProtocol', () => {
+      test('should handle undefined header', () => {
+        expect(getWebProtocol(undefined, 'http')).toBe('http');
+      });
+
+      test('should handle emtpy string', () => {
+        expect(getWebProtocol('', 'http')).toBe('http');
+      });
+
+      test('should have header priority over request protocol', () => {
+        expect(getWebProtocol("https", 'http')).toBe('https');
+      });
+
+      test('should have handle empty protocol', () => {
+        expect(getWebProtocol("https", '')).toBe('https');
+      });
+
+      describe('getWebProtocol and HAProxy variant', () => {
+        // https://github.com/verdaccio/verdaccio/issues/695
+
+        test('should handle http', () => {
+          expect(getWebProtocol("http,http", 'https')).toBe('http');
+        });
+
+        test('should handle https', () => {
+          expect(getWebProtocol("https,https", 'http')).toBe('https');
+        });
+      });
+    });
+
     describe('convertDistRemoteToLocalTarballUrls', () => {
       test('should build a URI for dist tarball based on new domain', () => {
         const convertDist = convertDistRemoteToLocalTarballUrls(cloneMetadata(),