Unexpected behavior in getModifyDateTime / getCreateDateTime (Possible bug)

[cfeied]

The example listfiles.ino exposes what I believe might be considered a bug:

 if (entry.getModifyTime(datetime)) {
   printSpaces(4);
   printTime(datetime);

Since printTime does no error checking, the example assumes that getModifyTime will return false unless datetime is valid. Unfortunately this is not correct: getModifyTime happily returns true for invalid datetimes. This can result in a crash.

To repro, you need an SD card containing a file with an invalid lastModified datetime. In my case I have an ExFat SD card containing files that were written by BillG's SdFat (with no attention paid to file dates) and one of the files returns an invalid modification datetime with month==255. After getModifyTime returns true, the subsequent call to printTime fails when we exceed the months array index bounds. The result is a program crash and truncated output at the point of failure.

Validity checking could be added to the example, but I think many people will find it misleading to have getModifyTime return true and an invalid datetime.

The File::getModifyTime() call passes through FsFile.h for triage to FatFile::getModifyDateTime() or ExFatFile::getModifyDateTime()

  bool getModifyDateTime(uint16_t* pdate, uint16_t* ptime) {
    return m_fFile ? m_fFile->getModifyDateTime(pdate, ptime) :
           m_xFile ? m_xFile->getModifyDateTime(pdate, ptime) : false;
  }

In my case the actual work is done in ExFatFile, which returns true so long as the file exists in the directory cache and will happily return invalid data for the date and time.

bool ExFatFile::getModifyDateTime(uint16_t* pdate, uint16_t* ptime) {
  DirFile_t* df = reinterpret_cast<DirFile_t*>
                 (m_vol->dirCache(&m_dirPos, FsCache::CACHE_FOR_READ));
  if (!df) {
    DBG_FAIL_MACRO;
    goto fail;
  }
  *pdate = getLe16(df->modifyDate);
  *ptime = getLe16(df->modifyTime);
  return true;

 fail:
  return false;
}

For the printout below, I modified printTime in the example to show the invalid entry rather than crashing

  if ((tm.mon > 11) || (tm.mon < 0))
      Serial.printf("Oops_[%d]", tm.mon);
  else 
    Serial.print(months[tm.mon]);

Yielding:

Initializing SD card...initialization done.
SessionFile00.raw                             1035441    06:14  December 8, 2021
SessionFile01.raw                             1866293    00:00  Oops_[255] 0, 1980
mtpindex.dat                                        0    19:05  December 15, 2021
SessionFile02.raw                             6946558    06:18  December 8, 2021
SessionFile03.raw                             1183576    18:08  December 15, 2021
SessionFile04.raw                             1380207    18:12  December 15, 2021
SETTINGS.WSS                                    40782    00:45  December 16, 2021
datalog.bin                                        61    00:00  January 1, 2021
done!

You will notice a possibly related second problem: datalog.bin was created 5 minutes ago by another SD example (SdFat_Usage.ino), and it is showing an incorrect file modification time and date. I could understand Jan 1, 1980, but Jan 1, 2021 is just... not... yeah.