pkulchenko / fullmoon

Fast and minimalistic Redbean-based Lua web framework in one file.
MIT License
684 stars 30 forks source link

multipart form validation #20

Closed ghost closed 1 year ago

ghost commented 1 year ago

I'm trying to ensure only certain types of files are accepted. When validating a file upload you just get the raw the bytes.

local validator = fm.makeValidator({
  { "file", msg = "...", test = function (file)
    -- file is the raw bytes; i.e. r.params.file.data
    -- it'd be nice to have file.data, file.headers, etc.
  end},
})

As a work around I'm using multi-file inputs everywhere and just processing the first file. This works because you get access to the full multipart table:

local validator = fm.makeValidator({
  { "files", msg = "...", test = function (files)
    local file = files[1]
    -- file.headers, filename, data etc. are there as expected
  end},
})

It got me thinking maybe the validators for single and multi file uploads could be more similar:

-- single file upload
local validator = fm.makeValidator({
  { "file", msg = "...", test = function (file)
    -- file.headers, filename, data etc.
  end},
})
-- multiple file upload
-- test function is called for each file instead of once as it is now
local validator = fm.makeValidator({
  { "files", msg = "...", test = function (file)
    -- file.headers, filename, data etc.
  end},
})

I could live without calling the test function for each of a multi-file upload but it seemed like a nice convenience. Having the full multipart table in the single file case would be most welcome.

pkulchenko commented 1 year ago

I thought about that, but I like the consistency of being able to extract parameter values in exactly the same way whether they are passed using multipart/form-data or application/x-www-form-urlencoded. If you do know that it's a multipart parameter, then you have two main options: extract them like you do using files[] syntax or extract them using { "multipart", test = function(multipart) end }. This makes it more explicit and still gives you access to everything you need (but only when the multipart parameters are indeed present).

I also thought about always returning a table, but making it behave as a string to handle these cases, but it's not possible to do it properly in many cases and requires overwriting len, concat, __tostring and some other metamethods and still leaves some wholes (for example, you can's use the comparison: file == "123" when file is a table and there is nothing I can do about it). To work properly it would require always using tostring on those parameters, which in my opinion is too much for the developer to do.

ghost commented 1 year ago

Thanks for the explaination.

{ "multipart", test = function(multipart) end }

This is the part I was missing. I didn't see that you can use "multipart" that way.

pkulchenko commented 1 year ago

Thanks @johnmuhl; it's good to hear that this option works for you. Would you expect it to be covered in the validation section? I do describe multipart pseudo-parameter in the section on multipart parameters, but I think it may benefit from adding a sentence or two about validation there.

ghost commented 1 year ago

After you mentioned it I went back to the form validation section to see if I had missed it so a note there would have helped me.