Mergedprologic merged 9 commits from
master2 months ago
Reference in new issue
There is no content yet.
Delete Branch 'gumbo2000/tube:preserve-video-filename'
Deleting a branch is permanent. It CANNOT be undone. Continue?
This should fix issue #40
I have not made the
allowed_charactersconfigurable yet nor the
Mostly because I couldn't decide if I should define those "globally" on a server basis, or on the library nodes.
Feel free to modify, extend, rip apart. 😁
Hmmm maybe we should make this configurable on a per-library basis? 🤔
Yeah. I thought so too. If we have different storage backends, they will have different sets of allowed characters too. But most people will have one library on one kind of backend (local Filesystem). Maybe a second library at a different path but most probable it will be the same backend...
So I will add this as an option on the server level AND on the library level.
But not just yet. First I will refractor the upload handler method, so that it becomes easier to do background processing.
BTW: I think we ought to add 'upload_allowed' as a boolean flag to the configuration of libraries. People might want to allow uploads only to one or a few selected libraries and keep other locations read-only.
Sounds good! 👌 Can you put the PR in WIP until you're done? 🙏
I think we can merge this as it is.
It vastly improves the user experience already as it is.
But yes, making the allowed_characters and replacement_characters configurable will make this a more complete change.
preserve-video-filenameto WIP: preserve-video-filename 3 months ago
Oh I'm happy to merge in that case, but I meant making the uuid vs. prserved fileaname behaviour configurable. I hope that's what we also mean? 🤔
Ah I thouht you were talking about making the sets of characters configurable.
Making the general "uuid vs. sanitized_original_name" configurable wasn't even on my radar. I presumed that preserving filenames was preferable over uuid-names in any case. But yes, now that I think about it, it should be configurable.
Haha all good 😅 Yeah in my case, Tube was used for the puprposes of uploading random videos from an iPhone and I run my instance mostly as a "demo" too for others to "try".
So in that sense, preserving original filenames for me doesn't make a lot of sense, I have Plex for that 😅
But as for "what users want", well hard to say, I'm not actually sure how many folks use Tube 😅
1342b7913f2 months ago
Originally I wanted to be able to set a default for
preserve_upload_filenameon the server level and the ability to override this on a per-path level in the library.
I just came across an ugly truth ...
A boolean value in json can have three values ( false, true, and absent ) and thus can perfectly encapsulate the three states that I wanted for the library-path-level ( shortuuid, preserve, do-what-the-server-default-says).
This collapses immediately into just two states (true and false) as soon as I read this json into a go struct.
So there will always be uncertainty if a false on the library-path-level is a user supplied value, or if it is the go default for a value that was omitted. 🤔
So, I guess I will have to change to logic a little bit.
preserve = (defined(library-path-level) ? library-path-level : server-level)
I will need to go with a simple
preserve = (library-path-level || server-level)logic.
Or is there an easy solution that I don't see yet?
When I think about adding some more variables to the upload handling, like
Somehow this looks like it is becoming rather ugly. It feels like there is another class / struct hiding in there.
It looks like we might have something like a
UploadConfigclass/struct there, who's configuration node exists in the "server" and also optionally inside each library path.
What if you use the securejoin package? Rather than trying to come up with "allow lists" and what not, use this to safelt and programatyically determine the final path. If it errors out, then either the supplied path was invalid or otherwise.
I use this package in a few of my other projects where user input is used to construct path naems.
Securejoin sounds good. 👍
I wonder if we will have to revisit this topic when we add alternative backend storage engines. 🤔 But that is a problem for another time. For now I think securejoin is more than good enough.
I think I will invert the
upload_allowedboolean config variable to
That way, existing configuration files stay valid. (Non-existing
readonlyvariables default to
72fc04d7642 months ago
@gumbo2000 Is this ready to go? Still has
@prologic Ready? 🤔 I was already halfway done, implementing the "readonly" feature. 😆
But, thinking about it, that would be a different feature and I am not finished documenting the
So, yes. I will finish documentation this and then this can be merged.
Any other places apart from the adding it to
Speaking of the
Do you prefer inline documentation in the json snippet (even if it violates json syntax),
or would you rather have the explanation underneath a valid json document?
BTW: I hope you are right about the security and maturity of
SecureJoin. In theory it would allow a lot more than the strict whitelist would permit. E.g. you could write into subdirectories of the chosen library location.
I've removed that possibility by stripping the path away from the name supplied by the user but code drifts and future changes might expose that possibility again.
OTOH it takes that burden and responsibility of configuring the allowed_characters away from the tube administrator.
ae5b61545e2 months ago
27ff679a3a2 months ago
WIP: preserve-video-filenameto preserve-video-filename 2 months ago
Ok. I think this one is ready.
19a1141af3into master 2 months ago