[KSambaShareData] Accept spaces in ACL host name
ClosedPublic
Actions

Authored by broulik on Aug 29 2018, 11:23 AM.

Details

Reviewers

dfaure
fvogt

Commits

R241:469b6cec127d: [KSambaShareData] Accept spaces in ACL host name

Summary

Otherwise it would reject Unix User\foo

Test Plan

(or whatever the part before the backslash is called)

Thanks Fabian for help with regular expressions :)

I can now enable and disable guest mode again in kdenetwork-filesharing Share dialog and can actually access my shares now (somewhat)

Diff Detail

Repository

R241 KIO

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

broulik created this revision.Aug 29 2018, 11:23 AM

Restricted Application added a project: Frameworks. · View Herald TranscriptAug 29 2018, 11:23 AM

Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript

broulik requested review of this revision.Aug 29 2018, 11:23 AM

Any chance you can create a unit test?

ngraham added a subscriber: ngraham.Aug 29 2018, 7:34 PM

Any chance you can create a unit test?

Will try. Should it just check a couple of strings for validity? And also check that the ACL it read are valid since they must be as they came from net usershare?

In D15141#317370, @broulik wrote:

Any chance you can create a unit test?

Will try. Should it just check a couple of strings for validity? And also check that the ACL it read are valid since they must be as they came from net usershare?

For the beginning, a couple of strings which previously failed, and some which are ok before and after, are a good start.

Some input which should be rejected would cover the majority of this functions scope.

Add unit test

Sidenote: can you explain "(somewhat)" in the Test Plan?

dfaure requested changes to this revision.Aug 30 2018, 8:10 PM

dfaure added inline comments.

autotests/ksambasharetest.cpp
27	This includes all of QtTest and all of QtCore. Please use <QTest> instead.
autotests/ksambasharetest.h
22	typo: s/AH/HA/

This revision now requires changes to proceed.Aug 30 2018, 8:10 PM

Does this patch help with https://bugs.kde.org/show_bug.cgi?id=398079? It just came in today.

In D15141#317900, @ngraham wrote:

Does this patch help with https://bugs.kde.org/show_bug.cgi?id=398079?

I don't know but without this patch I failed to configure permissions in the "Share" tab, e.g. "Allow guest" or "Everyone: full access", so maybe it fixes that.

Fix include guard
Fix include

dfaure accepted this revision.Aug 31 2018, 8:14 AM

This revision is now accepted and ready to land.Aug 31 2018, 8:14 AM

Closed by commit R241:469b6cec127d: [KSambaShareData] Accept spaces in ACL host name (authored by broulik). · Explain WhyAug 31 2018, 8:15 AM

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

		Path
M		autotests/CMakeLists.txt (1 line)
A	M	autotests/ksambasharetest.h (38 lines)
A	M	autotests/ksambasharetest.cpp (72 lines)
M		src/core/ksambashare.cpp (2 lines)

Diff	ID	Base	Description	Created	Lint	Unit
Base			Base
Diff 1	40633			Aug 29 2018, 11:22 AM	★	★
Diff 2	40705			Aug 30 2018, 1:17 PM	★	★
Diff 3	40730			Aug 31 2018, 7:15 AM	★	★
Diff 4	40736	d8a0cbc	R241:469b6cec127df35880710f27128a480f55a3b1be	Aug 31 2018, 8:15 AM	★	★