Mbed-cli: mbed add breaks certain URLs

[bmcdonnell-ionx]

Bug report. Using Mbed CLI 1.3.0; Mbed OS 5.7.3; on Windows 10.

I have some .lib files in my project which point to internal git repos which are of the form

http://servername:portnum/path/to/repo

mbed add clones the repo, but then it puts the path in the .lib file as

https://servername/portnum/path/to/repo

HTTPS instead of HTTP, and / instead of : delimiting servername from portnum. Then mbed update doesn't work because the repo path is wrong.

[theotherjimmy]

Thanks for the detailed bug report. I don't have time to look into this right now.

@screamerbg Is the http -> https conversion by design? I don't think that the port-number handling was intentional. Would you be open to patches that resolve this issue?

[screamerbg]

@bmcdonnell-ionx Thanks for reporting this and apologies for the late reply. This seems like a bug in the formaturl() function. Could you provide me with an example URL string (but mask the actual host and sensitive path names)?

[bmcdonnell-ionx]

Could you provide me with an example URL string (but mask the actual host and sensitive path names)?

Is the example provided insufficient? http://servername:portnum/path/to/repo What additional info do you need?

I don't know of a publicly-accessible repo that uses http and portnum, but if you do a PR I can test it on ours.

[screamerbg]

@bmcdonnell-ionx Would you be able to try my development branch at https://github.com/screamerbg/neo/tree/feature_safe_repo_urls ?

[bmcdonnell-ionx]

Progress...

$ mbed add http://servername:portnum/path/to/mylib
[mbed] ERROR: Cannot import "http://servername:portnum/path/to/mylib" in "C:\dev\myproj\mylib" due to arbitrary service schema/port in the repository URL.
[mbed] ERROR: Repositories are usually hosted on service port 443 (https), 80 (http) and 22 (ssh)
[mbed] ERROR: You can use "--insecure" switch enable the use arbitrary repository URLs.
---

Please do not enforce a port number. We are not using the ports you specified. That shouldn't be an error. IMO, it shouldn't even be a warning. (Even if we were using one of the typical ports, someday someone will use a nonstandard port, and this is an unnecessary restriction.)

Also, looking at the help text...

$ mbed add -h

[snip]

  --insecure            Allow insecure repository URLs. By default mbed CLI
                        imports only "safe" URLs, e.g. based on standard ports
                        - 80, 443 and 22. This option enables the use of
                        arbitrary URLs/ports.

I can understand why you might add the --insecure option for http. (I'm not sure it's better to require it vs. not, but it seems reasonable.)

However, I don't think specifying a port, even a non-standard number, should require the --insecure option. If you disagree, please explain.

...As instructed in the error message above, I used the specified option. It puts http (not s) in the .lib file as it should, but it strips the portnum from the URL.

$ mbed add --insecure http://servername:portnum/path/to/mylib
warning: LF will be replaced by CRLF in mylib.lib.
The file will have its original line endings in your working directory.
[mbed] Adding library "mylib" from "http://servername:portnum/path/to/mylib" at latest revision in the current branch
[mbed] Updating reference "mylib" -> "http://servername/path/to/mylib/#hash"

$ cat mylib.lib
http://servername/path/to/mylib/#hash

[screamerbg]

@bmcdonnell-ionx thanks for trying this out. I've just added a small fix to address the port number issue. Please let me know if that works for your use-case.

Regarding insecure, in majority of the use cases, the end-user doesn't know about every single repository URL that will be accessed/cloned during mbed import/add. And unlike the git clone <url> command, where end-user is de-facto aware what's being cloned (essentially it's a "consensual clone" as they can see the URL before executing git), mbed CLI clones many repositories recursively without prior consent/user awareness, except for the starting repository, or the program URL.

This poses some challenges, including legally, as an end-user could always blame on mbed CLI about causing their git or hg to try to access a funky URL/service port.

For example, combining the recursive nature of mbed CLI with bad intentions, could lead to terrifying results. It's not hard to imagine a malicious program containing 100s .lib files pointing at different ports at b1-rtr0-hsrp.jpl.nasa.gov (as repo URLs), which, once mbed CLI start cracking on it, would look a lot like port scanning. And making multiple connection attempts on a government monitored network, like NASA's, can get you in real trouble.

Furthermore, in many corporate networks, any connection attempt on arbitrary ports (usually below port 1024), is being flagged, logged and reported - even if it was for all the good reasons.

With everything said above "--insecure" provides this user consent, effectively acting as a legally binding agreement that the end-user know what they're doing.

Hope that makes sense. Please let me know if you'd like me to clarify further.

[bmcdonnell-ionx]

@screamerbg, with your latest changes, using the --insecure option, mbed add and mbed import now work for our repos. And your explanation makes perfect sense, thanks.

I notice that the --insecure option is also listed in the help for mbed update and mbed deploy commands. I did not have to use it, though. Is that intentional? [EDIT: see below.]

[bmcdonnell-ionx]

CORRECTION...

I notice that the --insecure option is also listed in the help for mbed update and mbed deploy commands. I did not have to use it, though.

Actually, it just requires the option on the first time you clone the repo, if either of those commands does so. If the library repo with the "insecure" URL is already present, then you don't need the --insecure option.

...Issue resolved, pending merge?

[bmcdonnell-ionx]

@screamerbg, is there anything left to do on this? Are you going to do a PR?

[bmcdonnell-ionx]

@screamerbg, bump.

[bmcdonnell-ionx]

@screamerbg, @theotherjimmy, is there a reason this is still pending?

@screamerbg has had this proposed fix sitting in a branch in his fork for about a month now. It seems fine to me, but IDK if further review is required, who other stakeholders may be, etc. You're not waiting for me to do a PR, are you?

Apparently you're currently in process of releasing v1.5.0, and this didn't make it in. ☹️ Maybe v1.5.1?

[theotherjimmy]

I opened #642.

[bmcdonnell-ionx]

Do you normally close issues when the fix has been merged, or released?

ETA on the release?

[screamerbg]

@bmcdonnell-ionx The release is now live. Mbed CLI 1.6.0. Thanks for your help and patience on this major change to Mbed CLI to support arbitrary URLs.

[screamerbg]

Feel free to close the issue if this is now resolved for you

[bmcdonnell-ionx]

Thanks, @screamerbg!