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

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

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
Labels
bug Something isn't working

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 "git@github.com:johnstcn/test-private-devcontainer.git": first path segment in URL cannot contain colon
@johnstcn johnstcn added the bug Something isn't working 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
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants