dronecan / gui_tool

MIT License
27 stars 30 forks source link

A file's pathkey may contain OS path separators, which break the file server in some cases #63

Closed grisharevzin closed 1 month ago

grisharevzin commented 1 month ago

The FileServer uses pathkeys to avoid sending long file paths over DroneCAN. The pathkeys are generated this way:

def FileServer_PathKey(path):
    return base64.b64encode(struct.pack("<I",zlib.crc32(bytearray(path,'utf-8'))))[:7].decode('utf-8')

Such keys can randomly contain / and \. If a pathkey happens to contain these characters, depending on the OS, all file-related requests to this file will be broken because it tries to normalize OS separators before checking the pathkey:

class FileServerJson(dronecan.app.file_server.FileServer):
  def _resolve_path(self, relative):
      rel = relative.path.decode().replace(chr(relative.SEPARATOR), os.path.sep)
      if rel in self._key_to_path:
          return self._key_to_path[rel]
      return super(FileServerJson, self)._resolve_path(relative)

Example:

2024-10-06 15:52:51,377 DEBUG dronecan.app.file_server [#120:uavcan.protocol.file.GetInfo] 'fBa2/A='
2024-10-06 15:52:51,377 ERROR dronecan.app.file_server [#{0:03d}:uavcan.protocol.file.GetInfo] error
Traceback (most recent call last):
  File "C:\code\dronecan_gui_tool\venv\Lib\site-packages\dronecan\app\file_server.py", line 77, in _get_info
    with open(self._resolve_path(e.request.path), "rb") as f:
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\code\dronecan_gui_tool\dronecan_gui_tool\widgets\file_server.py", line 106, in _resolve_path
    return super(FileServerJson, self)._resolve_path(relative)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\code\dronecan_gui_tool\venv\Lib\site-packages\dronecan\app\file_server.py", line 68, in _resolve_path
    raise OSError(errno.ENOENT)
OSError: 2

I think that replace(chr(relative.SEPARATOR), os.path.sep) shouldn't happen before the if rel in self._key_to_path: check:

class FileServerJson(dronecan.app.file_server.FileServer):
  def _resolve_path(self, relative):
    rel = relative.path.decode()
    if rel in self._key_to_path:
        return self._key_to_path[rel]
    return super(FileServerJson, self)._resolve_path(relative)

because it happens in FileServer's _resolve_path either way later:

class FileServer(object):
    def _resolve_path(self, relative):
        rel_decoded = relative.path.decode()
        if self.path_map and rel_decoded in self.path_map:
            return self.path_map[rel_decoded]
        rel = rel_decoded.replace(chr(relative.SEPARATOR), os.path.sep)
        out = _try_resolve_relative_path(self.lookup_paths, rel)
        if not out:
            raise OSError(errno.ENOENT)

        self._path_hit_counters[out] += 1
        return out