pufferpanel / pufferd

The PufferPanel daemon
https://pufferd.pufferpanel.com
Apache License 2.0
46 stars 15 forks source link

Fixed backwards logic #43

Closed abates closed 7 years ago

abates commented 7 years ago

If the 'noform' query parameter does not exist, then FormFile must be used because the upload will be a multipart form. When 'noform' is set, the uploaded file is plain text in the body of the request and can be simply copied out to the local file. Prior to this commit, if 'noform' was absent from the query, then the multipart form was copied (in encoded form) to the file.

LordRalex commented 7 years ago

This would be a breaking change to https://github.com/PufferPanel/PufferPanel/blob/master/app/views/node/files/index.html#L293 as that's how we expected it, so I'll need to see which is correct, because we'll have to fix both sides it seems.

abates commented 7 years ago

Are you sure this is a breaking change? When noform is true the logic would imply that the PUT does not include a form, yet the code then parses the request as a multipart form. If that's really how it is supposed to work then conditional variable is really poorly named.

I know that without this change when I upload files using PufferPanel they end up being the encoded form rather than the binary content.

LordRalex commented 7 years ago

I think it might make more sense to reverse what the flag means, because a PUT doesn't make much sense using a form like that.

I think renaming the flag to just ?form would make more sense (as doing inverses make things a little more complicated), and it makes more sense to default the other way.

This way, we can remove the flag from the panel, and then correct the logic here.

Not sure why I did it that way now.

LordRalex commented 7 years ago

Although I do confirm that it's happening, I thought we fixed it already, but guess not.

LordRalex commented 7 years ago

I'll pull this in and accept it as a fix, however we'll revise this flag later. It's easier to push a change for pufferd than it is for PufferPanel, thanks!