-
Notifications
You must be signed in to change notification settings - Fork 42
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
parse ssh invalid port ":sjatsh" after host #106
Comments
@sjatsh if you want to use these changes before they are merged into this repo, feel free to use my fork: https://github.com/awoodbeck/caddy-git I've merged these changes into it. Once these changes are merged, I'll resync my fork, which means you'll need to go back to using this repo. |
As you can see, url.Parse will return err if no port number is given. |
the problem is git don't accept port in the ssh url |
Why is something that crucial not being tested prior to a release?! Can't you just implement a custom parser that checks for this URL style? I'm not familiar with golang but is there a parser that can do this? |
@ricardoboss I think lacking a dependency management system like npm or composer of Golang is to blame. The author of this plugin uses Go 1.12.6 in testing, and the plugin works well in this environment. However, the updated official URL parser comes with later versions of Go, and the tests failed when I changed to Go v1.13. Official Caddy builds seem to use up-to-date Go packages, causing this plugin to break. :( |
This commit changed We often use SSH URLs with relative paths (e.g., [email protected]:abiosoft/caddy-git.git where "abiosoft/caddy-git.git" is the relative path indicated by the ":") as opposed to full paths (e.g., [email protected]/abiosoft/caddy-git.git indicated by the initial "/"), the first element of the relative path is treated as a named port (e.g. ":abiosoft" in my first example). This fails the URL parsing in Go since My proposed solution is to insert a pseudo port of |
@doowzs thanks for the explanation! This helps me understand what's going on... Thank you guys and @abiosoft for creating and maintining a nice project like this! Keep the good work up 👍 |
Does anybody have a workaround for current caddyserver git repository? It's pretty uncomfortable to have an entire server down because it uses a lot this git plugin 😞 |
@blankoworld Yes. Use https://github.com/awoodbeck/caddy-git temporarily until either this PR gets merged or Abiola comes up with another solution. My fork of this repo includes the changes I'm proposing here. I've been using them for a few weeks now without issue. |
@awoodbeck Could you please explain how to use the plugin? Should I compile something? Where should I place the result? How to create a caddy binary for that? It's a little bit cryptic for me :-/ |
@blankoworld I'd be happy to. The following is applicable to Caddy version 1. It wouldn't surprise me if these steps change with version 2. I haven't been following its development.
Granted, the above steps make some assumptions about your server, but they should get you started. The plugins are compiled into the caddy binary. If you want to add additional plugins at a later time, you need to recompile the caddy binary after adding the plugin's import path to the |
Thank you @awoodbeck ! What about all other "normal" plugins? How can I activate them? |
You add their import path to the example For example, I'm including the caddy-git plugin by adding |
Changing the URLs from |
@jonasrauber That's a workaround for now, but not a real solution. I'm with @ricardoboss: Such things need to be tested before release and this needs to be fixed code-wise. |
Problem is still exist :( |
The text was updated successfully, but these errors were encountered: