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

[Code] Remove regex git url validation and add more unit tests for repository utils #33919

Merged
merged 2 commits into from
Mar 27, 2019
Merged
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
2 changes: 1 addition & 1 deletion x-pack/plugins/code/common/git_url_utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ test('Git url validation', () => {

// An url without protocol
expect(isValidGitUrl('/Users/elastic/elasticsearch')).toBeFalsy();
expect(isValidGitUrl('github.com/elastic/elasticsearch')).toBeFalsy();
expect(isValidGitUrl('github.com/elastic/elasticsearch')).toBeTruthy();

// An valid git url but without whitelisted host
expect(isValidGitUrl('https://github.com/elastic/elasticsearch.git', ['gitlab.com'])).toBeFalsy();
Expand Down
33 changes: 15 additions & 18 deletions x-pack/plugins/code/common/git_url_utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,26 +11,23 @@ export function isValidGitUrl(
hostWhitelist?: string[],
protocolWhitelist?: string[]
): boolean {
const regex = /(?:git|ssh|https?|git@[-\w.]+):(\/\/)?(.*?)(\.git)?(\/?|\#[-\d\w._]+?)$/;
const isPatternValid = regex.test(url);
try {
const repo = GitUrlParse(url);

if (!isPatternValid) {
return false;
}

const repo = GitUrlParse(url);
let isHostValid = true;
if (hostWhitelist && hostWhitelist.length > 0) {
const hostSet = new Set(hostWhitelist);
isHostValid = hostSet.has(repo.source);
}

let isHostValid = true;
if (hostWhitelist && hostWhitelist.length > 0) {
const hostSet = new Set(hostWhitelist);
isHostValid = hostSet.has(repo.source);
}
let isProtocolValid = true;
if (protocolWhitelist && protocolWhitelist.length > 0) {
const protocolSet = new Set(protocolWhitelist);
isProtocolValid = protocolSet.has(repo.protocol);
}

let isProtocolValid = true;
if (protocolWhitelist && protocolWhitelist.length > 0) {
const protocolSet = new Set(protocolWhitelist);
isProtocolValid = protocolSet.has(repo.protocol);
return isHostValid && isProtocolValid;
} catch (error) {
return false;
}

return isHostValid && isProtocolValid;
}
70 changes: 70 additions & 0 deletions x-pack/plugins/code/common/repository_utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { FileTreeItemType } from '../model';
import { RepositoryUtils } from './repository_utils';

test('Repository url parsing', () => {
Expand Down Expand Up @@ -107,3 +108,72 @@ test('Normalize repository index name', () => {
const indexName4 = RepositoryUtils.normalizeRepoUriToIndexName('github.com/elastic/kibana-code');
expect(indexName3 === indexName4).toBeFalsy();
});

test('Parse repository uri', () => {
expect(RepositoryUtils.orgNameFromUri('github.com/elastic/kibana')).toEqual('elastic');
expect(RepositoryUtils.repoNameFromUri('github.com/elastic/kibana')).toEqual('kibana');
expect(RepositoryUtils.repoFullNameFromUri('github.com/elastic/kibana')).toEqual(
'elastic/kibana'
);

// For invalid repository uri
expect(() => {
RepositoryUtils.orgNameFromUri('foo/bar');
}).toThrowError('Invalid repository uri.');
expect(() => {
RepositoryUtils.repoNameFromUri('foo/bar');
}).toThrowError('Invalid repository uri.');
expect(() => {
RepositoryUtils.repoFullNameFromUri('foo/bar');
}).toThrowError('Invalid repository uri.');
});

test('Repository local path', () => {
expect(RepositoryUtils.repositoryLocalPath('/tmp', 'github.com/elastic/kibana')).toEqual(
'/tmp/github.com/elastic/kibana'
);
expect(RepositoryUtils.repositoryLocalPath('tmp', 'github.com/elastic/kibana')).toEqual(
'tmp/github.com/elastic/kibana'
);
});

test('Parse location to url', () => {
expect(
RepositoryUtils.locationToUrl({
uri: 'git://github.com/elastic/eui/blob/master/generator-eui/app/component.js',
range: {
start: {
line: 4,
character: 17,
},
end: {
line: 27,
character: 1,
},
},
})
).toEqual('/github.com/elastic/eui/blob/master/generator-eui/app/component.js!L5:17');
});

test('Get all files from a repository file tree', () => {
expect(
RepositoryUtils.getAllFiles({
name: 'foo',
type: FileTreeItemType.Directory,
path: '/foo',
children: [
{
name: 'bar',
type: FileTreeItemType.File,
path: '/foo/bar',
},
{
name: 'boo',
type: FileTreeItemType.File,
path: '/foo/boo',
},
],
childrenCount: 2,
})
).toEqual(['/foo/bar', '/foo/boo']);
});
12 changes: 6 additions & 6 deletions x-pack/plugins/code/common/repository_utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,29 +36,29 @@ export class RepositoryUtils {
const segs = repoUri.split('/');
if (segs && segs.length === 3) {
return segs[1];
} else {
return 'invalid';
}

throw new Error('Invalid repository uri.');
}

// From uri 'origin/org/name' to 'name'
public static repoNameFromUri(repoUri: RepositoryUri): string {
const segs = repoUri.split('/');
if (segs && segs.length === 3) {
return segs[2];
} else {
return 'invalid';
}

throw new Error('Invalid repository uri.');
}

// From uri 'origin/org/name' to 'org/name'
public static repoFullNameFromUri(repoUri: RepositoryUri): string {
const segs = repoUri.split('/');
if (segs && segs.length === 3) {
return segs[1] + '/' + segs[2];
} else {
return 'invalid';
}

throw new Error('Invalid repository uri.');
}

// Return the local data path of a given repository.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ const location: Location = createLocation({
pathname: '/github.com/google/guava/tree/master/guava/src/com/google',
});

const match: match<MainRouteParams> = createMatch({
const m: match<MainRouteParams> = createMatch({
path: '/:resource/:org/:repo/:pathType(blob|tree)/:revision/:path*:goto(!.*)?',
url: '/github.com/google/guava/tree/master/guava/src/com/google',
isExact: true,
Expand All @@ -40,7 +40,7 @@ test('render correctly', () => {
node={props.node}
openedPaths={props.openedPaths}
history={history}
match={match}
match={m}
location={location}
closeTreePath={mockFunction}
openTreePath={mockFunction}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`render file item 1`] = `
<Component
<SuggestionComponent
ariaId="suggestion-1-1"
innerRef={[Function]}
onClick={[Function]}
Expand Down Expand Up @@ -51,11 +51,11 @@ exports[`render file item 1`] = `
</div>
</div>
</div>
</Component>
</SuggestionComponent>
`;

exports[`render repository item 1`] = `
<Component
<SuggestionComponent
ariaId="suggestion-1-1"
innerRef={[Function]}
onClick={[Function]}
Expand Down Expand Up @@ -102,11 +102,11 @@ exports[`render repository item 1`] = `
</div>
</div>
</div>
</Component>
</SuggestionComponent>
`;

exports[`render symbol item 1`] = `
<Component
<SuggestionComponent
ariaId="suggestion-1-1"
innerRef={[Function]}
onClick={[Function]}
Expand Down Expand Up @@ -197,5 +197,5 @@ exports[`render symbol item 1`] = `
</div>
</div>
</div>
</Component>
</SuggestionComponent>
`;
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ exports[`render full suggestions component 1`] = `
</div>
</div>
</EuiFlexGroup>
<Component
<SuggestionComponent
ariaId="suggestion-0-0"
innerRef={[Function]}
key="tokenClass - 0-0 - java.lang.String"
Expand Down Expand Up @@ -304,7 +304,7 @@ exports[`render full suggestions component 1`] = `
</div>
</div>
</div>
</Component>
</SuggestionComponent>
</div>
<div
className="kbnTypeahead__items codeSearch-suggestion__group"
Expand Down Expand Up @@ -387,7 +387,7 @@ exports[`render full suggestions component 1`] = `
</div>
</div>
</EuiFlexGroup>
<Component
<SuggestionComponent
ariaId="suggestion-1-0"
innerRef={[Function]}
key=" - 1-0 - src/foo/bar.java"
Expand Down Expand Up @@ -432,7 +432,7 @@ exports[`render full suggestions component 1`] = `
</div>
</div>
</div>
</Component>
</SuggestionComponent>
</div>
<div
className="kbnTypeahead__items codeSearch-suggestion__group"
Expand Down Expand Up @@ -517,7 +517,7 @@ exports[`render full suggestions component 1`] = `
</div>
</div>
</EuiFlexGroup>
<Component
<SuggestionComponent
ariaId="suggestion-2-0"
innerRef={[Function]}
key=" - 2-0 - elastic/kibana"
Expand Down Expand Up @@ -560,8 +560,8 @@ exports[`render full suggestions component 1`] = `
</div>
</div>
</div>
</Component>
<Component
</SuggestionComponent>
<SuggestionComponent
ariaId="suggestion-2-1"
innerRef={[Function]}
key=" - 2-1 - elastic/elasticsearch"
Expand Down Expand Up @@ -604,7 +604,7 @@ exports[`render full suggestions component 1`] = `
</div>
</div>
</div>
</Component>
</SuggestionComponent>
<div
className="codeSearch-suggestion__link"
>
Expand Down
2 changes: 1 addition & 1 deletion x-pack/test/functional/apps/code/code_intelligence.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* you may not use this file except in compliance with the Elastic License.
*/

import expect from 'expect.js';
import expect from '@kbn/expect';
import { TestInvoker } from './lib/types';

// tslint:disable:no-default-export
Expand Down
2 changes: 1 addition & 1 deletion x-pack/test/functional/apps/code/explore_repository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* you may not use this file except in compliance with the Elastic License.
*/

import expect from 'expect.js';
import expect from '@kbn/expect';
import { TestInvoker } from './lib/types';

// tslint:disable:no-default-export
Expand Down
2 changes: 1 addition & 1 deletion x-pack/test/functional/apps/code/manage_repositories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* you may not use this file except in compliance with the Elastic License.
*/

import expect from 'expect.js';
import expect from '@kbn/expect';
import { TestInvoker } from './lib/types';

// tslint:disable:no-default-export
Expand Down
2 changes: 1 addition & 1 deletion x-pack/test/functional/apps/code/search.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* you may not use this file except in compliance with the Elastic License.
*/

import expect from 'expect.js';
import expect from '@kbn/expect';
import { TestInvoker } from './lib/types';

// tslint:disable:no-default-export
Expand Down
2 changes: 1 addition & 1 deletion x-pack/test/functional/apps/code/with_security.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* you may not use this file except in compliance with the Elastic License.
*/

import expect from 'expect.js';
import expect from '@kbn/expect';
import { TestInvoker } from './lib/types';

// tslint:disable:no-default-export
Expand Down