Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
//registry.yarnpkg.com/:_authToken=abc123
//registry.yarnpkg.com/:always-auth=true
@types:registry=https://registry.yarnpkg.com
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
//registry.yarnpkg.com/:_authToken=abc123
//registry.yarnpkg.com/:always-auth=true
@types:registry=https://registry.yarnpkg.com/
2 changes: 1 addition & 1 deletion __tests__/package-resolver.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import * as fs from '../src/util/fs.js';
import * as constants from '../src/constants.js';
import inquirer from 'inquirer';

jasmine.DEFAULT_TIMEOUT_INTERVAL = 60000;
jasmine.DEFAULT_TIMEOUT_INTERVAL = 100000;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it mean that requests get slower after this change?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also can you add a test here for the supported case?


// automatically chose the first available version if cached does not fit
inquirer.prompt = jest.fn((questions) => {
Expand Down
2 changes: 2 additions & 0 deletions src/fetchers/base-fetcher.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ const path = require('path');
export default class BaseFetcher {
constructor(dest: string, remote: PackageRemote, config: Config) {
this.reporter = config.reporter;
this.packageName = remote.packageName;
this.reference = remote.reference;
this.registry = remote.registry;
this.hash = remote.hash;
Expand All @@ -24,6 +25,7 @@ export default class BaseFetcher {
reporter: Reporter;
remote: PackageRemote;
registry: RegistryNames;
packageName: ?string;
reference: string;
config: Config;
hash: ?string;
Expand Down
2 changes: 1 addition & 1 deletion src/fetchers/tarball-fetcher.js
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ export default class TarballFetcher extends BaseFetcher {
.on('error', reject);
}
},
});
}, this.packageName);
}

_fetch(): Promise<FetchedOverride> {
Expand Down
13 changes: 10 additions & 3 deletions src/registries/is-request-to-registry.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,25 @@

import url from 'url';

const SUFFIX_VISUALSTUDIO = '.pkgs.visualstudio.com';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I missed that detail earlier.
It looks hard to scale a maintain a solution where every registry has to be hardcoded in Yarn.
Can this setting be moved to .yarnrc for each registry to configure?


export default function isRequestToRegistry(requestUrl: string, registry: string): boolean {
const requestParsed = url.parse(requestUrl);
const registryParsed = url.parse(registry);
const requestHost = requestParsed.hostname || '';
const registryHost = registryParsed.hostname || '';
const requestPort = getPortOrDefaultPort(requestParsed.port, requestParsed.protocol);
const registryPort = getPortOrDefaultPort(registryParsed.port, registryParsed.protocol);
const requestPath = requestParsed.path || '';
const registryPath = registryParsed.path || '';

return (requestParsed.protocol === registryParsed.protocol) &&
(requestParsed.hostname === registryParsed.hostname) &&
(requestPort === registryPort) &&
requestPath.startsWith(registryPath);
(requestHost === registryHost) &&
(requestPort === registryPort) && (
requestPath.startsWith(registryPath) ||
// For pkgs.visualstudio.com, the package path does not prefix with the registry path
requestHost.endsWith(SUFFIX_VISUALSTUDIO)
);
}

function getPortOrDefaultPort(port: ?string, protocol: ?string): ?string {
Expand Down
73 changes: 45 additions & 28 deletions src/registries/npm-registry.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import * as fs from '../util/fs.js';
import NpmResolver from '../resolvers/registries/npm-resolver.js';
import envReplace from '../util/env-replace.js';
import Registry from './base-registry.js';
import {addSuffix, removePrefix} from '../util/misc';
import {addSuffix} from '../util/misc';
import isRequestToRegistry from './is-request-to-registry.js';

const userHome = require('../util/user-home-dir').default;
Expand All @@ -17,6 +17,8 @@ const url = require('url');
const ini = require('ini');

const DEFAULT_REGISTRY = 'https://registry.npmjs.org/';
const REGEX_REGISTRY_PREFIX = /^https?:/;
const REGEX_REGISTRY_SUFFIX = /registry\/?$/;

function getGlobalPrefix(): string {
if (process.env.PREFIX) {
Expand Down Expand Up @@ -50,16 +52,14 @@ export default class NpmRegistry extends Registry {
return name.replace('/', '%2f');
}

request(pathname: string, opts?: RegistryRequestOptions = {}): Promise<*> {
const registry = addSuffix(this.getRegistry(pathname), '/');
request(pathname: string, opts?: RegistryRequestOptions = {}, packageName: ?string): Promise<*> {
const registry = this.getRegistry(packageName || pathname);
const requestUrl = url.resolve(registry, pathname);
const alwaysAuth = this.getScopedOption(registry.replace(/^https?:/, ''), 'always-auth')
|| this.getOption('always-auth')
|| removePrefix(requestUrl, registry)[0] === '@';
const alwaysAuth = this.getRegistryOrGlobalOption(registry, 'always-auth');
Copy link

@mshustov mshustov Mar 15, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bestander does it make sense to force adding auth key to resoled url?
I faced similar problem some months ago with our artifactory.


const headers = {};
if (this.token || (alwaysAuth && isRequestToRegistry(requestUrl, registry))) {
const authorization = this.getAuth(pathname);
const authorization = this.getAuth(packageName || pathname);
if (authorization) {
headers.authorization = authorization;
}
Expand Down Expand Up @@ -152,15 +152,15 @@ export default class NpmRegistry extends Registry {
const availableRegistries = this.getAvailableRegistries();
const registry = availableRegistries.find((registry) => packageName.startsWith(registry));
if (registry) {
return registry;
return addSuffix(registry, '/');
}
}

for (const scope of [this.getScope(packageName), '']) {
const registry = this.getScopedOption(scope, 'registry')
|| this.registries.yarn.getScopedOption(scope, 'registry');
if (registry) {
return String(registry);
return addSuffix(String(registry), '/');
}
}

Expand All @@ -172,28 +172,26 @@ export default class NpmRegistry extends Registry {
return this.token;
}

for (let registry of [this.getRegistry(packageName), '', DEFAULT_REGISTRY]) {
registry = registry.replace(/^https?:/, '');
const registry = this.getRegistry(packageName);

// Check for bearer token.
let auth = this.getScopedOption(registry.replace(/\/?$/, '/'), '_authToken');
if (auth) {
return `Bearer ${String(auth)}`;
}
// Check for bearer token.
const authToken = this.getRegistryOrGlobalOption(registry, '_authToken');
if (authToken) {
return `Bearer ${String(authToken)}`;
}

// Check for basic auth token.
auth = this.getScopedOption(registry, '_auth');
if (auth) {
return `Basic ${String(auth)}`;
}
// Check for basic auth token.
const auth = this.getRegistryOrGlobalOption(registry, '_auth');
if (auth) {
return `Basic ${String(auth)}`;
}

// Check for basic username/password auth.
const username = this.getScopedOption(registry, 'username');
const password = this.getScopedOption(registry, '_password');
if (username && password) {
const pw = new Buffer(String(password), 'base64').toString();
return 'Basic ' + new Buffer(String(username) + ':' + pw).toString('base64');
}
// Check for basic username/password auth.
const username = this.getRegistryOrGlobalOption(registry, 'username');
const password = this.getRegistryOrGlobalOption(registry, '_password');
if (username && password) {
const pw = new Buffer(String(password), 'base64').toString();
return 'Basic ' + new Buffer(String(username) + ':' + pw).toString('base64');
}

return '';
Expand All @@ -202,4 +200,23 @@ export default class NpmRegistry extends Registry {
getScopedOption(scope: string, option: string): mixed {
return this.getOption(scope + (scope ? ':' : '') + option);
}

getRegistryOption(registry: string, option: string): mixed {
const pre = REGEX_REGISTRY_PREFIX;
const suf = REGEX_REGISTRY_SUFFIX;

// When registry is used config scope, the trailing '/' is required
const reg = addSuffix(registry, '/');

// 1st attempt, try to get option for the given registry URL
// 2nd attempt, remove the 'https?:' prefix of the registry URL
// 3nd attempt, remove the 'registry/?' suffix of the registry URL
return this.getScopedOption(reg, option)
|| reg.match(pre) && this.getRegistryOption(reg.replace(pre, ''), option)
|| reg.match(suf) && this.getRegistryOption(reg.replace(suf, ''), option);
}

getRegistryOrGlobalOption(registry: string, option: string): mixed {
return this.getRegistryOption(registry, option) || this.getOption(option);
}
}
1 change: 1 addition & 0 deletions src/resolvers/registries/npm-resolver.js
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,7 @@ export default class NpmResolver extends RegistryResolver {
reference: this.cleanRegistry(dist.tarball),
hash: dist.shasum,
registry: 'npm',
packageName: info.name,
};
}

Expand Down
1 change: 1 addition & 0 deletions src/types.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ export type PackageRemote = {
reference: string,
resolved?: ?string,
hash: ?string,
packageName?: string,
};

// `dependencies` field in package info
Expand Down