BASIC command SSELECT on directory file with item %t segfaults

[mbullr]

The BASIC command SSELECT will segfault when processing a directory file with a item id of "%chr", where chr is any character not found in df_substitute_chars "ACEGLPSVXYZBQ" as defined in qm.h, or the letters "T" and "D".

This is actually a multi part issue.

op_dio4.c dir_select () - Perform select on a directory file
This routine converts mapped names for upper case T, D and for the characters found in df_substitute_chars.
If the character following the "%" character does not meet the above, a null valued btree entry will be created for the file name.

op_sort.c / op_sortdata() SSELECT operation
When processing the btree items created from the list created by dir_select, the code performs a strlen function call on the item. On the null items this causes a segfault.

op_dio3.c map_t1_id() - Perform name mapping for directory files
Maps "~" to "%t"
Maps "." to "%d"
This is inconsistent with the mapping performed in dir_select

The following demonstrates the issue:

1. Add tilde file id to BP via editor

:ED BP ~
BP ~
New record
----: I
0001= this is a tilde test
0002=
Bottom at line 1
----: fi
'~' filed in BP
:sort only BP
0 record(s) listed
:

2. Look at the file from linux

:!vdir BP
total 4
-rw-r--r-- 1 mabull mabull 21 Sep 12 12:18 %t
:

3. Add the following program to BP:

PROGRAM TILDE_TEST

• TEST FIX FOR SSELECT / OP_SORT.C
• ASSUMES DIRECTORY FILE BP HAS ITEM NAMED "%t"
  OPEN "BP" TO BP.FV ELSE ABORT "CANNOT OPEN BP"

SSELECT BP.FV TO 2
*
DONE = @false
LOOP
WHILE NOT(DONE)
READNEXT ID FROM 2 THEN
CRT "ID: ":ID
END ELSE
DONE = @true
END
REPEAT
CRT "ALL DONE"
END

sort only BP Page 1
BP..........
tilde_test

1 record(s) listed
:

From linux:
:!vdir BP
total 8
-rw-r--r-- 1 mabull mabull 21 Sep 12 12:18 %t
-rw-r--r-- 1 mabull mabull 311 Sep 12 12:22 tilde_test
:

4. Run the test program
   :RUN BP tilde_test
   Fault type 11. PC = 000000DF (CF 4A) in SSELCT
   Errno : 00000002
   4 16E92CF8: 00 00 00000000 00000000
   3 16E92CE0: 02 00 00000000 00000000
   2 16E92CC8: 02 00 00000000 00000000
   1 16E92CB0: 02 00 00000001 00000000
   0 16E92C98: 02 04 00000002 00000000
   -1 16E92C80: 01 00 16E7EA88 00000000
   Segmentation fault (core dumped)

To prevent the segfault, modify op_sort.c as follows:

op_sort.c
ln 584 change to:
if (bte->key[0] != NULL)
ts_copy(bte->key[0], strlen(bte->key[0]));

[geneb]

Your recent merge fixed the crash. Have you thought about a method to handle the "%t" in the directory file?
I'm wondering if the tilde should be added to the list of "forbidden" characters...

[mbullr]

Not sure about adding tilde to forbidden characters. Its actually a VOC entry as a synonym for SAID or SPOKEN. As for use in record ids, this is what it says in the QM 4.07 doc (its not exactly the same as what is said in the 2.6.6 version):

Section 3.2 Directory Files.
.
.

Record Ids
Where a record id contains characters that are not valid in operating system file names, QM
automatically replaces them with an alternative representation. This is totally invisible from inside
QM but other software that accesses directory file records must allow for these translations. Rather
than have a different set of translations for each platform, QM adopts a single set based on the
most restrictive platform (Windows) so that data may be moved between environments without
modification of record names. The translations performed are:

 * %A % %P
 \ %B " %Q
  , %C / %S
 = %E + %V
  > %G : %X
  | %J ; %Y
  < %L ? %Z

 In addition, use of a period or tilde character as the first character in the name will translate this
 character to %D or %T respectively."

I think the following changes need to be made:

op_dio3.c map_t1_id() - Perform name mapping for directory files:

Map "~" to "%T" (currently maps to "t")
Map "." to "%D" (currently maps to "d")
Then we match the docs and are consistent with what dir_select in op_dio4.c is doing.

op_dio4.c dir_select:

Pass thru any character sequence that does NOT fall into:
"%chr" where chr is any character found in df_substitute_chars "ACEGLPSVXYZBQ" or the letters "T" and "D"

I think what will then happen with a read or write operation using the id with the passed thru %chr will result in an illegal record id error, which is better than returning an empty id with no real clue as to what happened (at least I think so...).

Remember its not just %t that would cause the segfault, its any %chr string (two character) where the character after the % does not match the above.

I think this is simply the following change in op_dio4.c dir_select starting at line 1199 from:

   while ((c = *(p++)) != '\0') {
        if (c == '%') {
          r = strchr(df_substitute_chars, *(p++));
          if (r != NULL) {
            *(q++) = df_restricted_chars[r - df_substitute_chars];
          }
        } else {
          *(q++) = c;
    }

to

   while ((c = *(p++)) != '\0') {
        if (c == '%') {
          r = strchr(df_substitute_chars, *p);    
          if (r != NULL) {
            *(q++) = df_restricted_chars[r - df_substitute_chars];   // replace %char with translate character
          } else {
             *(q++) = c;           // pass thru %
             *(q++) =*p;         // and the following character
          }
          p++;    // point to next char
        } else {
          *(q++) = c;
    }

My pointer math is sometime suspect, and I am not entirely sure this is the correct path to take. I need to try this out and see what happens.
Stay tuned.......

[mbullr]

Had some time to work on this.

Mods at https://github.com/mbullr/ScarletDME dev branch.

Not really sure I like the way it works, looking for comments (I am tempted to leave the code as is).

Notes:

mapping controlled by map_dir_ids, declared and set to true in kernel.h

can be changed via basic kernel() function key - K$MAP.DIR.IDS

the mapping function is not turned on or off in GPL.BP source

in the newer versions of qm the open statement supports the NO.MAP option (file specific)

Questions:
Why map % to %P? does not make any sense!
Still can end up with '~' ending up twice in a select list

Tested with BP containing the following:

-rw-r--r-- 1 mab mab 0 Sep 16 13:05 ~
-rw-r--r-- 1 mab mab 0 Sep 13 12:48 %F
-rw-r--r-- 1 mab mab 0 Sep 16 13:20 file%Atxt
-rw-r--r-- 1 mab mab 21 Sep 13 10:31 %L
-rw-r--r-- 1 mab mab 0 Sep 16 13:06 %t
-rw-r--r-- 1 mab mab 0 Sep 16 13:06 %T
-rw-r--r-- 1 mab mab 1261 Sep 23 15:04 tilde_test
-rw-r--r-- 1 mab mab 0 Sep 13 12:40 %Z

With mods installed:

    :SORT ONLY BP
    Error 3000 reading %t
    :SELECT BP
    8 record(s) selected to list 0
    ::LIST BP
    LIST BP                                                                  Page 1
    BP..........
    ?
    tilde_test
    ~
    ~
    file*txt
    Error 3000 reading %t

compile and run program bp tilde_test:

    :RUN BP tilde_test
    with mapping turned on
    -------------------------------
    listing of ids from sselect
    -------------------------------
    8 items selected
    id: %F
    id: %t
    id: <
    id: ?
    id: file*txt
    id: tilde_test
    id: ~
    id: ~
    Attempt read of id: %F
      error reading id: %F
      3000 Illegal record id
    Attempt read of id: %t
      error reading id: %t
      3000 Illegal record id
    Attempt read of id: <
    < found
    Attempt read of id: ?
    ? found
    Attempt read of id: file*txt
    file*txt found
    Attempt read of id: tilde_test
    tilde_test found
    Attempt read of id: ~
    ~ found
    Attempt read of id: ~
    ~ found
    -------------------------------
    -------------------------------
    with mapping turned off
    -------------------------------
    listing of ids from sselect
    -------------------------------
    8 items selected
    id: %F
    id: %L
    id: %T
    id: %Z
    id: %t
    id: file%Atxt
    id: tilde_test
    id: ~
    Attempt read of id: %F
    %F found
    Attempt read of id: %L
    %L found
    Attempt read of id: %T
    %T found
    Attempt read of id: %Z
    %Z found
    Attempt read of id: %t
    %t found
    Attempt read of id: file%Atxt
    file%Atxt found
    Attempt read of id: tilde_test
    tilde_test found
    Attempt read of id: ~
    ~ found
    all done
    :

[geneb]

Hey @WolWise1, any suggestions on this one? Thanks!

g.

[WolWise1]

@geneb @mbullr

I don't know as I agree with using Windows as the most restrictive and close down all things as per Windows. But this seems sensible.

Why does Scarlet/OpenQM convert % to %%? Presumably because of this exact problem! If I try and write a file "%", isn't that going to tickle this exact "bug"? So it needs to be converted to/from %%, otherwise people are going to trip over a crash and not have a clue why.

tilde - mixed thoughts. Given that Scarlet uses ~ (as in ~0, ~1, ~2) as bucket files in the OS representation of a hashed file, we probably shouldn't be writing those into a directory file.

The problem is, this is a very tricky problem. We cannot have a directory-file entry with a two-letter name that starts with % ...

And given that Scarlet/OpenQM uses ~ and % internally ...

So you'll need to think about this, but I'm okay with converting a lone dot to %D. Likewise ~ to %T. Then we need to treat any file that starts with a % as special. If it's of length 2, then either it's escaped, or it's a bucket file that's got lost :-)

Otherwise, EVERY leading % needs to be escaped - which means the OS-level file will have an even number of leading percent signs - you double them on write, and halve them on read - "%1" <-> "%%1", "%%1" <-> "%%%%1", etc. A right pain, but if we don't, we're screwed as soon as the user tries to explicitly write a file like "%1", "%T", or whatever. If they write a file "A%T", that's not a problem, it's not a leading %.

(I guess that's why PI/Open didn't have that problem, it used as its escape character for bucket files, it had a different problem. The catalog was a directory file, so it contained loads of files starting with "*". That blew up our backup program pretty spectacularly !!! :-)

[WolWise1]

Sorry - PI/Open used "space backspace" as its bucket escape character - I used angle-brackets in the above post, and the renderer has clearly eaten them ...

[geneb]

The commercial version of OpenQM has transitioned to using "%0" and "%1" for "normal" files, so I suspect we really should be doing that - if not converting existing "~" files to "%", then at least create new files with the "%" prefix.

One thing to note, when duplicating @mbullr's test in OpenQM 3.4, it functions correctly in that creating a "" item in a directory file shows as "" when listed or sorted, and shows as "%T" in the directory where the item lives in Linux. The QMBasic program shows "~" as the item name when run. I would strongly recommend that we follow this behavior. (Creating a "%" item results in a "%P" directory entry)

I think that if you can tweak things to show the "~" item in list/sort/BASIC in ScarletDME, that should probably be enough. I apologize for not digging into this further.

[mbullr]

I never thought of validating with a recent QM version, I will modify the test program to hit a few more cases and run on both. Also I have been looking at this from the OS side, which is probably the wrong way to look at. As for conversion from ~ to % for hash files, I was the one who made the mods for SD. If you want to proceed with the change I can work on it (may take a month or so, I generally work on this stuff when the weather turns crappy). We did not support t backwards compatibility on SD, so there will be an issue with current users. I think there were some programs posted on the forum to automate the file renaming from % to ~ for those wishing to try Scarlet, may need to dig those up.

[geneb]

I think the original issue was some utility treating anything prefixed with "~" as a temp file and was deleting it.

I'm not sure how the original OpenQM update worked. I think they changed the qmsys file directory tree items to be prefixed with "%" on install, but I don't know what was done if it was an upgrade. I don't know for sure if it would grok "~" prefixes on previously existing hash files. I think we could hash out (ha!) the exact process in a separate git issue.

Thanks for doing the additional testing!

g.

[mbullr]

Should I add an issue (enhancement request) "convert hash file prefix from ~ to %"? It would be a place to start documenting what needs to be done for the change.

[WolWise1]

@mbullr Sounds a good idea. Just my thoughts you might want to copy across ...

1. Open File needs to check (and remember) whether it's a type 1, ~hashed, or %hashed. This'll have the C code that actually opens the OS-level files. In other words, first move is the code that opens files, to make it detect and handle either type.

2. Create File (and create index, and all the other stuff that creates files) to create stuff as %hashed files. In other words, once the code is there we move to creating % files by default.

3. Resize File converts files - I believe it copies from an old file into a new one, so all it needs to do is make sure all the new files are %

[mbullr]

Mods to make Scarlet map directory files similar to QM can now be found at https://github.com/mbullr/ScarletDME
Test file tilde_test found in qmsys/BP.
To compile on qm define IS_QM needs to be uncommented.
Anyone what to test prior to submitting pull request?