[FEATURE REQUEST] Rewriting Content-Disposition filename header on direct nginx serving block
camjac251 opened this issue · 15 comments
I hope this'll make sense but one thing I noticed about the script that in both the file manager as well as files being served by nginx, the original filename is not used.
I was thinking this might be possible with nginx to rewrite the Content-Disposition filename to the original filename so that the url https://i.fiery.me/ykJfU.jpg could be accessible through that url but when you would save it in browser, it could write as the original filename grandcanyon-image142.jpg instead of ykJfU.jpg
Kipp has this working nicely
My idea was possibly if nginx could parse a local text file or csv/json/xml file that would be written to by lolisafe of entries like ykJfU.jpg|image142.jpg which it would read the first field, find its match, then set the Content-Disposition filename to that second field so the url can use the original filename on saving.
So far my findings have not been great so far
It might be possible with lua in nginx?
https://stackoverflow.com/questions/57612920/nginx-lua-write-original-filename-to-file
or possibly https://www.gakhov.com/articles/implementing-api-based-fileserver-with-nginx-and-lua.html
Frankly not a fan of implementing this into the service. I'd have went with mixtape.moe's setup of identifier.filename.extension (e.g. abcd.my_image.jpg) if I really wanted to include their file names into the downloadable files while making sure no collisions happen.
Scrambling filename is simply the soul of the service I guess.
Either way, you should already be able to at least build the CSV file by querying the DB periodically or something. Since the original file names are indeed stored into DB.
Not sure about the part of using that through Nginx however.
I am kind of lost at how to do it in nginx. I was hoping for original filenames on save (through Content-Disposition, the filename on server disk would remain the same random characters, just when the client presses ctrl s, it uses the original as the save name) so that I can send someone a before and after video or image and if they aren't viewable in browser, the save window would display the original filename so they can know which is which.
Could this be an optional functionality in lolisafe? since the original filename is already written to db, could an extra file be maintained built into the app that generates/adds to a csv/json/xml file that is usable if someone wishes?
I speculate I'd have to hook into various other things if it needs to be built into the service (file deletion for example).
Even if not, when serving uploads with node, DB wouldn't actually get queried on file access, cause it'd just add the uploads directory into another directory that the web server needed to serve (static basically). But if you need the header support, among other things, DB would then need to be queried on every file access.
Caching, temporary or not, would then add more complexity.
I'd advise just have an external bash script query the DB and do the CSV magic, then run that on crontab or something. It's unnecessarily complex for an optional feature.
Would it be possible to support hooks for batch scripts in the config file? Like an after upload, after deletion optional variable?
I'll hopefully figure out how to write the original filename, actual filename in a csv file with a batch script but running it through a hook instead of cron would be insanely better.
The lack of standard makes me hesitate (e.g. we don't have event hooks of any kind).
Also it's a definite deep rabbit hole.
If I make a hook for one thing, then I will eventually have to make hooks for everything else, or at least some other people would want them.
I'd also then need to support various hook methods.
On the top of my head I'd definitely go with webhooks (e.g. POST JSON-formatted payloads to specific webhook URLs), as that appears to be pretty commonly used on various web services, but then if running locally you'd need to have some sort of lightweight web server to listen and make sense of the requests at all times.
maybe my use of the term hook was incorrect here. I'll keep looking around. Hopefully I can find a method that would be unintrusive to the way things currently operate
I mean, certainly for your case, I was thinking of having the service just run a specific local bash script file or something. One that is specific to handle what to do when a new file gets uploaded, probably passing the filename and original name as parameters. But imho, that'd be a very limited hooking system, and I'd very much prefer webhook if I really had to, basically.
Closed for now.
Looking back on the convo I think I overcomplicated the setup. It looks like half is already there to support it. If it builds on serveFilesWithNode and maybe an additional option, adding a Content-Disposition response header with the filename directly could work. Only thing is it would need to match the filename it's reading on disk to the entry in the database to grab the originalname to set the filename directive. Is this sort of thing out of the scope of the project?
Also I think wonder if there was confusion on my part of explaining the feature at first.
What I was hoping the feature suggestion would bring is just the header being added to the file serving part. It wouldn't change how files are written to disk, they'd still be randomized like before.
https://github.com/dhknudsen/downloadheaders/blob/master/download.js#L14 I saw this when looking up to see how it's done in node.
If it builds on
serveFilesWithNodeand maybe an additional option, adding a Content-Disposition response header with the filename directly could work. Only thing is it would need to match the filename it's reading on disk to the entry in the database to grab the originalname to set the filename directive.
Huh, now that I think about it, that'd be fairly easy in comparison. Of course you won't be able to serve files directly with Nginx, but I suppose that'll be a fair trade for your needs.
Also I think wonder if there was confusion on my part of explaining the feature at first.
Probably, lmao. That suggestion is much less complex in comparison, yet I don't think I even thought of that back then. I was probably too fixated on the whole "serving with Nginx" meme to notice? Not sure at this point.
Is this sort of thing out of the scope of the project?
If it's only that, then imo it isn't. I'll reopen this for the time being. I'll probably work on it real quick in an hour or so.
I am getting this error in the logs after updating to the latest with this feature. It might possibly be since I'm using mysql with knex, I'm not sure.
0|app | 2020-09-27T03:51:26: [2020-09-27 03:51:26] TypeError: Cannot read property 'original' of undefined
0|app | 2020-09-27T03:51:26: at opts.preSetHeaders (/path/to/code/app/lolisafe.js:82:66)
0|app | 2020-09-27T03:51:26: at processTicksAndRejections (internal/process/task_queues.js:97:5)
0|app | 2020-09-27T03:51:26: at async serveStatic (/path/to/code/app/node_modules/serve-static/index.js:99:7)
It seems I misunderstood how serve-static worked. I was trying to apply the headers even before it tried to confirm whether the files existed in uploads directory, meaning it'd first capture all requests, including requests to the page files, css, jss, etc. Turn it off for the time being. I'll give it more look within the next few days, as it doesn't seem as straightforward as I assumed it would be, lmao
It's been working for me. I see in the header content-disposition: inline; filename="cool-picture-of-a-thing.jpg". It is able to rewrite the filename on saving great, but the error shows up in the log when the site is used (opening files, browsing pages)
Yeah that's what I meant. The codes didn't notice that the requests weren't all from uploaded files, but other files too, so it failed in querying DB as obv there'd be no uploads by the name home, dashboard, etc.
Try 5720749.
It took awhile as not only did I already have to fork expressjs/serve-static, I actually had to fork prismjs/send as well, which is the library serve-static used to initiate the send file stream, as the said library is actually the one responsible for checking whether a requested file exist in the configured uploads directory.
An alternative for not forking the said library would have been checking for file existence from lolisafe, but that'd introduce redundancy, as the said library would still try to check file existence by itself (meaning on every file access, its existence would have to be checked twice).
This all came to be cause serve-static wasn't designed with this specific use case in mind (that is querying DB to set headers).
But oh well.