-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
All services need a manifest.json #4
Comments
I have some questions:
This is highly OnionShare specific. Isn't there any related standard, which we could use? E.g. some manifest format, which just lists all accessible files? (Where we hide the static UI stuff.) Or at least, remove info, which needs a lot of context to interpret? E.g. for the While thinking about that: Shouldn't we just point to the zip file, always? Or do both, like, "here's everything together", and "here's the single items".
Do we really need that? I mean, if the goal is, that only one app is needed, that means, both have the downloader, anyway. So for the user, this basically ends up to just being a question of who sends whom the link to operate on. But software-wise, that means we not only need a downloader, but also an uploader. That's a lot more to implement. Ok, maybe there's some benefit in having the users being able to decide who's the passive one, but does that justify the effort? Besides all that: From an API design standpoint, this "disable_text": false,
"disable_files": false seems quite ugly. Shouldn't we rather tell the consuming client, what we support, not what we don't want? Again, right now, this info needs a lot of context, but, when we do something like this, we probably should also think about downloader clients, which aren't necessarily OnionShare.
The "website" mode is just, well, a web server. What's the point of the manifest there at all? That thing is not meant for a non-browser to access. There's no additional information for a downloader software piece, but we do expose to an attacker, that OnionShare is used. |
Thanks for kicking this off Micah! I am only looking at share mode now as this is what we'll definitely need soon on mobile. Here are some random comments from the top of my brain:
While I'd also prefer the more simple approach as a developer, I understand that OnionShare desktop currently supports this feature and you probably don't want to remove the ability to download individual files from the desktop app, so the mobile download clients would need to be able to handle it as well, right? |
Sure, definitely be able to download single items, I'm not against that. What I meant was, instead of showing highly contextual flags like However, if we do want to keep that information, I would suggest not calling it |
I'm just now reading these replies.
The information listed for each mode just maps to the settings that can be set for that mode. Since the goal of the manifest file is to make OnionShare machine-readable so we can have non-Tor Browser clients (starting with Android and iOS, for at least certain modes), I think it makes sense to include the relevant settings that I client would need to understand in order to fully implement it. The link to download all the files is always The reason why it makes sense to include a full directory listing in all cases is so the client can show you what's in the the zip that you're about to download. Even if you can't download individual items, you should still be able to browse the folders to see what's there before deciding to download.
That's only the goal for Android and iOS. Since receive mode isn't part of MVP the mobile apps can ignore it for now, and it probably makes sense to do some user study work to figure out if it should ever be supported in mobile. Receive mode's primary use-case in desktop I think would be the same in mobile: offer a way for people to anonymously send you tips. You can post the onion link in your social media profiles, your website, etc., and then people can safely send you information without needing to figure out how to make an anonymous email address, without having to trust a service provider, etc. I could see it being useful for the anonymous tips to go straight to your phone.
It's just because We could use friendlier names, but it would just mean we'll have different names for the same settings in the server vs in the client.
This is a great point! I agree that website mode doesn't need a manifest.
Yes, good idea.
What if we make it way simpler, like instead of a nested object, just a list, with filenames that have trailing slashes being directories? Like: [
"/filename1.txt",
"/filename2.txt",
"/Documents/",
"/Documents/nested_file.txt",
"/Documents/Empty Folder/"
] Although, it occurs to me that the client will probably also want the file size in bytes for each file, which isn't included above... So maybe something like: [
{ "name": "/filename1.txt", "size": 1024 },
{ "name": "/filename2.txt", "size": 42 },
{ "name": "/Documents/" },
{ "name": "/Documents/nested_file.txt", "size": 15360 },
{ "name": "/Documents/Empty Folder/" }
]
Good point. If we change the file listing structure to above, then the download link for an individual file would just be its name. However, you will need to manually parse these if you plan to build a file browser, e.g. list everything in the root, and make
Yes, I would definitely prefer it if, for share mode at least, mobile and desktop were interoperable. |
I edited the text of the issue up at top to reflect some of the stuff you've all suggested. |
Where does that filename come from btw.? Is there a rule for generating it?
Sounds good!
Right. I guess there can be no slashes in the filenames, right? So each slash is a file path separator no matter which platform OnionShare was generating them on? In this case, I guess splitting a string by slashes is still better than supporting infinite nesting. It would be a bit more tricky to build the folder structure, but I don't see a way around this when not using potentially infinite trees. Btw. could there be zip bomb attacks? The zipped json that gets send over the wire is small, but explodes in the app that wants to parse it? |
If more than one file is shared and it zips them up, the filename is generated here: https://github.com/onionshare/onionshare/blob/develop/cli/onionshare_cli/web/share_mode.py#L546-L548 Basically, it's
These are great questions, and I honestly haven't given them much thought at all because OnionShare has never been a client before. We could do some validation, e.g. w can decide to limit the nesting to 100 folders deep (to pick something arbitrary) and throw errors if it's deeper than that. |
Yes, obviously your goal was to just map out relevant settings, and of course, from the view point of the programmers of the app, it's easier to keep the naming scheme. However, the point I want to drive home here, is, that, when you design an API, IMHO you should design it from the consumer's perspective. You should want to make it as easy as possible for somebody from the outside to understand what each knob is supposed to do, without consulting the source code or the UI first. That means:
Imagine somebody, who doesn't have a clue what OnionShare is, but wants to extend their downloader browser plugin to assist their users in automatically downloading stuff from OnionShare endpoints.
Yes, I already figured that out, and I suggest, you put that detailed info in the test plan. About implicit behaviour... see my last comment.
Agreed, that makes sense. I therefore suggest two sections:
(Might actually better be nested instead of two sections.)
Again, see my comment above...
[
{ "name": "/filename1.txt", "size": 1024 },
{ "name": "/filename2.txt", "size": 42 },
{ "name": "/Documents/" },
{ "name": "/Documents/nested_file.txt", "size": 15360 },
{ "name": "/Documents/Empty Folder/" }
] This looks great to me! I suggest to drop the leading slash, though, because that points to the root file system, typically. Paths are relative to the root of the ZIP or the OnionShare URL, so should these. (e.g. ZIP file entries are like that, too, AFAIK)
The JSON gets zipped? You mean implicitly gziped by the web server software? So, then, that's an inherent risk for all clients of any JSON API, isn't it? And the unzipping is transparently done by the HTTP client library used, which should, hopefully, be robust enough to not eat all the devices memory. No? Seems, like we would get sidetracked a lot with that question, if it turns out, there's no good HTTP client libraries out there handling that case. |
Picking this up again, because this is a needed feature. Should we put the manifest in the |
If there will be non-Tor Browser clients, then every service should have a machine-readable manifest at
/manifest.json
that the clients can read.The manifests will describe to the client the relevant general settings, and the relevant mode settings, for each mode. This is what the mode settings object looks like: https://github.com/onionshare/onionshare/blob/develop/cli/onionshare_cli/mode_settings.py#L37-L60
Here's what they should look like for each mode:
Share mode:
The reason the
autostop_sharing
setting should be exposed is so the client knows if it can download individual files or not. If this is false, the client's only option is to download all files as a zip. If it's true, it can download individual files.Filenames that end in a trailing slash are directories.
Receive mode:
Receive mode should expose
disable_text
anddisable_files
to allow the client to know what data it can submit.Website mode:
Website mode doesn't need a manifest.
Chat mode:
The text was updated successfully, but these errors were encountered: