In ReadOnlyTokenFile._load_targets() and TokenFileName.lookup() you keep open files

[he1f]

ReadOnlyTokenFile

    def _load_targets(self):
  ...
      for f in cfg_files:
            for line in [l.strip() for l in open(f).readlines()]:
...

TokenFileName

    def lookup(self, token):
        token = os.path.basename(token)
        path = os.path.join(self.source, token)
        if os.path.exists(path):
            return open(path).read().strip().split(':')
        else:
            return None

It make sence to open file using context manager, to ensure it always get closed. Also I would be happy to prepare commit which migrate this code to modern pathlib, if it is interesting for you.

[CendioZeijlon]

I agree, we should definitely make sure to close opened files.

Do you want to open a PR and fix it?

[he1f]

I'll be happy to open a PR, but can I use a pathlib module or I should use old-fasioned os and os.path modules?

[CendioZeijlon]

Migrating to pathlib sounds nice, go ahead!

[he1f]

I've prepared local commit, but I'm not familiar how to create PR to the websockify. Can someone advise me? :)

[samhed]

I've prepared local commit, but I'm not familiar how to create PR to the websockify. Can someone advise me? :)

Have you cloned novnc/websockify, or have you made your own fork? Easiest is if you create your own fork of websockify and commit to a new branch. Then you can simply open your fork in your browser and GitHub will suggest to open a pull request.

[he1f]

thanks a lot, I've created a fork and after that made PR from from the fork

[kajinamit]

https://github.com/novnc/websockify/pull/602 was merged and I believe we can close this issue.

[CendioOssman]

Thanks for investigating. Closing.