mtconnect: adding optional user pages / html files - #4406
Conversation
| if not self.user_pages: | ||
| return None | ||
| path = os.path.realpath(os.path.join(self.user_pages, name)) | ||
| if path.startswith(self.user_pages) and os.path.isfile(path): |
There was a problem hiding this comment.
What happens here when USER_PAGES=/cfg/pages and the request is /html/../pages_evil/twin.html? realpath gives /cfg/pages_evil/twin.html, and that string still starts with /cfg/pages. Also, self.user_pages keeps any ../symlink form from the ini-dir join while path is canonicalized, so the two sides are not in the same form. Suggest realpath-ing the base once in __init__ and using os.path.commonpath((base, path)) == base (or path.startswith(base + os.sep)).
| return None | ||
| path = os.path.realpath(os.path.join(self.user_pages, name)) | ||
| if path.startswith(self.user_pages) and os.path.isfile(path): | ||
| with open(path, "r") as fh: |
There was a problem hiding this comment.
Reading in text mode means any binary asset (png, ico, woff2, ...) raises UnicodeDecodeError, which the handler turns into a 500. "r" also uses the locale encoding rather than utf-8. Could this be "rb" like extension_schema/model_file?
| path = os.path.realpath(os.path.join(self.user_pages, name)) | ||
| if path.startswith(self.user_pages) and os.path.isfile(path): | ||
| with open(path, "r") as fh: | ||
| return fh.read(), "text/html" |
There was a problem hiding this comment.
Every file goes out as text/html. Browsers refuse to execute <script src> served with that MIME type, and stylesheets too in standards mode, so a digital-twin page with its own .js/.css will not load. mimetypes.guess_type(path) with a fallback would cover this; _send already accepts bytes.
| self._lock = threading.Lock() | ||
| self.user_pages = self.ini.find("MTCONNECT", "USER_PAGES", "") | ||
| if self.user_pages and self.user_pages[0] != "/": | ||
| # use relativ path |
There was a problem hiding this comment.
nit: typo ("relativ"), and the comment restates the code. Either drop it or say why (anchors the pages dir to the ini location).
| with open(path, "rb") as fh: | ||
| return fh.read(), "application/xml" | ||
|
|
||
| def user_page(self, name): |
There was a problem hiding this comment.
Would you consider a small test next to the existing mtc tests: serve a page, 404 on a missing one, and 404 on a ../ escape attempt? That third case is what guards the containment check long-term.
| else: | ||
| data, content_type = result | ||
| self._send(data, content_type=content_type) | ||
| elif route.startswith(f"/html/"): |
There was a problem hiding this comment.
nit: f-string with no placeholders, drop the f.
| *TRANSPORT*:: Comma-separated list of *http*, *mqtt*, *shdr* (default *http*). | ||
| *HTTP_PORT*:: Embedded HTTP agent port (default 5000). | ||
| *HTTP_BIND*:: Interface to bind (default 127.0.0.1; use 0.0.0.0 for the LAN). | ||
| *USER_PAGES*:: Path to optional user pages / html files. |
There was a problem hiding this comment.
Where do these files appear on the wire? The route prefix /html/<name> only exists in the code right now; one sentence here would make it discoverable.
2 participants
so you can add your digital twin and other pages to your mtconnect agent.