Refactor submodule URL parsing #7100
Conversation
Use combination of url.Parse and regex to parse refURL rather than by hand with indexes & attempt to check if refURL is from same instance and adjust output to match. Also now return empty string instead of our original guess at URL if we are unable to parse it. Fixes go-gitea#1526
Codecov Report
@@ Coverage Diff @@
## master #7100 +/- ##
==========================================
+ Coverage 41.54% 41.57% +0.02%
==========================================
Files 446 446
Lines 60825 60848 +23
==========================================
+ Hits 25271 25297 +26
+ Misses 32271 32268 -3
Partials 3283 3283
Continue to review full report at Codecov.
|
GiteaBot
added
the
lgtm/need 2
GiteaBot
added
lgtm/need 1
| if urlPrefixHostname == refHostname { | ||
| return prefixURL.Scheme + "://" + urlPrefixHostname + path | ||
| } | ||
| return "http://" + refHostname + path |
There was a problem hiding this comment.
Shouldn't it default to https better?
There was a problem hiding this comment.
It wouldn't work if the server didn't support https (we don't know anything about it in this case). most servers should issue a redirect from http to https if they do support it.
There was a problem hiding this comment.
@lafriks what do you think? I agree with @mrsdizzie here - most https sites will simply redirect http to https.
There was a problem hiding this comment.
true but from security point of view it would be better to prefer https and as most of users should be running https this would result in unneeded http request to server.. So it is trade-off for security vs usability :)
There was a problem hiding this comment.
I think we might need to add some repository configuration to allow for people to set their own conversions for submodules, but it's not particularly high priority.
GiteaBot
added
lgtm/done
Use combination of url.Parse and regex to parse refURL rather than by hand with indexes & attempt to check if refURL is from same instance and adjust output to match. Also now return empty string instead of our original guess at URL if we are unable to parse it. Fixes go-gitea#1526
Use combination of url.Parse and regex to parse refURL rather than by hand with indexes & attempt to check if refURL is from same instance and adjust output to match.
Also now return empty string instead of our original guess at URL if we are unable to parse it.
Fixes #1526
The logic behind comparing the refURL hostname with the prefixURL hostname is that if they are the same, we should use whatever the entire prefixURL host is when creating the link -- which allows us to take both the scheme and optional port number and return a more correct link.
For submodules from different hosts, I think it is still safe to create links (most github/gitlab/etc... will have corresponding web links).