Skip to content
Snippets Groups Projects
  1. Sep 18, 2024
  2. Sep 17, 2024
    • Rob Swindell's avatar
      25f8f69d
    • Rob Swindell's avatar
      NUL-terminate the SMB fileidxrec_t.name field, just in case · 45363fee
      Rob Swindell authored
      The terminating NUL is actually part of the index record, but with file
      corruption, it's technically possible the NUL could be missing.
      Fixes CID 509552
      
      Use strnicmp() insted of stricmp() in smb_removefile()
      Fixes CID 509551
      45363fee
    • Rob Swindell's avatar
      NUL-terminate the SMB fileidxrec_t.name field, just in case · daa1b788
      Rob Swindell authored
      The terminating NUL is actually part of the index record, but with file
      corruption, it's technically possible the NUL could be missing.
      
      Fixes CID 509554
      daa1b788
    • Rob Swindell's avatar
      Use the liberal file matching to continue file listing/searches · fee120db
      Rob Swindell authored
      If the user didn't supply a wildcard, only a single file (the first file
      matching the liberal file pattern) would ever be listed in the file
      listing/searches, even when multiple files in the directory matched the
      liberal file matching pattern. This is not a new bug.
      fee120db
    • Rob Swindell's avatar
      Only use liberal file pattern matching in the terminal server listfile funcs · b7aaac27
      Rob Swindell authored
      Commit 3a3c889b (2 years ago now) changed loadfiles() to use liberal file
      matching (e.g. "syncterm.exe" matched both "syncterm.exe" and
      "syncterm_v1.2b.exe").
      
      This could produce surprising results when doing file list querieis/operations
      with the FileBase methods via JS (e.g. jsexec utils) and (now that I look at
      it), the FTP server too.
      
      So we should not have been doing liberal file matching *everywhere* loadfiles
      is used, just where it was a usability issue (due to displayed filenames being
      truncated to 12 chars for <=80 column terminals).
      
      Now solved by add/use of new liberal_filepattern() function only in the
      built-in file listing methods: sbbs_t::listfiles() and sbbs_t::listfileinfo().
      
      Note: Custom JS file searching/listing scripts may now need their own
      work-arounds for this usability issue, if they have it.
      b7aaac27
    • Rob Swindell's avatar
      Update JSDOC description of update() method · 2391c3c9
      Rob Swindell authored
      Clarify that this is the method to use to rename a file.
      2391c3c9
  3. Sep 15, 2024
  4. Sep 14, 2024
    • Rob Swindell's avatar
      Don't heap allocate argument to MsgBase and FileBase constructors · a398abb2
      Rob Swindell authored
      Nelgin reported a weird error with a failed very large allocation for the
      base/code argument to the FileBase constructor. There's no good reason
      these strings were heap-allocated in the first place, so just change to
      use a stack allocated variable instead. I don't know why this would fix
      anything, but at least there's one less heap allocation and potential
      for memory leak here.
      
      Fix 2 bugs in js_update_file():
      1. missing parenthesis (really?!? Caught by Coverity - sigh) in last
      commmit caused attempt/failure/error to remove file after making any
      updates. The removing is only supposed to happen when its necessary to
      remove and re-add the file to the filebase (e.g. updating extended
      description or auxdata).
      2. Wrong filename used in 'removing' exception string.
      a398abb2
    • Rob Swindell's avatar
      Add/use smb_removefile_by_name() · 09d721d4
      Rob Swindell authored
      This fixes a filebase corruption issue that could be triggered by using
      FileBase.update() to rename a file while also changing the file's auxdata or
      extended description: you can't remove a file header that has the new filename
      (in the file_t.name field), cause it doesn't exist yet. So we needed an SMBLIB
      API function to pass the (original) filename instead. Much like the filedat.c
      removefile() function, but without the SMB opening/closing feature.
      09d721d4
    • Rob Swindell's avatar
      Don't allow FileBase.update() to create a duplicate file · 397d6ebe
      Rob Swindell authored
      FileBase.update() will now throw an exception if attempting to rename a file
      to a filename that already exists in the filebase index.
      
      Prior to this change, one could (via JS methods) rename a file in the base to
      create a duplicate filename which would corrupt the base (one index entry and
      two header records for the same filename).
      397d6ebe
    • Rob Swindell's avatar
      MSVC requires a 0 in the struct initializer · ffa2c51b
      Rob Swindell authored
      ffa2c51b
    • Rob Swindell's avatar
      FileBase.update() method throws more exceptions upon error · 2fc23c32
      Rob Swindell authored
      This will help to determine cause of any file update (e.g. rename)
      failures, as reported by Nelgin.
      2fc23c32
    • Rob Swindell's avatar
      Perform filename checks (index versus header fields) on filebases · 5f0fb1c7
      Rob Swindell authored
      I was able to corrupt a filebase using fileman.js, renaming a file
      (the filename in the index as viewed with 'smbutil x' did not match
      the filename listed with 'smbutil l') - yet, chksmb reported no errors
      with the filebase.
      
      Now chksmb will detect that type of corruption.
      5f0fb1c7
    • Rob Swindell's avatar
      smb_getmsgidx() now reads entire index records from file bases too · 1ab4ef2b
      Rob Swindell authored
      chksmb needed this to perform filename checks (index against headers).
      1ab4ef2b
    • Rob Swindell's avatar
      fileidexrec_t doesn't need to contain union · 62b0507b
      Rob Swindell authored
      This served exactly no purpose (since the idxrec_t existed at the same
      offset in both parts of the union). So this was just some weird artifact
      from the new filebase development.
      62b0507b
    • Rob Swindell's avatar
      Eliminte weird gcc (12.2) warning in release build (only) · 10562789
      Rob Swindell authored
      Increasing size of mode[] element by 2 bytes eliminated these GCC warnings
      that seem like false-positives to me:
      
      sbbs.h:194:48: warning: writing 1 byte into a region of size 0 [-Wstringop-overflow=]
        194 |                                 (ret)[JSSTSpos]=(char)JSSTSstrval[JSSTSpos]; \
            |                                 ~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~
      js_file.c:225:25: note: in expansion of macro ‘JSSTRING_TO_STRBUF’
        225 |                         JSSTRING_TO_STRBUF(cx, str, p->mode, sizeof(p->mode), NULL);
            |                         ^~~~~~~~~~~~~~~~~~
      js_file.c:42:17: note: at offset 5 into destination object ‘mode’ of size 5
         42 |         char    mode[5];
            |                 ^~~~
      
      Similar use of JSSTRING_TO_STRBUF in other files (js_console.cpp, js_archive.c)
      (with larger target buffers) does not trigger the same warnings.
      10562789
  5. Sep 13, 2024
    • Rob Swindell's avatar
      Add keyboard shortcuts to Clients list view: Ctrl-A, Ctrl-F and Del · c31203fc
      Rob Swindell authored
      Ctrl-A - Select All
      Ctrl-F - Filter IP address
      DEL - Close Socket
      
      Also, use Begin/EndUpdate() calls to batch updates to the ListView in Timer
      tick (no visible difference, but it's the right thing to do).
      
      I don't know why C++Builder increased the TextHeight value of this form from
      13 to 15, but doesn't seem to really matter (or at least, I couldn't tell).
      c31203fc
    • Rob Swindell's avatar
      Change the height of the horizontal splitter to the minimum (1 pixel) · 513715f0
      Rob Swindell authored
      I don't know why this was set so wide (10 pixels), but that did help to easily
      see that it was being placed in the wrong spot in the form (between the tool
      button bar and the top panel). But we don't need it to be that thick. 1 pixel
      is still enough to be able to grab it with the mouse and resize the top/bottom
      panels.
      513715f0
    • Rob Swindell's avatar
      Display horizontal splitter in the correct spot. · 89637f0d
      Rob Swindell authored
      For who knows how long now, the horizontal splitter that allows the sysop
      to control the size of the top 2 panels (server forms), was displayed above
      the top panel, not between the top and bottom panel, thus making the panel
      heights not controllable. Weird. Change the order of making the controls
      visible to insure that the splitter is displayed below the top panel and
      on top of the bottom panel.
      
      Also (non-functional changes):
      Remove redunant mutex from client_on() - makes no difference.
      Use try/catch in client_on() to try to catch VCL exceptions - doesn't catch
      the "invalid index" error popups that we see on occasion. <shrug>
      89637f0d
    • Rob Swindell's avatar
      Fix crash due to use after free or double-free · aff554a4
      Rob Swindell authored
      ... bug introduced in commit 29a35642.
      
      strListMerge() doesn't realloc the strings in the list, so we don't want to
      free the strings in this list here.
      
      This is likely the cause of the crash Keyop eluded to in #synchronet.
      aff554a4
  6. Sep 12, 2024
    • Rob Swindell's avatar
      Split-up the opening and writing of msg pointer files, to help debug issue · 20cfdb18
      Rob Swindell authored
      Reported by Keyop (upon being auto-disconnected overnight):
      !ERROR 9 (Bad file descriptor) in data_ovl.cpp line 47 (putmsgptrs) writing
      "message pointers" access=0
      
      Unfortunately, the way putmsgptrs() was written, we couldn't tell if this was
      an open (data/user/*.subs file) issue or a writing issue.
      
      So create putmsgptrs_fp() and use that from sbbs_t::putmsgptrs(), so we can
      log a different error (with more accurate details) if it's a file-open failure
      versues a file-write failure.
      20cfdb18
    • Rob Swindell's avatar
      Blocked IP addresses in ip-silent.can weren't filtered from QWK and REP pkts · 29a35642
      Rob Swindell authored
      The "from_ip" header field of QWK messages is checked against blocked IP
      addresses, but was only checking against IP addresses from ip.can, not any
      addresses listed in ip-silent.can. These 2 list files are now merged together
      for the purposes of filtering during QWK/REP packet import.
      29a35642
  7. Sep 10, 2024
    • Rob Swindell's avatar
      Indicate vote/poll messages and files (with details) when listing msgs/files · d84b8e3d
      Rob Swindell authored
      Add '-v' (increase verbosity) option, used to display msg dates and timezones
      ... when using the the 'l' (list messages) command (to view post date/time).
      Use '-vv' or '-v -v' to see timezones of messages.
      
      The -v option is now also applicable to the 'v' (view) messages command (now
      redundant with the 'V' command).
      
      Features as requested by Nelgin as part of issue #786.
      
      Removed day-of-week from date/times displayed. We don't need that level of
      user-friendliness with this tool.
      However, we are also displaying 12h/am/pm times. Some sysops probably would
      prefer 24hour time, so that should be considered at some point.
      d84b8e3d
    • Rob Swindell's avatar
      Don't check header fields of deleted messages for control characters · 0fd380f8
      Rob Swindell authored
      ... related to issue #786.
      
      Also, don't check for a message-ID if the message-type does not match the
      expected message type ("type mismatch").
      0fd380f8
    • Rob Swindell's avatar
      Refactor the function: unpack_bundle() · 0f0a8a1a
      Rob Swindell authored
      1. When a 0-length bundle file was encountered or an unpacking error occurred,
         any remaining bundles for the current search day-of-week (e.g. *.SU*) would
         be skipped/ignored. This bug (issue #764, regarding the 0-length file part),
         is fixed by not incrementing the day-of-week index in the main loop, but
         rather only incremeting the index when all bundles for the current
         day-of-week have been processed.
      2. The age calculation for 0-byte/length bundle files was incorrect, so all
         0-length  bundle files would always be considered "less than 24-hours old"
         (and thus, never auto-deleted).
         This exacerbated the problem of issue #764 since it would persist until the
         0-length files were manually deleted. Fixed the file age calculation and
         now logging the date/timestamp of the 0-length file as well.
      3. Don't do the switch/case/sprintf dance when we're not re-running a glob()
         search.
      4. Replace the switch/case statement with an array of week day names/patterns.
      5. Ignore (with a warning log message) any sub-directories of the inbound
         directory that happen to match the bundle file search pattern.
      6. Use better variable naming.
      7. Refer to files with a length of 0 as "0-length" instead of "0-byte" in log
         messages.
      0f0a8a1a
  8. Sep 07, 2024
    • Rob Swindell's avatar
      Track broker-connected status via Mosquitto connect/disconnect callbacks · cefca3c6
      Rob Swindell authored
      I think this might fix issue #781. I suspect that SBBS (the MQTT client) is
      being disconnected by the server ("due to protocol error") *after* the
      call to mosquitto_connect_bind() is successful. We don't have any correponding
      log output for this case, but at least we can track the connection status
      accurately using the Mosquitto client callbacks and not try to publish when
      we're not connected.
      cefca3c6
    • Rob Swindell's avatar
      Don't attempt to publish MQTT messages unless/until connected to broker · 9b6138f2
      Rob Swindell authored
      For cases where an mqtt struct is shared between threads without concurrency
      control. I'm making this improvement in light of research into issue #781,
      though I don't expect this change to fix the reported issue.
      
      The reported error seems to come from the event thread (publishing node
      status upon starting to run the "DAILY" event) when a broker connection was
      not successful, however the reporter (Nelgin) may not have had debug-level
      logging turned on, so didn't capture the successful broker-connect log
      message. I think the broker connection *was* successful and perhaps then
      terminated by the broker ("due to protocol error"?).
      9b6138f2
    • Rob Swindell's avatar
      Log more/better messages when message fails to post (e.g. duplicate detected) · 7c11e274
      Rob Swindell authored
      It appears I had the idea to make the CantPost text.dat string a format string
      before, but never quite finished that change (e.g. made the change to
      email.cpp and text.dat too).
      7c11e274
    • Rob Swindell's avatar
      Add llprintf() to log a formatted line to the node.log and term server log · ba9f4a44
      Rob Swindell authored
      There's a whole lotta calls to sprintf() then logline() that could be reduced
      with this function.
      ba9f4a44
    • Rob Swindell's avatar
      Don't use QWK reply message date unless time zone is also specified · 984c371a
      Rob Swindell authored
      For regular user QWK Relpy packet uploads only, if no timezone is specified
      (e.g. via @TZ kludge or HEADERS.DAT), then over-ride the message's "posted"
      date/time with the current date/time since we're going to set the message's
      timezone to the BBS's local timezone as well.
      
      This is a fix for issue #783 reported by Chris Jacobs.
      
      If/when we support user-specified timezones, then this likely would be a
      place where we'd want to use the user's timezone.
      984c371a
  9. Sep 06, 2024
  10. Aug 27, 2024
    • Rob Swindell's avatar
      Fix NULL pointer deref in random_menu() · 46c603ce
      Rob Swindell authored
      When no files with extensions are found, though they matched the glob()
      pattern, a NULL pointer deref would result (segfault).
      
      This could happen if the only files matching the pattern had no extenions or
      were directories (not files).
      
      Fix for issue #779
      46c603ce
  11. Aug 23, 2024
    • Rob Swindell's avatar
      Don't call utime() on the node.dab file for every read · e93b6dfa
      Rob Swindell authored
      ... this was the cause of some observed unnecessarily high disk/file server
      (Samba) utilization, as we call getnodedat() a lot. utime() opens and closes
      the file, which was already open - and we're not modifying the file, so
      updating the 'modification time' here is wrong anyway.
      
      Disabling this 21-year old bit of logic resulted in a pretty dramatic
      reduction in Samba (smbd) CPU utilization on Vertrauen.
      
      If a BBS actually needes this hack (e.g. for NFS compatibility, as Deuce
      eluded in the comment), they'd be better off just setting the "Keep Node File
      Open" node setting (in SCFG->Nodes) to "No".
      e93b6dfa
    • Rob Swindell's avatar
      Extend (and back-off) the user.tab record lock attempts · 69fc70ab
      Rob Swindell authored
      I'v been getting errors locking user.tab (for read) for a while (over samba),
      so hopefully this helps. The lockuserdat() total timeout duration extends from
      about 5 seconds to about 45 seconds (with an incremental back-off).
      
      Implement the same lock-retry logic/limit in putuserdat().
      69fc70ab
  12. Aug 19, 2024
  13. Aug 16, 2024
  14. Aug 15, 2024
  15. Aug 13, 2024
Loading