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] encode/decode branchs and tags #34683

Merged
merged 1 commit into from
Apr 8, 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
7 changes: 7 additions & 0 deletions x-pack/plugins/code/public/components/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,24 @@
import { connect } from 'react-redux';
import { Route as ReactRoute, RouteProps } from 'react-router-dom';
import { Match, routeChange } from '../actions';
import { decodeRevisionString } from '../utils/url';

interface Props extends RouteProps {
routeChange: (match: Match) => void;
}
class CSRoute extends ReactRoute<Props> {
// eslint-disable-next-line @typescript-eslint/camelcase
public UNSAFE_componentWillMount() {
if (this.state.match && this.state.match.params && this.state.match.params.revision) {
this.state.match.params.revision = decodeRevisionString(this.state.match.params.revision);
}
this.props.routeChange({ ...this.state.match, location: this.props.location });
}

public componentDidUpdate() {
if (this.state.match && this.state.match.params && this.state.match.params.revision) {
this.state.match.params.revision = decodeRevisionString(this.state.match.params.revision);
}
this.props.routeChange({ ...this.state.match, location: this.props.location });
}
}
Expand Down
4 changes: 3 additions & 1 deletion x-pack/plugins/code/public/sagas/blame.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@ import { loadBlame, loadBlameFailed, LoadBlamePayload, loadBlameSuccess } from '
import { blamePattern } from './patterns';

function requestBlame(repoUri: string, revision: string, path: string) {
return kfetch({ pathname: `/api/code/repo/${repoUri}/blame/${revision}/${path}` });
return kfetch({
pathname: `/api/code/repo/${repoUri}/blame/${encodeURIComponent(revision)}/${path}`,
});
}

function* handleFetchBlame(action: Action<LoadBlamePayload>) {
Expand Down
6 changes: 3 additions & 3 deletions x-pack/plugins/code/public/sagas/file.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ function requestRepoTree({
query.parents = true;
}
return kfetch({
pathname: `/api/code/repo/${uri}/tree/${revision}/${path}`,
pathname: `/api/code/repo/${uri}/tree/${encodeURIComponent(revision)}/${path}`,
query,
});
}
Expand Down Expand Up @@ -158,7 +158,7 @@ function requestCommits(
) {
const pathStr = path ? `/${path}` : '';
const options: any = {
pathname: `/api/code/repo/${uri}/history/${revision}${pathStr}`,
pathname: `/api/code/repo/${uri}/history/${encodeURIComponent(revision)}${pathStr}`,
};
if (loadMore) {
options.query = { after: 1 };
Expand All @@ -174,7 +174,7 @@ export async function requestFile(
line?: string
): Promise<FetchFileResponse> {
const { uri, revision, path } = payload;
const url = `/api/code/repo/${uri}/blob/${revision}/${path}`;
const url = `/api/code/repo/${uri}/blob/${encodeURIComponent(revision)}/${path}`;
const query: any = {};
if (line) {
query.line = line;
Expand Down
8 changes: 8 additions & 0 deletions x-pack/plugins/code/public/utils/url.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,11 @@ import createHistory from 'history/createHashHistory';
export const history = createHistory();

export const isImportRepositoryURLInvalid = (url: string) => url.trim() === '';

export const decodeRevisionString = (revision: string) => {
return revision.replace(':', '/');
};

export const encodeRevisionString = (revision: string) => {
return revision.replace('/', ':');
};
6 changes: 3 additions & 3 deletions x-pack/plugins/code/server/routes/file.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ export function fileRoute(server: hapi.Server, options: ServerOptions) {
const fileResolver = new GitOperations(options.repoPath);
const { uri, path, ref } = req.params;
try {
const blob = await fileResolver.fileContent(uri, path, ref);
const blob = await fileResolver.fileContent(uri, path, decodeURIComponent(ref));
if (blob.isBinary()) {
const type = fileType(blob.content());
if (type && type.mime && type.mime.startsWith('image/')) {
Expand Down Expand Up @@ -149,7 +149,7 @@ export function fileRoute(server: hapi.Server, options: ServerOptions) {
const after = queries.after !== undefined;
try {
const repository = await gitOperations.openRepo(uri);
const commit = await gitOperations.getCommit(repository, ref);
const commit = await gitOperations.getCommit(repository, decodeURIComponent(ref));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@spacedragon this will raise exception if pass through a branch with /. try this repo: https://github.com/elastic/apm-agent-nodejs/tree/greenkeeper/ws-6.2.1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

image

both the commented code (borrowed from here) and uncommented code in the screenshot above work.

I think the getCommit(xxx) function needs to be more comprehensive to cover all cases (branch/tag/remote, etc) of revisions. https://stackoverflow.com/a/50192006

let's merge this PR first and use a separate issue to track this.

const walk = repository.createRevWalk();
walk.sorting(Revwalk.SORT.TIME);
walk.push(commit.id());
Expand Down Expand Up @@ -223,7 +223,7 @@ export function fileRoute(server: hapi.Server, options: ServerOptions) {
const gitOperations = new GitOperations(options.repoPath);
const { uri, path, revision } = req.params;
try {
const blames = await gitOperations.blame(uri, revision, path);
const blames = await gitOperations.blame(uri, decodeURIComponent(revision), path);
return blames;
} catch (e) {
if (e.isBoom) {
Expand Down