From 18ef37393e9cd59adda8860e60b111fec0990ee1 Mon Sep 17 00:00:00 2001 From: "Juan Picado @jotadeveloper" Date: Sat, 24 Feb 2018 11:41:36 +0100 Subject: [PATCH] test: add unit test for isUplinkValid #571 --- src/lib/up-storage.js | 104 ++++++++++++-------------- test/functional/sanity/nullstorage.js | 24 +++--- test/unit/up-storage.spec.js | 81 +++++++++++++++++--- 3 files changed, 130 insertions(+), 79 deletions(-) diff --git a/src/lib/up-storage.js b/src/lib/up-storage.js index 18869f6ce..013ac2aa2 100644 --- a/src/lib/up-storage.js +++ b/src/lib/up-storage.js @@ -7,19 +7,17 @@ import _ from 'lodash'; import request from 'request'; import Stream from 'stream'; import URL from 'url'; -import {parseInterval, isObject, ErrorCode} from './utils'; +import {parseInterval, is_object, ErrorCode} from './utils'; import {ReadTarball} from '@verdaccio/streams'; import type { IProxy, Config, - UpLinkConf, Callback, - Headers, Logger, } from '@verdaccio/types'; -// import type {IUploadTarball, IReadTarball} from '@verdaccio/streams'; +import type {IUploadTarball} from '@verdaccio/streams'; const LoggerApi = require('./logger'); const encode = function(thing) { @@ -44,15 +42,15 @@ const setConfig = (config, key, def) => { * (same for storage.js, local-storage.js, up-storage.js) */ class ProxyStorage implements IProxy { - config: UpLinkConf; + config: Config; failed_requests: number; userAgent: string; ca: string | void; logger: Logger; server_id: string; url: any; - maxage: number; - timeout: number; + maxage: string; + timeout: string; max_fails: number; fail_timeout: number; upname: string; @@ -78,11 +76,11 @@ class ProxyStorage implements IProxy { this.config.url = this.config.url.replace(/\/$/, ''); - if (this.config.timeout && Number(this.config.timeout) >= 1000) { + if (Number(this.config.timeout) >= 1000) { this.logger.warn(['Too big timeout value: ' + this.config.timeout, - 'We changed time format to nginx-like one', - '(see http://nginx.org/en/docs/syntax.html)', - 'so please update your config accordingly'].join('\n')); + 'We changed time format to nginx-like one', + '(see http://nginx.org/en/docs/syntax.html)', + 'so please update your config accordingly'].join('\n')); } // a bunch of different configurable timers @@ -98,14 +96,14 @@ class ProxyStorage implements IProxy { * @param {*} cb * @return {Request} */ - request(options: any, cb?: Callback) { + request(options: any, cb: Callback) { let json; if (this._statusCheck() === false) { let streamRead = new Stream.Readable(); process.nextTick(function() { - if (cb) { + if (_.isFunction(cb)) { cb(ErrorCode.get500('uplink is offline')); } // $FlowFixMe @@ -133,7 +131,7 @@ class ProxyStorage implements IProxy { uri: uri, }, 'making request: \'@{method} @{uri}\''); - if (isObject(options.json)) { + if (is_object(options.json)) { json = JSON.stringify(options.json); headers['Content-Type'] = headers['Content-Type'] || 'application/json'; } @@ -144,7 +142,6 @@ class ProxyStorage implements IProxy { // $FlowFixMe processBody(err, body); logActivity(); - // $FlowFixMe cb(err, res, body); /** @@ -167,7 +164,7 @@ class ProxyStorage implements IProxy { } } - if (!err && isObject(body)) { + if (!err && is_object(body)) { if (_.isString(body.error)) { error = body.error; } @@ -179,8 +176,8 @@ class ProxyStorage implements IProxy { function logActivity() { let message = '@{!status}, req: \'@{request.method} @{request.url}\''; message += error - ? ', error: @{!error}' - : ', bytes: @{bytes.in}/@{bytes.out}'; + ? ', error: @{!error}' + : ', bytes: @{bytes.in}/@{bytes.out}'; self.logger.warn({ err: err, request: {method: method, url: uri}, @@ -264,9 +261,8 @@ class ProxyStorage implements IProxy { * @private */ _setAuth(headers: any) { - const auth = this.config.auth; - if (typeof auth === 'undefined' || headers['authorization']) { + if (_.isNil(this.config.auth) || headers['authorization']) { return headers; } @@ -277,12 +273,10 @@ class ProxyStorage implements IProxy { // get NPM_TOKEN http://blog.npmjs.org/post/118393368555/deploying-with-npm-private-modules // or get other variable export in env let token: any = process.env.NPM_TOKEN; - - if (auth.token) { - token = auth.token; - } else if (auth.token_env ) { - // $FlowFixMe - token = process.env[auth.token_env]; + if (this.config.auth.token) { + token = this.config.auth.token; + } else if (this.config.auth.token_env) { + token = process.env[this.config.auth.token_env]; } if (_.isNil(token)) { @@ -290,9 +284,8 @@ class ProxyStorage implements IProxy { } // define type Auth allow basic and bearer - const type = auth.type; + const type = this.config.auth.type; this._setHeaderAuthorization(headers, type, token); - return headers; } @@ -328,28 +321,25 @@ class ProxyStorage implements IProxy { * Eg: * * uplinks: - npmjs: - url: https://registry.npmjs.org/ - headers: - Accept: "application/vnd.npm.install-v2+json; q=1.0" - verdaccio-staging: - url: https://mycompany.com/npm - headers: - Accept: "application/json" - authorization: "Basic YourBase64EncodedCredentials==" + npmjs: + url: https://registry.npmjs.org/ + headers: + Accept: "application/vnd.npm.install-v2+json; q=1.0" + verdaccio-staging: + url: https://mycompany.com/npm + headers: + Accept: "application/json" + authorization: "Basic YourBase64EncodedCredentials==" * @param {Object} headers * @private */ - _overrideWithUplinkConfigHeaders(headers: Headers) { - if (!this.config.headers) { - return headers; - } - + _overrideWithUplinkConfigHeaders(headers: any) { // add/override headers specified in the config - /* eslint guard-for-in: 0 */ for (let key in this.config.headers) { + if (Object.prototype.hasOwnProperty.call(this.config.headers, key)) { headers[key] = this.config.headers[key]; + } } } @@ -359,10 +349,12 @@ class ProxyStorage implements IProxy { * @return {Boolean} */ isUplinkValid(url: string) { - // $FlowFixMe - url = URL.parse(url); - // $FlowFixMe - return url.protocol === this.url.protocol && url.host === this.url.host && url.path.indexOf(this.url.path) === 0; + // $FlowFixMe + url = URL.parse(url); + return (url.protocol === this.url.protocol) && + (url.host === this.url.host) && + // $FlowFixMe + (url.path.indexOf(this.url.path) === 0); } /** @@ -457,8 +449,8 @@ class ProxyStorage implements IProxy { * @return {Stream} */ search(options: any) { - const transformStream: any = new Stream.PassThrough({objectMode: true}); - const requestStream: stream$Readable = this.request({ + const transformStream: IUploadTarball = new Stream.PassThrough({objectMode: true}); + const requestStream: IUploadTarball = this.request({ uri: options.req.url, req: options.req, headers: { @@ -467,7 +459,7 @@ class ProxyStorage implements IProxy { }); let parsePackage = (pkg) => { - if (isObject(pkg)) { + if (is_object(pkg)) { transformStream.emit('data', pkg); } }; @@ -496,8 +488,6 @@ class ProxyStorage implements IProxy { }); transformStream.abort = () => { - // FIXME: this is clearly a potential issue - // $FlowFixMe requestStream.abort(); transformStream.emit('end'); }; @@ -521,8 +511,8 @@ class ProxyStorage implements IProxy { if (this.proxy === false) { headers['X-Forwarded-For'] = ( req && req.headers['x-forwarded-for'] - ? req.headers['x-forwarded-for'] + ', ' - : '' + ? req.headers['x-forwarded-for'] + ', ' + : '' ) + req.connection.remoteAddress; } } @@ -530,8 +520,8 @@ class ProxyStorage implements IProxy { // always attach Via header to avoid loops, even if we're not proxying headers['Via'] = req && req.headers['via'] - ? req.headers['via'] + ', ' - : ''; + ? req.headers['via'] + ', ' + : ''; headers['Via'] += '1.1 ' + this.server_id + ' (Verdaccio)'; } @@ -616,7 +606,7 @@ class ProxyStorage implements IProxy { if (this.proxy) { this.logger.debug({url: this.url.href, rule: noProxyItem}, 'not using proxy for @{url}, excluded by @{rule} rule'); - // $FlowFixMe + // $FlowFixMe this.proxy = false; } break; diff --git a/test/functional/sanity/nullstorage.js b/test/functional/sanity/nullstorage.js index 7ca9aa28d..a78a0bbe2 100644 --- a/test/functional/sanity/nullstorage.js +++ b/test/functional/sanity/nullstorage.js @@ -8,26 +8,28 @@ function getBinary() { export default function (server, server2) { - test('trying to fetch non-existent package / null storage', () => { - return server.getPackage('test-nullstorage-nonexist') - .status(404) - .body_error(/no such package/); + describe('should check whether test-nullstorage is on server1', () => { + test('trying to fetch non-existent package / null storage', () => { + return server.getPackage('test-nullstorage-nonexist') + .status(404) + .body_error(/no such package/); + }); }); - describe('test-nullstorage on server2', () => { + describe('should check whether test-nullstorage is on server2', () => { beforeAll(function() { return server2.addPackage('test-nullstorage2'); }); - test('creating new package - server2', () => {/* test for before() */}); + test('should creaate a new package on server2', () => {/* test for before() */}); - test('downloading non-existent tarball', () => { + test('should fails on download a non existent tarball', () => { return server.getTarball('test-nullstorage2', 'blahblah') .status(404) .body_error(/no such file/); }); - describe('tarball', () => { + describe('test and publish test-nullstorage2 package', () => { beforeAll(function() { return server2.putTarball('test-nullstorage2', 'blahblah', getBinary()) .status(201) @@ -42,9 +44,9 @@ export default function (server, server2) { .body_ok(/published/); }); - test('uploading new tarball', () => {/* test for before() */}); + test('should upload a new version for test-nullstorage2', () => {/* test for before() */}); - test('downloading newly created tarball', () => { + test('should fetch the newly created published tarball for test-nullstorage2', () => { return server.getTarball('test-nullstorage2', 'blahblah') .status(200) .then(function(body) { @@ -52,7 +54,7 @@ export default function (server, server2) { }); }); - test('downloading newly created package', () => { + test('should check whether the metadata for test-nullstorage2 match', () => { return server.getPackage('test-nullstorage2') .status(200) .then(function(body) { diff --git a/test/unit/up-storage.spec.js b/test/unit/up-storage.spec.js index f3b3849de..e19efaf32 100644 --- a/test/unit/up-storage.spec.js +++ b/test/unit/up-storage.spec.js @@ -5,26 +5,20 @@ import _ from 'lodash'; // $FlowFixMe import configExample from './partials/config'; import {setup} from '../../src/lib/logger'; - -import type {UpLinkConf, Config} from '@verdaccio/types'; +import type {IProxy, Config} from '@verdaccio/types'; setup([]); describe('UpStorge', () => { - const uplinkDefault: UpLinkConf = { - url: 'https://registry.npmjs.org/', - fail_timeout: '5m', - max_fails: 2, - maxage: '2m', - timeout: '1m', + const uplinkDefault = { + url: 'https://registry.npmjs.org/' }; - - let generateProxy = (config: UpLinkConf = uplinkDefault) => { + const generateProxy = (config: UpLinkConf = uplinkDefault): IProxy => { const appConfig: Config = new AppConfig(configExample); return new ProxyStorage(config, appConfig); - } + }; test('should be defined', () => { const proxy = generateProxy(); @@ -143,4 +137,69 @@ describe('UpStorge', () => { }); }); + describe('UpStorge::isUplinkValid', () => { + const validateUpLink = ( + url: string, + tarBallUrl?: string = `${url}/artifactory/api/npm/npm/pk1-juan/-/pk1-juan-1.0.7.tgz`) => { + const uplinkConf = { url }; + const proxy: IProxy = generateProxy(uplinkConf); + + return proxy.isUplinkValid(tarBallUrl); + } + + test('should validate tarball path against uplink', () => { + expect(validateUpLink('https://artifactory.mydomain.com')).toBe(true); + }); + + test('should validate tarball path against uplink case#2', () => { + expect(validateUpLink('https://artifactory.mydomain.com:443')).toBe(true); + }); + + test('should validate tarball path against uplink case#3', () => { + expect(validateUpLink('http://localhost')).toBe(true); + }); + + test('should validate tarball path against uplink case#4', () => { + expect(validateUpLink('http://my.domain.test')).toBe(true); + }); + + test('should validate tarball path against uplink case#5', () => { + expect(validateUpLink('http://my.domain.test:3000')).toBe(true); + }); + + // corner case https://github.com/verdaccio/verdaccio/issues/571 + test('should validate tarball path against uplink case#6', () => { + expect(validateUpLink('https://my.domain.test', + `https://my.domain.test:443/artifactory/api/npm/npm/pk1-juan/-/pk1-juan-1.0.7.tgz`)).toBe(false); + }); + + test('should fails on validate tarball path against uplink', () => { + const url = 'https://artifactory.mydomain.com'; + const tarBallUrl = 'https://localhost/api/npm/npm/pk1-juan/-/pk1-juan-1.0.7.tgz'; + const uplinkConf = { url }; + const proxy: IProxy = generateProxy(uplinkConf); + + expect(proxy.isUplinkValid(tarBallUrl)).toBe(false); + }); + + test('should fails on validate tarball path against uplink case#2', () => { + const url = 'https://localhost:5001'; + const tarBallUrl = 'https://localhost/api/npm/npm/pk1-juan/-/pk1-juan-1.0.7.tgz'; + const uplinkConf = { url }; + const proxy: IProxy = generateProxy(uplinkConf); + + expect(proxy.isUplinkValid(tarBallUrl)).toBe(false); + }); + + test('should fails on validate tarball path against uplink case#3', () => { + const url = 'http://localhost:5001'; + const tarBallUrl = 'https://localhost/api/npm/npm/pk1-juan/-/pk1-juan-1.0.7.tgz'; + const uplinkConf = { url }; + const proxy: IProxy = generateProxy(uplinkConf); + + expect(proxy.isUplinkValid(tarBallUrl)).toBe(false); + }); + + }); + });