Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Find a way how to work with binary files in hooks #617

Closed
aledeg opened this issue Aug 29, 2016 · 13 comments · Fixed by #1094
Closed

Find a way how to work with binary files in hooks #617

aledeg opened this issue Aug 29, 2016 · 13 comments · Fixed by #1094

Comments

@aledeg
Copy link

aledeg commented Aug 29, 2016

I don't know if I should open that ticket here or here.

I am trying to change the request body with the content of a file with a hook, but it does not work.
I have the following error:

SyntaxError: Unexpected end of input
  at Object.parse (native)
  at Socket.<anonymous> (/usr/lib/node_modules/dredd/lib/hooks-worker-client.js:268:34)
  at emitOne (events.js:90:13)
  at Socket.emit (events.js:182:7)
  at readableAddChunk (_stream_readable.js:153:18)
  at Socket.Readable.push (_stream_readable.js:111:10)
  at TCP.onread (net.js:534:20)

This is what I have in my hook:

$transaction->request->body = file_get_contents($filename);

Is this the good way of doing that?

Ping @ddelnano

@ddelnano
Copy link
Contributor

@aledeg can you give more information about the http request you are trying to send?? What the type of file this is, content type of the req should be, etc.

@aledeg
Copy link
Author

aledeg commented Aug 30, 2016

@ddelnano here is the information you wanted.

  • method used: POST
  • type of file: jpeg image
  • content type: image/*

Here is the actual image used for the tests:
default

I know that my api is working because I've tested it with behat and manually. It seems limited to dredd so far.

@aledeg
Copy link
Author

aledeg commented Oct 17, 2016

@honzajavorek Any insight about that issue?

@honzajavorek
Copy link
Contributor

@aledeg We're still missing a good canonical example for sending/testing binary data in Dredd using hooks. It's on my roadmap to prepare related examples, write tests for them so we're sure Dredd really works the way we recommend and put them to documentation. Related issues: #52 #87 and maybe even more.

From what you shared I can't tell exactly whether it's PHP hook handler issue, Dredd issue or issue in your hooks. Please try to investigate further what exactly might be happening so we can take a closer look.

  • What command line options do you use? $ dredd ( write your CLI options here )
  • What is in your dredd.yml?
  • What's your dredd --version output?
  • Does dredd --level=debug uncover something?

@aledeg
Copy link
Author

aledeg commented Oct 31, 2016

@honzajavorek I've made some more tests. Here is what I've got now.

  • error message:
SyntaxError: Unexpected end of JSON input
  at Object.parse (native)
  at Socket.<anonymous> (/usr/lib/node_modules/dredd/lib/hooks-worker-client.js:279:34)
  at emitOne (events.js:96:13)
  at Socket.emit (events.js:188:7)
  at readableAddChunk (_stream_readable.js:176:18)
  at Socket.Readable.push (_stream_readable.js:134:10)
  at TCP.onread (net.js:548:20)
  • dredd --version output:
dredd v2.2.3 (Linux 4.4.0-45-generic; x64)
  • cli options:
dredd --config tests/dredd.yml --level silly
  • dredd.yml content:
dry-run: null
hookfiles:
  - tests/Hook.php
language: vendor/bin/dredd-hooks-php
sandbox: false
server: 'bin/console server:start 127.0.0.1:8888 --force --quiet --env=test'
server-wait: 3
init: true
custom: {}
names: false
only: []
reporter:
  - apiary
  - dot
output: []
header: []
sorted: false
user: null
inline-errors: false
details: false
method: []
color: true
level: complete
timestamp: false
silent: false
path: []
hooks-worker-timeout: 50000
hooks-worker-connect-timeout: 15000
hooks-worker-connect-retry: 500
hooks-worker-after-connect-wait: 1000
hooks-worker-term-timeout: 50000
hooks-worker-term-retry: 500
hooks-worker-handler-host: localhost
hooks-worker-handler-port: 61321
blueprint: [test.apib]
endpoint: 'http://127.0.0.1:8888/app_dredd.php'
  • Hook content:
Hooks::before('Test picture', function (&$transaction) {
        $transaction->request->body = file_get_contents('dredd/default.jpg');
});
  • Apib content:
### Add picture [POST /picture]

+ Request (image/*)

+ Response 204

For what I understand, it looks like dredd is expecting JSON in the request. But obviously, I didn't provide some since I am pushing an image.
Digging in the code, I found this. It probably is the root of the problem.

@ddelnano I think that dredd-hook-php is not faulty.

@ddelnano
Copy link
Contributor

@aledeg the hooks server always JSON encodes the payloads fyi. https://github.com/ddelnano/dredd-hooks-php/blob/master/src/Server.php#L111-L119

So because of that I would expect Dredd to still need to decode the JSON but instead it needs to understand how to treat that data. Is the point you highlighted where Dredd decodes the json initially from the hooks server?

@aledeg
Copy link
Author

aledeg commented Oct 31, 2016

@ddelnano I wasn't aware of that.

I am not sure I understand your question. Could you please explain what you mean?

@ddelnano
Copy link
Contributor

@aledeg what I mean is from the sample hookfile you showed where you add the picture contents to the request body the hook server json encodes your data in this key. Dredd is then going to take that data and decode the json, which I would assume is correct since I would expect dredd to need to decode the json to get the original image content back.

So I am wondering if that area you identified in Dredd's source is the where Dredd decodes the json from the hooks server since my thinking would be it still needs to do that json decoding to get the original image content back.

If that is the area where Dredd decodes it maybe the way the text is encoded in the original json is the problem.

@aledeg
Copy link
Author

aledeg commented Oct 31, 2016

@ddelnano I see what you mean.
For what I understand in Coffee, it's the location where the server is decoding the input from the hook. But I am not 100% sure.

@honzajavorek
Copy link
Contributor

This is an interesting situation I didn't realise previously. You modify the transaction in PHP hooks by setting body to a binary data.

PHP hooks handler then encodes the transaction to JSON message to send it to Dredd over a socket. Dredd listens on socket, receives the JSON message, decodes this message, and then applies the changes to transaction according to what it received in the JSON message.

This error seems to happen when Dredd is receiving the JSON message from socket. The error message you posted makes me think Dredd receives incomplete JSON message from the socket. How is that possible? From what I see in the Coffee code, Dredd recognises \n as a separator of messages coming from the hook handler in the socket. So when Dredd meets \n, it thinks the JSON message is complete and tries to parse it. Obviously, when sending a binary file, the binary code, if interpreted as text, can contain any kind of characters, including \n.

This is my hypothesis on what happens, but I didn't perform any experiments to confirm my thoughts. Also, if this is really the case, I don't think there's an easy solution. In general, I would say Dredd isn't really ready to do any image comparison. It's something we didn't really try much nor have working examples prepared for.

Currently, the standard way dealing with binary responses would be to skip comparison of the body payload and let Dredd to judge only headers, parameters, status code, etc. That should be possible by hooks, but as I mentioned, needs some more tests, docs, and maybe fixes, since reportedly people are a bit grappling with it: #52 #87 ...

@honzajavorek honzajavorek changed the title [Question] how to send a file as a request body Find a way how to work with binary files in hooks Mar 8, 2017
@honzajavorek
Copy link
Contributor

Once #906 is done, we should add following tests and examples into docs, which work with binary responses:

  • example with response body
  • example ignoring response body
  • example with request body
  • example ignoring request body

@honzajavorek
Copy link
Contributor

Reminder: work in #836 should be considered once we're working on binary files support.

honzajavorek added a commit that referenced this issue Jul 25, 2018
Uses Base64 encoding as the serialization, which allows also non-JS
hooks to set request/response bodies to a binary content.

Close #617
Close #87
Close #836
honzajavorek added a commit that referenced this issue Jul 25, 2018
Uses Base64 encoding as the serialization, which allows also non-JS
hooks to set request/response bodies to a binary content.

Close #617
Close #87
Close #836
honzajavorek added a commit that referenced this issue Jul 26, 2018
Uses Base64 encoding as the serialization, which allows also non-JS
hooks to set request/response bodies to a binary content.

Close #617
Close #87
Close #836
@honzajavorek
Copy link
Contributor

This is going to get resolved, tested, and documented in #1094. Thank you @aledeg @ddelnano in participating here! It was really helpful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants