Skip to content

bug: handle git URLs that are not valid URLs #175

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

Closed
johnstcn opened this issue May 3, 2024 · 2 comments · Fixed by #188
Closed

bug: handle git URLs that are not valid URLs #175

johnstcn opened this issue May 3, 2024 · 2 comments · Fixed by #188
Assignees

Comments

@johnstcn
Copy link
Member

johnstcn commented May 3, 2024

We cannot assume that anything folks pass to GIT_URL will be a valid URL.

Example:

error: parse "[email protected]:johnstcn/test-private-devcontainer.git": first path segment in URL cannot contain colon
@johnstcn johnstcn added the bug label May 3, 2024
@johnstcn johnstcn added this to the envbuilder v1.0 milestone May 3, 2024
@Forestsoft-de
Copy link

the issue comes from CloneRepo method. url.Parse is not a good solution for git url. Would this package a solution? https://pkg.go.dev/github.com/whilp/git-urls#section-documentation

I am on a fix right now because i need the ssh clone.

@mtojek mtojek assigned johnstcn and BrunoQuaresma and unassigned johnstcn May 9, 2024
@BrunoQuaresma
Copy link
Contributor

More context after talking to @johnstcn about this issue:

There are two places we parse git URLs right now apart from test code:

  • DefaultWorkspaceFolder() (envbuilder.go#L858) -- in this case we are only interested in the "name" of the repo, which we can probably get without parsing the URL completely.
  • CloneRepo() (git.go#L48) -- in this case there's a couple of things we do with the parsed URL (check for dev.azure.com, checking URL fragment for reference name), both of which are probably possible without calling url.Parse()

We could also use the library suggested, but my preference is to avoid extra imports if possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants