balderdashy / sails

Realtime MVC Framework for Node.js
https://sailsjs.com
MIT License
22.82k stars 1.95k forks source link

Skipper: Does not respect quotes in Content-Disposition header #4709

Open thegoldenmule opened 5 years ago

thegoldenmule commented 5 years ago

Description:

Skipper requires quotes around name and filename parameters of multipart/form-data Content-Disposition headers. The builtin HTTP library for .NET Core (System.Net.Http.HttpClient) leaves out these quotes, which means that by default, .NET Core applications cannot upload files to sails.

The related RFC allows for both usages.

Details:

In my SailsJs project (v1.0.0), I have the following logic in a controller:

req
    .file('file')
    .upload((err, files) =>
    {
        if (0 === files.length) {
            sails.log('No file!')
        }
    });

In my .NET Core project I have the following upload logic:

var multipartContent = new MultipartFormDataContent();

var bytes = File.ReadAllBytes(srcPath);
var content = new ByteArrayContent(bytes, 0, bytes.Length);
content.Headers.ContentDisposition = new System.Net.Http.Headers.ContentDispositionHeaderValue("form-data")
{
    Name = "file",
    FileName = "test.png"
};
content.Headers.ContentType = new System.Net.Http.Headers.MediaTypeHeaderValue("image/jpeg");

multipartContent.Add(content);

This generates the following body headers:

Content-Disposition: form-data; name=file; filename=test.png
Content-Type: image/jpeg

My callback in SailsJs is given 0 files, as the body is incorrectly parsed.

Workaround:

There is a workaround in C# land-- simply add extra quotes. However, the default behavior of Skipper and .NET Core and incompatible. Since the RFC uses quotes, I believe skipper should be able to parse such a request.

content.Headers.ContentDisposition = new System.Net.Http.Headers.ContentDispositionHeaderValue("form-data")
{
    // Skipper requires quotes
    Name = "\"file\"",
    FileName = "\"test.png\""
};

This generates a request body that Skipper likes:

Content-Disposition: form-data; name="file"; filename="test.png"
Content-Type: image/jpeg
sailsbot commented 5 years ago

@thegoldenmule Thanks for posting, we'll take a look as soon as possible. In the meantime, if you haven’t already, please carefully read the issue contribution guidelines and double-check for any missing information above. In particular, please ensure that this issue is about a stability or performance bug with a documented feature; and make sure you’ve included detailed instructions on how to reproduce the bug from a clean install. Finally, don’t forget to include the version of Node.js you tested with, as well as your version of Sails or Waterline, and of any relevant standalone adapters/generators/hooks.

Thank you!


For help with questions about Sails, click here. If you’re interested in hiring @sailsbot and her Austin-based minions, click here.

streleck commented 5 years ago

Thanks @thegoldenmule for writing up this issue and providing a workaround! As far as I can tell, there aren't any current plans to work on this in Skipper, so hopefully other people having trouble connecting SailsJs and .Net because of the quotes will find this.

thegoldenmule commented 5 years ago

I or someone on my team would be happy to get involved. Would you or someone else be able to point me in the right direction in the codebase?

raqem commented 5 years ago

hi @thegoldenmule, We are currently in the process of moving all open Sails issues into one repo to help the devs keep a better eye on things. ~Cheers!

johnabrams7 commented 5 years ago

@thegoldenmule - Feel free to make a pull request for Skipper here: https://github.com/balderdashy/skipper/pulls . Thanks for the workaround! 👍