Export highlights: secure export filename - #14325
Conversation
| -- full file path without extension | ||
| clipping_filepath = clipping_dir .. "/" .. | ||
| util.getSafeFilename(self.ui.bookinfo:expandString(clipping_filename, file, timestamp), nil, nil, -1) | ||
| clipping_filepath = clipping_filepath:gsub("[\n\r]", ", "):gsub("\t", " ") |
There was a problem hiding this comment.
I meant to handle it in getSafeFilename so you can pass a string with a newline in it. I could see a similar problem inadvertently coming from OPDS or NewsDownloader or something, or perhaps user input. Of course special behavior like replacing newline with , would then have to be done first.
Incidentally, wouldn't a Windows \r\n become , , as written?
There was a problem hiding this comment.
special behavior like replacing newline with
,
What is the common behaviour? Replace with the white space?
There was a problem hiding this comment.
It's likely the most common but I also think it makes the most sense generically. This particular replacement is tailored to a list, but that wouldn't apply to a title like:
Moby-Dick
or, The Whale
Or any other assortment of lines that don't form a list.
Incidentally, I believe that ; is acceptable, so it's probably better than , for the use case (Author, Firstname; Author2, Firstname2)
There was a problem hiding this comment.
For book info we use the pipeline
Is it good in filenames?
There was a problem hiding this comment.
I think that's one of the forbidden Windows characters. ;-)
Edit: see util.replaceAllInvalidChars()
There was a problem hiding this comment.
Okay, to summarize:
-in export, \n and \r\n are replaced with ;
-in util, \n and \r\n and \t are replaced with space
Right?
There was a problem hiding this comment.
I think that makes the most sense, yes.
| ---- @treturn string safe filename | ||
| function util.getSafeFilename(str, path, limit, limit_ext) | ||
| local filename, suffix = util.splitFileNameSuffix(str) | ||
| filename = filename:gsub("\r?\n", " "):gsub("\t", " ") |
There was a problem hiding this comment.
Would it make more sense conceptually around line 1010 (just before the length limit)? Fine by me either way.
2 participants
This change is