Binary content no longer showing in terminal.

[bogdiw]

Solves #2306

Added an additional check to see if the output is binary. If so, an error message is printed.

[asggWa]

fantastico. tendremos que testearlo. gracias.

[fabricereix]

Thanks @bogdiw for your PR.
First, you can check the Contributing guide to sign your commits and run the integration tests.

Then, the behaviour in Hurl needs to be consistent with curl:

• the warnings only applies when outputing to a terminal
• output can still written with --output option
  ...

I'm going first to implement (#2312) as a prerequisite for your PR.

[bogdiw]

I've solved the cargo clippy errors, but the /tests_ok/assert_base64.py it's failing. Any advice in handling this? Maybe adding the test to /tests_failed should change it because now we don't print binary on screen anymore.

[fabricereix]

This test must clearly pass.
You can always double check with curl:

$ curl 'http://localhost:8000/assert-base64'
line1
line2
line3 

[bogdiw]

Ok, thank you. I'll try to fix the code then.

[jcamiel]

Hi @bogdiw the code doesn't seem OK to me:

let is_binary = response.body.iter().any(|&byte| !(32..=126).contains(&byte));

I don't understand why this condition is linked to the output being binary and doesn't seem correct to me. Could you explain it?

[bogdiw]

Hi @jcamiel my main idea was to check whether any byte in the response body is outside the ASCII range of printable characters (32 to 126).

[jcamiel]

I think we can have false positive doing this.
We should try to see how curl is doing it. In the proposed PR, I see another problem: we don't want to check every byte for a big file for instance (testing if a 1GB text file is binary would imply testing every bytes).

[jdicke]

Looks good

[hurl-bot]

📆 This PR has been closed because there is no activity (commits/comments) for more than 15 days 😥. Feel free to reopen it with new commits/comments.