freesoftwarefactory / parse-multipart

A javascript/nodejs multipart/form-data parser which operates on raw data.
MIT License
48 stars 80 forks source link

Parses all data as buffers, rather than selectively depending on if data is file or not. #23

Closed samhuk closed 3 years ago

samhuk commented 5 years ago

in Parse.process(), should discriminate the parsing of form-data items depending on their content-type, i.e:

const fileContentTypes = [
  'application/pdf',
  ...
]
Object.defineProperty(parsedPart, 'type', {
  value: contentType,
  writable: true,
  enumerable: true,
  configurable: true,
})
Object.defineProperty(parsedPart, 'data', {
  value: fileContentTypes.some(c => c === contentType)
    ? Buffer.from(part.part)
    : String.fromCharCode.apply(String, part.part).replace(/\0/g,''),
  writable: true,
  enumerable: true,
  configurable: true,
})

This makes the resulting object "parts" a lot more useful to a consumer.

christiansalazar commented 5 years ago

Hello dear @sanuks1 :) well. let me start by saying that this is a abstract component, which will never discriminate if the payload is a file or a buffer. I dont agree on this issue (in good terms of course :) because in the worst case you can overload the process() method to provide the correct behavior that better match your requirements.

samhuk commented 5 years ago

Thank you for your response. "file or a buffer" is maybe not the right terminology here, since part.part is always a byte array. What you discriminate against is simply the contentType. I'de say it would make a lot of sense to a consumer if the plain/text part.part items are available as strings. It means that other people's code doesn't have to do the fileContentTypes <-> contentType mapping or the fromCharCode. This hasn't been asked before however, so maybe it isn't a worthwhile feature. I understand your point, which I think is to keep the code simple.