stream validation fails

[smoebody]

Hi,
the master-code gives me a "double callback!" error when validating my file-response. i could nail it down to 17e77be, but i am fairly overwhelmed with what happens here. tag v0.10.1 works well.

this is my path-definition:

  produces:
    - 'image/png'
    - 'image/jpeg'
  responses:
    200:
      description: Successful request.
      schema:
        type: file

the controller responds with res.sendFile(filePath). i try to provide a test for it soon but maybe somebody has an idea already.

[whitlockjc]

I'll look into it.

[smoebody]

update: it seems to be a problem with express.sendFile(). i tried to set up a test and that seems to work (although I'm not sure if i did it right). but since the tests do not make use of express ...

[smoebody]

@whitlockjc, @SandyChapman, i made a test for this issue. i hope you can come up with a solution.

[whitlockjc]

I'll take a peek.

[stv8]

To backpack onto this, it appears in my project that validations don't properly validate when using pipe() from the request package. I believe that is a "stream" correct?

For example request.get(url).pipe(res) will "bypass" validations. I have set required values for the response in my swagger.yaml file but they have no effect.

Should I make a separate issue?

I want to also note that I am piping back JSON

[SandyChapman]

@smoebody @stv8 : I've had this PR open for some time now to address this issue I think. #367

Can either of you verify? If so, then maybe @whitlockjc would be kind enough to merge it.

[stv8]

@SandyChapman I can confirm that your change fixes my problem

[stv8]

@SandyChapman would you mind explaining what exactly your change does? I am a bit confused because it looks like it has to do with streaming files but I am just streaming a JSON response. Or is that basically the same thing as streaming a file?

[SandyChapman]

@stv8 : I believe what's fixing your problem is the portion of the code:

if(writtenData.length === 0) {
      // Some code expects val to be undefined and not an empty string
      // when no data is passed to 'end' or written with 'write'.
      val = undefined; 
} 

Based on my memory of 8 months ago and this comment, I think the validation occurs on any defined value. If val is undefined, the validation is skipped. Hence, if we write nothing to the output, we force the val to be undefined and bypass the validator.

The fix has to do with there being different code paths for pipe vs json or data.

The rest of the code is just ensuring that if you are using a file type for your response, we don't parse to a string and mangle the encoding. Validation isn't run on file types, but the conversion from buffer to string will mess of the encoding of binary files as its interpreted as UTF8 in the toString method, so in this case we know validation isn't happening and we don't need the string conversion.

I'm going off my memory here as I'm not a JS dev and it's been a while so this might not be correct, but hopefully it's helpful.

[stv8]

Thanks, I also think it might be this code you added as well. It appears to handle my JSON response because it is not a file but is an instance of buffer, which then sets val to be my JSON string. Now that val is defined it triggers the validations correctly.

var isFile = checkIsFile(schema, req.swagger.apiDeclaration ? '1.2' : '2.0');
     if (val instanceof Buffer && !isFile) {
     val = val.toString(encoding);
}

[smoebody]

i can confirm that it fixes this issue. conflicts need to be resolved manually though. @SandyChapman can you please have a look whether my test in PR #464 is obsolete by your PR's test or not?

[whitlockjc]

Once we get @smoebody's question answer, I'll take the appropriate course of action. Thanks for all the help everyone.

[smoebody]

@whitlockjc i merged @SandyChapman s fix in my PR #464 and resolved all conflicts. can you merge into master please? i need this :) http://giphy.com/gifs/tN1lvnT4M6nte

[alx13]

@smoebody I think that the better way is to always pass original intact response data (captured by validation middleware) to sendData function (obviously in case of successful validation).
But that fix is OK.
@whitlockjc merge please 👍

[lerit]

@smoebody @stv8 : I've had this PR open for some time now to address this issue I think. #367

Can either of you verify? If so, then maybe @whitlockjc would be kind enough to merge it.

this pull request resolve my problem