sebastianbergmann / phpunit

The PHP Unit Testing framework.
https://phpunit.de/
BSD 3-Clause "New" or "Revised" License
19.69k stars 2.2k forks source link

Binary strings in assertion messages #736

Closed p closed 9 years ago

p commented 11 years ago

https://gist.github.com/30d38ecbe6bf2f5c4f5d

It seems nobody on our team knows what a binary string is or how to convert it to a normal string. Any ideas?

To clarify, I want to see the readable text in the assertion message itself.

This is the test file: https://github.com/phpbb/phpbb3/blob/develop/tests/functional/fileupload_form_test.php

ghost commented 11 years ago

It looks like you're trying to compare a binary file to some HTML.

My guess is that whatever the binary string is, it's the contents of the test file that was "uploaded".

p commented 11 years ago

No, it is actually the response.

We hunted down where this originated:

https://github.com/sebastianbergmann/phpunit/blob/master/PHPUnit/Util/Type.php#L129-132

In the case being discussed, the data is mostly ascii. We use unicode quotes in language strings, perhaps they or similar are making phpunit decide the string is binary.

A more practical implementation would check to see how many nonprintable characters a string has. If, say, less than 10%, then only replace those with \xxx escapes.

edorian commented 11 years ago

Escaping non printable chars with \xxx and mixing that with the actual output would create an irreversible output that can't be turned back.

With the current way all that is needed to get the binary data is to take it and php -r 'hex2bin()' it.

If you have chars that are printable but falsely detected by the regex (maybe due to missing flags?) let me know but I don't think creating a completely unparseable / irreversible output string would help anyone.

For comparing html assertTag might also be an option.

Closing this for now, feel free to get back to me on this if you disagree

p commented 11 years ago

https://github.com/p/php-test-repo/blob/master/phpunit/binary_string/test.php https://github.com/p/php-test-repo/blob/master/phpunit/binary_string/actual_output

What is your definition of "printable"?

The characters I used are supposed to be used by all language strings in phpbb:

https://github.com/phpbb/phpbb3/blob/develop/phpBB/language/en/common.php#L37

We tell people to replace ascii quotes and ellipses with those unicode ones.

p commented 11 years ago

Escaping non printable chars with \xxx and mixing that with the actual output would create an irreversible output that can't be turned back.

I'm sorry but I don't understand this. How are \xxx escapes irreversible?

edorian commented 11 years ago

What is your definition of "printable"?

The definition in the code is the one you linked to: https://github.com/sebastianbergmann/phpunit/blob/master/PHPUnit/Util/Type.php#L129-132 If that can be improved let me know :)

I'm sorry but I don't understand this. How are \xxx escapes irreversible?

The output string might be "Hello \0123 whatever" and mixing this with escaped unicode chars would make it impossible to turn the string back without breaking it in the process?


Thanks for the reproduce case in https://github.com/p/php-test-repo/blob/master/phpunit/binary_string/test.php.

Those should all get printed and not be a cause of the output turned into binary.

p commented 11 years ago

You could have three output formats:

  1. ascii text
  2. ascii text with non-ascii characters escaped and backslashes etc. also escaped to make the inverse correct
  3. "binary string"
sebastianbergmann commented 9 years ago

Dear contributor,

let me start by apologizing for not commenting and/or working on the issue you have reported or merging the pull request you have sent sooner.

PHPUnit 5.0 was released today. And today I am closing all open bug reports and pull requests for PHPUnit and its dependencies that I maintain. Please do not interpret the closing of this ticket as an insult or a lack of interest in your problem. I am sorry for any inconvenience this may cause.

If the topic of this ticket is still relevant then please open a new ticket or send a new pull request. If your ticket or pull request is about a defect then please check whether the issue still exists in PHPUnit 4.8 (which will received bug fixes until August 2016). If your ticket or pull request is about a new feature then please port your patch PHPUnit 5.0 before sending a new pull request.

I hope that today's extreme backlog grooming will allow me to respond to bug reports and pull requests in a more timely manner in the future.

Thank you for your understanding, Sebastian