Commit f40191a1 authored by Harald Sitter's avatar Harald Sitter 🌈
Browse files

smb: add hack to support spaces in workgroup names

Summary:
workgroup names are as best I can tell always still netbios names which
means they can contain a bunch of characters ordinarily not found in valid
host names. e.g. spaces
this causes trouble with the IANA SMB URI draft, as used by libsmbc,
since the workgroup would be the host field of the RI when browsing
a workgroup (i.e. filtering hosts that are member of a given workgroup)
because QUrl does not allow invalid hostnames in the host field.

to bypass this problem we now put the workgroup name into the query of the
url as `kio-workgroup`, should it cause trouble in the host field. SMBUrl
takes this query into account when constructing the url for smbc.
since the latter has uniquely exciting potential for breakage this entire
dance is only done when absolutely necessary and otherwise we continue with
all the same code and behavior as without this commit.

on a side note: the awkward name flexibility seems to not extend to
computer names anymore (supposedly because of LLMNR) so this entire
use case is already very niche as we (and libsmbclient) currently only
support workgroup browsing for NT1 networks, and NT1 is by default not
supported on windows10 or samba.

FIXED-IN: 20.04
BUG: 204423

Test Plan: builds, test passes, can browse workgroup with space in name

Reviewers: ngraham

Subscribers: kde-frameworks-devel, kfm-devel, thiago

Tags: #dolphin, #frameworks

Differential Revision: https://phabricator.kde.org/D27804
parent daec307b
......@@ -89,6 +89,41 @@ private Q_SLOTS:
QCOMPARE(SMBUrl(QUrl("smb://[fe80::9cd7:32c7:faeb:f23d]/share")).toSmbcUrl(),
"smb://fe80--9cd7-32c7-faeb-f23d.ipv6-literal.net/share");
}
void testWorkgroupWithSpaces()
{
// Workgroups can have spaces but QUrls cannot, so we have a hack
// that puts the workgroup info into a query.
// Only applicable to SMB1 pretty much, we do not do workgroup browsing
// for 2+.
// https://bugs.kde.org/show_bug.cgi?id=204423
// wg
QCOMPARE(SMBUrl(QUrl("smb://?kio-workgroup=hax max")).toSmbcUrl(),
"smb://hax max/");
// wg and query
QCOMPARE(SMBUrl(QUrl("smb://?kio-workgroup=hax max&q=a")).toSmbcUrl(),
"smb://hax max/?q=a");
// host and wg and query
QCOMPARE(SMBUrl(QUrl("smb://host/?kio-workgroup=hax max&q=a")).toSmbcUrl(),
"smb://hax max/host?q=a");
// host and wg and query
QCOMPARE(SMBUrl(QUrl("smb://host/share?kio-workgroup=hax max")).toSmbcUrl(),
"smb://hax max/host/share");
// Non-empty path. libsmbc hates unclean paths
QCOMPARE(SMBUrl(QUrl("smb:///////?kio-workgroup=hax max")).toSmbcUrl(),
"smb://hax max/");
// % character - run through .url() to simulate behavior of our listDir()
QCOMPARE(SMBUrl(QUrl(QUrl("smb://?kio-workgroup=HAX%25MAX").url())).toSmbcUrl(),
"smb://HAX%25MAX/");
// !ascii - run through .url() to simulate behavior of our listDir()
QCOMPARE(SMBUrl(QUrl(QUrl("smb:///?kio-workgroup=DOMÄNE A").url())).toSmbcUrl(),
"smb://DOMÄNE A/"); // works as-is with smbc.
// Also make sure type detection knows about this
QCOMPARE(SMBUrl(QUrl("smb:/?kio-workgroup=hax max")).getType(),
SMBURLTYPE_WORKGROUP_OR_SERVER);
}
};
QTEST_GUILESS_MAIN(SMBUrlTest)
......
......@@ -40,8 +40,8 @@
#include <KLocalizedString>
#include <QEventLoop>
#include <QTimer>
#include <QUrlQuery>
#include <grp.h>
#include <pwd.h>
......@@ -519,6 +519,16 @@ void SMBSlave::listDir(const QUrl &kurl)
// QString workgroup = m_current_url.host().toUpper();
QUrl u("smb://");
u.setHost(dirpName);
if (!u.isValid()) {
// In the event that the workgroup contains bad characters, put it in a query instead.
// This is transparently handled by SMBUrl when we get this as input again.
// Also see documentation there.
// https://bugs.kde.org/show_bug.cgi?id=204423
u.setHost(QString());
QUrlQuery q;
q.addQueryItem("kio-workgroup", dirpName);
u.setQuery(q);
}
udsentry.fastInsert(KIO::UDSEntry::UDS_URL, u.url());
// Call base class to list entry
......
......@@ -34,6 +34,7 @@
#include <QDir>
#include <QHostAddress>
#include <QUrlQuery>
#include <KConfig>
#include <KIO/Global>
......@@ -112,10 +113,62 @@ void SMBUrl::updateCache()
break;
}
if (sambaUrl.url() == "smb:/")
// NetBios workgroup names may contain characters that QUrl will not
// allow in a host. Yet the SMB URI requires us to have the workgroup
// in the host field when browsing a workgroup.
// As a hacky workaround we'll not set a host but use a query param
// when encountering a workgroup that causes QUrl to error out.
// For libsmbc we then need to translate the query back to SMB URI.
// Since this is super daft string construction it will doubltlessly
// be imperfect and so we do still prefer deferring the string
// construction to QUrl whenever possible.
// https://support.microsoft.com/en-gb/help/909264/naming-conventions-in-active-directory-for-computers-domains-sites-and
// https://bugs.kde.org/show_bug.cgi?id=204423
//
// Should we ever stop supporting workgroup browsing this entire
// hack can be removed.
QUrlQuery query(sambaUrl);
const QString workgroup = query.queryItemValue("kio-workgroup");
if (workgroup.isEmpty()) {
// If we don't have a hack to apply we can simply defer to QUrl
if (sambaUrl.url() == "smb:/") {
m_surl = "smb://";
} else {
m_surl = sambaUrl.toString(QUrl::PrettyDecoded).toUtf8();
}
} else {
// If we have a workgroup hack to apply we need to manually construct
// the stringy URI.
query.removeQueryItem("kio-workgroup");
sambaUrl.setQuery(query);
m_surl = "smb://";
else
m_surl = sambaUrl.toString(QUrl::PrettyDecoded).toUtf8();
if (!sambaUrl.userInfo().isEmpty()) {
m_surl += sambaUrl.userInfo() + "@";
}
m_surl += workgroup;
// Workgroups can have ports per the IANA definition of smb.
if (sambaUrl.port() != -1) {
m_surl += ':' + QString::number(sambaUrl.port());
}
// Make sure to only use clear paths. libsmbc is allergic to excess slashes.
QString path('/');
if (!sambaUrl.host().isEmpty()) {
path += sambaUrl.host();
}
if (!sambaUrl.path().isEmpty()) {
path += sambaUrl.path();
}
m_surl += QDir::cleanPath(path);
if (!sambaUrl.query().isEmpty()) {
m_surl += '?' + sambaUrl.query();
}
if (!sambaUrl.fragment().isEmpty()) {
m_surl += '#' + sambaUrl.fragment();
}
}
m_type = SMBURLTYPE_UNKNOWN;
// update m_type
......@@ -133,7 +186,7 @@ SMBUrlType SMBUrl::getType() const
}
if (path().isEmpty() || path(QUrl::FullyDecoded) == "/") {
if (host().isEmpty())
if (host().isEmpty() && !query().contains("kio-workgroup"))
m_type = SMBURLTYPE_ENTIRE_NETWORK;
else
m_type = SMBURLTYPE_WORKGROUP_OR_SERVER;
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment