scan tests in make test (was Re: Retiring or fixing smbtorture?)

[Richard Sharpe]

On Wed, Oct 24, 2012 at 10:03 AM, Richard Sharpe
<realrichardsharpe at gmail.com> wrote:
> On Wed, Oct 24, 2012 at 7:02 AM, Richard Sharpe
> <realrichardsharpe at gmail.com> wrote:
>> On Wed, Oct 24, 2012 at 5:05 AM, Volker Lendecke
>> <Volker.Lendecke at sernet.de> wrote:
>>> On Wed, Oct 24, 2012 at 03:57:39AM -0700, Richard Sharpe wrote:
>>>> On Tue, Oct 23, 2012 at 6:17 PM, Andrew Bartlett <abartlet at samba.org> wrote:
>>>> > On Tue, 2012-10-23 at 15:43 -0700, Richard Sharpe wrote:
>>>> >> Hi Folks,
>>>> >>
>>>> >> Shouldn't we either retire smbtorture or fix it?
>>>> >>
>>>> >> For example, the TRANS2SCAN test fails when a call is made to
>>>> >> cli_open/cli_openx (depending on version) to open a directory ("\\")
>>>> >> but the underlying code sets the file attributes to not a directory.
>>>> >>
>>>> >> This is just bit rot. Should it be fixed? It could probably be simply
>>>> >> fixed by adding a file_attributes parameter to the calls made all the
>>>> >> way down.
>>>> >
>>>> > So, some of the history here is that we don't run scan tests in make
>>>> > test, because they really need to be run against windows.  We then don't
>>>> > run them against windows because we now have an official way of
>>>> > discovering new protocol elements, rather than scanning and guessing.
>>>> >
>>>> > Part of the reason we don't run them in make test is that they tend to
>>>> > be fairly slow, but because they don't validate their output they are
>>>> > not particularly useful, except in filling logs with unexpected command
>>>> > messages.
>>>> >
>>>> > The blocking of these is done by the selftest/skip file, and comments on
>>>> > some other scan entries include:
>>>> >
>>>> > ^samba4.rpc.autoidl  # this one just generates a lot of noise, and is no
>>>> > longer useful
>>>> > ^samba4.rpc.countcalls # this is not useful now we have full IDL
>>>> > ^samba4.rap.scan # same thing here - we have docs now
>>>> > ^samba4..*trans2.scan # uses huge number of file descriptors
>>>> > ^samba4.*.base.scan.ioctl # bad idea in make test
>>>> > ^samba4.*.base.scan.pipe_number # bad idea in make test
>>>>
>>>> OK, so, I think I understand what you are telling me :-)
>>>>
>>>> However, QA tends to find it difficult to understand these things, and
>>>> lots of people seem to still use smbtorture from source3 and then run
>>>> into trouble with all the tests that fail for all sorts of reasons.
>>>
>>> This is a well-known problem. There is a complete business
>>> to be made around smbtorture-like Software. Look at the list
>>> of companies at SDC offering SMB validation. I doubt that
>>> any of those can be run without some level of manual
>>> interpretation. Sure, smbtorture is really an acquired
>>> taste, but I bet the competition is very good at selling you
>>> the interpretation of the scan results.
>>
>> Sure, but my original point about the TRANS2SCAN test is that:
>>
>> 1. Either the code is wrong, or
>>
>> 2. Samba is wrong in responding to a request to open the directory \
>> as a file with an error
>>
>> 3. Or Windows is wrong in some way.
>>
>> I will check what Windows does when that test is run against it, and
>> submit a patch so that at least the test is correct, rather than
>> broken as I currently suspect it is.
>
> OK, I have run the test against Windows Server 2003 and it fails in
> exactly the same way. It is trying to open the root directory ('\\')
> as a file, and it is being rejected with the error
> STATUS_FILE_IS_A_DIRECTORY.
>
> The TRANS2SCAN and NTTRANSSCAN tests are simply broken as a result of
> a change to the underlying cli_open code, it looks like. Probably a
> separate cli_opendir function should be created, but I am not sure
> that is useful.
>
> It might simply be better to delete those two tests.

Even the equivalent test in the Samba4 version of smbtorture seems broken.

Here is the open request in torture/basic/scanner.c:torture_nttrans_scan:

        dnum = smbcli_open(cli->tree, "\\", O_RDONLY, DENY_NONE);

And here is the block of code in smbcli_open that initializes the SMB:

        open_parms.openx.level = RAW_OPEN_OPENX;
        open_parms.openx.in.flags = 0;
        open_parms.openx.in.open_mode = accessmode;
        open_parms.openx.in.search_attrs = FILE_ATTRIBUTE_SYSTEM |
FILE_ATTRIBUTE_HIDDEN;
        open_parms.openx.in.file_attrs = 0;
--------------------------------------------^^^^
        open_parms.openx.in.write_time = 0;
        open_parms.openx.in.open_func = openfn;
        open_parms.openx.in.size = 0;
        open_parms.openx.in.timeout = 0;
        open_parms.openx.in.fname = fname;

As you can see, we insist that it is a file, not a directory.

-- 
Regards,
Richard Sharpe
(何以解憂？唯有杜康。--曹操)