Facepunch / garrysmod-requests

Feature requests for Garry's Mod
86 stars 24 forks source link

Allow file.Write to create .xml, .properties and .json #1262

Closed Nak2 closed 3 years ago

Nak2 commented 5 years ago

I've had troubles with people editing textfiles wrong, causing scripts to break. It would be awesome to allow more file-formats for settings/data, so people can use the correct IDE.

Remscar commented 5 years ago

I'd really appreciate this as well.

PotcFdk commented 5 years ago

Can't we rather agree on a blacklist instead of a whitelist? I see why executables like .exe, shortcut files (.lnk), commonly-expected-to-be-evil files like .html, .docx, .pdf or potentially game-manipulating files like .cfg or whatever else aren't something that people want the game to be able to create, but I don't see the point in restricting, say, .id, .tex, .vcf or even .mp3/.ogg/... or any other extension that would otherwise have to get requested to be whitelisted.

If you're scared that a malicious server might run a malicious script on players to write a malicious file that is not an .exe, .com, .bat, .cmd, .com, .ps1, .mht, .msi then I think you're heavily overthinking the situation: Do you expect servers to write a malicious .png to a system and instruct the user to manually open that file in a vulnerable version of IrfanView or the like? As a normal user I'd rather just assume the /data folder to be a dangerous place and never, ever run any non-plaintext file from there, even though the blacklist would cover practivally every good way of doing something bad. Plus, it's not like there is any defined way of running a file from in-game anyways.

Sure, there's always this "you can't be sure the blacklist covers everything"-approach, but honestly, name one extension that wouldn't fall under the categories that I've mentioned above (that would be blacklisted), that could cause harm to a user even if they, for some dumb reason, decide to literally double-click all files in their /data folder.

Bonus points if you can do that without them having to join a server that is run by the mafia which obviously hoards srcds zero-days to RCE before you even spawn.

VaasKahnGrim commented 5 years ago

it would also be very helpful for mappers if you atleast added .lmp to the whitelist. we could then patch maps alot easier for certain things without having to do entire recompiles of everything.

sanny-io commented 5 years ago

@PotcFdk

As a normal user I'd rather just assume the /data folder to be a dangerous place and never, ever run any non-plaintext file from there, even though the blacklist would cover practivally every good way of doing something bad.

But you're not a normal user, you're a developer - and It's easier to manage a whitelist instead of a blacklist, so I think it should stay a whitelist. There's too much room for things to go wrong with a blacklist. I've noticed that all of the extensions you've mentioned are Windows specific. I wonder if that was intentional?

Sure, there's always this "you can't be sure the blacklist covers everything"-approach, but honestly, name one extension that wouldn't fall under the categories that I've mentioned above (that would be blacklisted), that could cause harm to a user even if they, for some dumb reason, decide to literally double-click all files in their /data folder.

.jar

PotcFdk commented 5 years ago

@sannysc

I've noticed that all of the extensions you've mentioned are Windows specific. I wonder if that was intentional?

Yes. Other systems have a better concept of dealing with this (executable flag / MIME type instead of file extension). Assigning this high amount of functionality to file extensions is mainly a Windows thing. If there are non-Windows filetypes to also consider, there is no reason not to include them, though! Feel free to add your .sh et cetera, but you can't run these anyways unless you set +x.

.jar

Falls under the category 'executable'.

sanny-io commented 5 years ago

@PotcFdk the problem with that is it's far simpler to make a blanket statement that you should just ban all potentially dangerous files than it is to actually ban all potentially dangerous files. When you say to ban them all but only list a few of them, of course it's easy for you to say "yeah that wouldn't be allowed".

Unless you are willing to provide a complete list, it's not a very fair argument to be making when my point is that there is probably going to be something that gets through the blacklist.

PotcFdk commented 5 years ago

I think I've said everything that I needed to say already. I don't want to repeat myself so I won't.

As an alternative I'm requesting the whitelisting of:

Kefta commented 5 years ago

You can use RegQueryValueEx in Windows.h with the "Content Type" key to detect if the file is an application or executable - you could allow any non-executable to be written and have executables refer to the whitelist.

For Linux, you can use stat in POSIX, but you could honestly hardcode the blacklist for that without much worry.

robotboy655 commented 5 years ago

Added .json in Dev, I don't see the point for .properties or .xml or anything else suggested.

Maybe .mp3/wav/ogg are a good idea too, but are they tho?

Kefta commented 5 years ago

Writing our own sound files would be great.

Cherry commented 5 years ago

.json support is awesome, and makes sense given the built-in util.JSONToTable. Since there's no first-class support for .properties or .xml, I don't see a lot of value in these.

Nak2 commented 5 years ago

Added .json in Dev, I don't see the point for .properties or .xml or anything else suggested.

Maybe .mp3/wav/ogg are a good idea too, but are they tho?

.properties are used for language files. Useful since it is supported in most IDE. .xml for settings and other things. Since it is a common file-format that most IDE's support. Even IE.

Kefta commented 5 years ago

.properties are also used in the base game already

robotboy655 commented 5 years ago

You can't use properties files directly. And what madman uses XML to store settings in GMod?

Nak2 commented 5 years ago

You can't use properties files directly. And what madman uses XML to store settings in GMod?

You can easily make a script that does. It would make it easier for people to translate mods. And I'm the madman who made a BSP parser and SoundScape-system. XML is still wildly used.

GitSparTV commented 5 years ago

XML is used in the GMod translation files. I see the sense in .json, .properties and .xml Don't see the sense in .mp3 because gmod doesn't support it. Maybe some ".gmod*" filetype filter. (As Lua pattern: `%.gmod.+$`)

meepen commented 5 years ago

Add csv files please

On Wed, Jul 31, 2019, 3:33 PM Spar notifications@github.com wrote:

XML is used in the GMod translation files. I see the sense in .json, .properties and .xml Don't see the sense in .mp3 because gmod doesn't support it. Maybe some ".gmod_*" filetype filter.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/Facepunch/garrysmod-requests/issues/1262?email_source=notifications&email_token=ABPGKSNG6PBSHTSLJHJXU63QCHSJZA5CNFSM4GNHWHSKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD3IKFYY#issuecomment-516989667, or mute the thread https://github.com/notifications/unsubscribe-auth/ABPGKSKW4BMLXVZ4WJVQXX3QCHSJZANCNFSM4GNHWHSA .

robotboy655 commented 5 years ago

You can easily make a script that does.

Then do, You don't need a special file type for your scripts.

XML is still wildly used.

I have never seen it used in GMod, and GMod has no built-in parser for it.

XML is used in the GMod translation files.

No it isn't

Don't see the sense in .mp3 because gmod doesn't support it.

Yes it does

Maybe some ".gmod*" filetype filter. (As Lua pattern: %.gmod.+$)

No

Add csv files please

Why?

meepen commented 5 years ago

csv files are really generic and very good to export to excel and databases. These would be good for people doing data analysis or database stuff as it's a very easy format

Nak2 commented 5 years ago

Then do, You don't need a special file type for your scripts.

It makes it easier to control files, edit and open them right. There is a reason why all data-files aren't .txt files.

I have never seen it used in GMod.

gxml - A XML parser MusicXML player .. thingy and I'm sure I can find more examples. The reason is that more people know xml than json. (And a shit ton of programs use XML still)

Why?

CSV is a simple data array file. Allows to import data to excel (Like logs 'n stuff)

thegrb93 commented 5 years ago

Save it as .txt? lol

Nak2 commented 5 years ago

Save it as .txt? lol

Not the point. Why do you think there are other data-formats than .txt?

thegrb93 commented 5 years ago

The only reason is to let the file manager be able to decide the default application to open the file. And gmod restricts this to prevent people going into their data folder seeing some kind of neat looking .doc file containing one of those classic microsoft word viruses or whatever. Doesn't stop it from being written in .txt but at least the user wouldn't be temped to double-click it and auto start microsoft word.

Nak2 commented 5 years ago

The only reason is to let the file manager be able to decide the default application to open the file. And gmod restricts this to prevent people going into their data folder seeing some kind of neat looking .doc file containing one of those classic microsoft word viruses or whatever. Doesn't stop it from being written in .txt but at least the user wouldn't be temped to double-click it and auto start microsoft word.

I'm not asking for .docx or .doc. (Read title) I'm asking for data files like .json, .xml and .properties. All files that are associated with data and not executable code.

If you read more I've stated a few reasons. Like IDE, users less prone to corrupting them and easier editing the data within.

thegrb93 commented 5 years ago

Use a symlink to your .txt file if you need ease of use. It shouldn't be up to the devs to maintain a whitelist of acceptable filename extensions. The whole idea is just stupid.

Kefta commented 5 years ago

It's stupid to have a whitelist at all, it should be a blacklist. Who just goes into their garrysmod/data folder and randomly executes files?

sanny-io commented 5 years ago

It's stupid to have a whitelist at all, it should be a blacklist. Who just goes into their garrysmod/data folder and randomly executes files?

Players can be directed to do so by the server admins, and a lot of players are kids. Kids can be easily convinced that they have to double click a file in their gmod folder in order to fix all the errors they're seeing, or whatever.

Kefta commented 5 years ago

They could just as easily direct them to download something malicious online or phish them through an in-game browser. There'd be nothing inherently malicious if executables were blacklisted using my previously posted solution.

Nak2 commented 5 years ago

I'm happy for the .json, but I would also like the .properties for language files.

I do understand that we might not get .xml files, but could we get .dat files instead?

Kefta commented 5 years ago

.dat is already allowed https://wiki.garrysmod.com/page/file/Write

Nak2 commented 5 years ago

.dat is already allowed https://wiki.garrysmod.com/page/file/Write

Didn't notice, I'm still advocating for .properties, but I'm happy with the outcome so far.

GitSparTV commented 5 years ago

XML is used in the GMod translation files.

No it isn't

Hm, yeah, it looks more like CSV or "key = value"

GitSparTV commented 5 years ago

Also, code_gs forgot to suggest: file.Write can write .vtf, but not .vmt

Kefta commented 5 years ago

I was going to make a separate suggestion for .vmt, .bsp, and .mdl.

Nak2 commented 5 years ago

I was going to make a separate suggestion for .vmt, .bsp, and .mdl.

If you can write your own .bsp files, you could make some crazy things. Dynamic maps, patching them or even "build" your own map with static-models. (Sadly the collision in maps for static props are baked into the map)

I'm not sure if writing your own vmt would be useful. You can do the same result with a CreateMaterial I think.

gonzalologorg commented 5 years ago

I've just found out that you can play wav and mp3 files from data folder, but as you know, you can't write mp3 or wav files, so playing those with EmitSound wouldn't work (Not showing up a warning or error, just don't play) Although to workaround this, creating a sound with sound.Add allows you to play it, so...

sound.Add({ name = "testpath", sound="../data/castor.dat"} ) 
player.GetByID(1):EmitSound("testpath") -- works
player.GetByID(1):EmitSound("../data/castor.dat") -- doesn't work
thegrb93 commented 5 years ago

I remember someone was able to get sounds to load from stuff written do data folder. Don't remember how they did it.

thegrb93 commented 5 years ago

Oh yeah, only works with the BASS module.

GitSparTV commented 4 years ago

Ladies and gentelmen, .vmt's are now writable.

robotboy655 commented 3 years ago

Added .xml, .csv, .jpeg (widespread alias of .jpg), .mp3, .wav and .ogg file formats to file.Write whitelist

.properties cannot be loaded from data/ so its not added.