Use HTTPS if no scheme has been provided in wizard #9621
Conversation
fmoc
force-pushed
the
work/wizard-auto-use-https
branch
from
d8c2d27
to
9ca6c1f
Compare
|
Kudos, SonarCloud Quality Gate passed! |
| if (QUrl(userProvidedUrl).isRelative() && !userProvidedUrl.isEmpty()) { | ||
| userProvidedUrl.prepend(QStringLiteral("https://")); | ||
| if (!userProvidedUrl.startsWith(QStringLiteral("http://")) && !userProvidedUrl.startsWith(QStringLiteral("https://"))) { | ||
| static const QString defaultScheme(QStringLiteral("https://")); |
There was a problem hiding this comment.
Move to anonymous namespace.
Remove the qDebug/promise to implement a logging category in an upcoming pr.
What was wrong with
client/src/gui/wizard/owncloudsetuppage.cpp
Line 123 in ca7f788
now you will convert foo://bar to https://foo://bar.
Does Qt detect foo:8080 wrong?
There was a problem hiding this comment.
Apparently, yes. isRelative returns false for me on localhost:9200 at the moment. I first tried to use other QUrl properties, e.g., schema, but schema is set to localhost in this example, whereas host is an empty string then. That makes sense from a technical perspective, since URLs like mailto:a@b.c are legal, too. But from a UX perspective, it makes no sense.
I think we should maintain a list with legal protocols anyway.
We could combine the current check with some QUrl-based ones, e.g.,:
auto url = QUrl::fromUserInput(userProvidedUrl);
if (url.scheme() == "http" || url.scheme() == "https") {...}
if (url.host().isEmpty() && url.scheme() != "http" && url.scheme() != "https") {...}
fmoc
force-pushed
the
work/wizard-auto-use-https
branch
from
9ca6c1f
to
ddcf9af
Compare
| @@ -33,6 +33,8 @@ QString initLocalFolder() | |||
| return OCC::FolderMan::instance()->findGoodPathForNewSyncFolder(localFolder); | |||
| } | |||
|
|
|||
| const QString defaultUrlScheme = QStringLiteral("https://"); | |||
There was a problem hiding this comment.
I'd move constants to the top of the anonymous namespace.
Historically the constants at least for strings where followed the schema fooC.
The C might have meant CString or constant, I don't care but we should come up with a standard for constants.
I'd suggest to either keep the C or to start Uppercase?
fmoc
force-pushed
the
work/wizard-auto-use-https
branch
from
ddcf9af
to
4687aab
Compare
No description provided.