greiman / SdFat

Arduino FAT16/FAT32 exFAT Library
MIT License
1.09k stars 511 forks source link

.name() breaks WebServer on esp32 #338

Open MartinNohr opened 3 years ago

MartinNohr commented 3 years ago

This code used to work: FsFile download = SD.open("/" + filename); if (download) { server.sendHeader("Content-Type", "text/text"); server.sendHeader("Content-Disposition", "attachment; filename=" + filename); server.sendHeader("Connection", "close"); //server.streamFile(download, "application/octet-stream"); server.streamFile(download, "image/bmp"); download.close(); } server is a WebServer object. From WebServer.h: template size_t streamFile(T &file, const String& contentType) { _streamFileCore(file.size(), file.name(), contentType); return _currentClient.write(file); }

The reason it fails is that file.name() doesn't return the filename. The new function is .GetName(), but I don't want to change the WebServer source. What is a good way to fix this?

greiman commented 3 years ago

Have the author of WebServer fix the program to use long file names. file.name() returns a pointer to a 12 character 8.3 short file name. I suspect major changes would be needed to support long filenames with potential utf-8 characters.

The file.name() call was never part of SdFat. It was a call added to the SD.h wrapper for a 2009 version of SdFat. Arduino's SD.h still only supports short filenames and is based on the 2009 SdFat.

I added the dummy call that returned a pointer to "use getName()" in version 2.0.5 but that was a mistake since programs would compile but not work.

I made use of file.name() a hard error in 2.1.0 with the error message error("use getName(name, size)")

greiman commented 3 years ago

Probably best to just use the esp32 SD library. The esp32 library used the FatFs library.

The ESP8266 library uses a version of SdFat:

https://github.com/esp8266/Arduino/tree/master/libraries/SD

The ESP8266 wrapper appears to implement a static buffer for long names or allocates a buffer with new/malloc. I can't do that in general since it would take way too much memory for smaller boards and I am committed not to use dynamic memory.

Edit: esp32 allocates whatever memory is required to save the file path using strdup in open.

There are now almost 200 Arduino like boards and I can't support all the functions added by wrappers of SdFat.

greiman commented 3 years ago

I looked at how name() is used in _streamFileCore. It is only used to see if the file is gziped. So when I made name() a hard error WebServer failed to compile. It was never working correctly for files encoded gzip when used with SdFat.

template <typename ServerType>
void ESP8266WebServerTemplate<ServerType>::_streamFileCore(const size_t fileSize, const String &fileName, const String &contentType)
{
  using namespace mime;
  setContentLength(fileSize);
  if (fileName.endsWith(String(FPSTR(mimeTable[gz].endsWith))) &&
      contentType != String(FPSTR(mimeTable[gz].mimeType)) &&
      contentType != String(FPSTR(mimeTable[none].mimeType))) {
    sendHeader(F("Content-Encoding"), F("gzip"));
  }
  send(200, contentType, emptyString);
}
greiman commented 3 years ago

See this for a possible solution for file.name()

https://github.com/greiman/SdFat/issues/337

MartinNohr commented 3 years ago

Thanks, the last solution there should work for me. I didn't want to go back to the standard esp32 sd.h. I found during testing about a year ago that your code was significantly faster. I also needed support for cards larger than 32Gb. I greatly appreciate your help on this and for making this library available!