FAForever / client

FAF Python Client
GNU General Public License v3.0
74 stars 88 forks source link

Map upload function writes to deprecated stream api (?) #361

Open Justify87 opened 8 years ago

Justify87 commented 8 years ago

https://github.com/FAForever/client/blob/develop/src/vault/__init__.py#L128 self.client.writeToServer("UPLOAD_MAP", zipName, scenarioInfos, qfile)

https://github.com/FAForever/client/blob/develop/src/client/_clientwindow.py#L954 def writeToServer(self, action, _args, *_kw): ''' Writes data to the deprecated stream API. Do not use. '''

I haven't worked on the code yet, just took a look at it at the weekend. I'm not fimiliar with python but this seems to be a bug? Or is this only a warning and it still somehow works?

HardlySoftly commented 8 years ago

Map vault is just a web view of the page http://content.faforever.com/faf/vault/maps.php?username=example so there shouldn't be any need for client side code to manage map uploading.

I'm not sure when this uploadMap function was used in the past, but it doesn't seem to be in use now.

Someone should probably delete it, thanks for spotting that Justify :)

yorick-ne commented 8 years ago

I'm not quite sure about this, but i think the map upload button in the webpage triggered a call to that function. Seems to work the same way for the download button.

HardlySoftly commented 8 years ago

I was able to open a perfectly good file dialog through the whats new page, so is code for uploading maps explicitly needed?

yorick-ne commented 8 years ago

The client checks if the map you want to upload has all the neccesary files, checks if the lua used is valid and generates some map previews i think. All of these should probably be done on the server anyway, esp if you can upload with only the api and not the client. The api currently only seems to check if the file you want to upload is zipped ( atleast if it has .zip as extension, you could probably upload arbitrary files if you rename them to filename.zip) https://github.com/FAForever/api/blob/develop/api/maps.py#L38

HardlySoftly commented 8 years ago

I agree, and being able to upload arbitrary files surely isn't something to be addressed in the client.

heelix commented 8 years ago

Silly question - but is the plan that random folks can upload files, or this limited to privileged accounts?

On Wed, Jun 8, 2016 at 12:56 PM, HardlySoftly notifications@github.com wrote:

I agree, and being able to upload arbitrary files surely isn't something to be addressed in the client.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/FAForever/client/issues/361#issuecomment-224656393, or mute the thread https://github.com/notifications/unsubscribe/ACAzTJzr81IK1wSrxjHPmv3USUl9Iu5Aks5qJvQ1gaJpZM4IwPlp .

HardlySoftly commented 8 years ago

When the user connects to the map vault page they authenticate (in the clear) with name and password hash, see https://github.com/FAForever/client/blob/develop/src/vault/__init__.py#L59

So there is some authentication going on :)

yorick-ne commented 8 years ago

There isnt any authentification on the actual map upload over the api though. Atleast to my understanding.

Silly question - but is the plan that random folks can upload files, or this limited to privileged accounts?

IMO any FAF user should be able to upload maps/mods, which also is how it used to be. I dont actually know what the plan is, maybe one of the main devs can clear that up.