-
Notifications
You must be signed in to change notification settings - Fork 39
Resolves issue #43 #136
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
Resolves issue #43 #136
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks a lot for picking this one up!
i added some suggestions. @sagikazarmark do we need to worry about a BC break when the default changes from sha1 to sha512?
d4ae46d
to
b641a6a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
finally got around to look at this. can we make the hashing completely configurable, while we are at it?
@@ -10,7 +10,7 @@ | |||
final class CookieJar implements \Countable, \IteratorAggregate | |||
{ | |||
/** | |||
* @var \SplObjectStorage | |||
* @var \SplObjectStorage<object, mixed> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix static analysis issue via PHPStan
:
------ --------------------------------------------------------------
Line CookieJar.php
------ --------------------------------------------------------------
15 Property Http\Message\CookieJar::$cookies with generic class
SplObjectStorage does not specify its types: TObject, TData
💡 You can turn this off by setting
checkGenericClassInNonGenericObjectType: false in your
phpstan.neon.dist.
------ --------------------------------------------------------------
use StreamDecorator { | ||
rewind as private doRewind; | ||
seek as private doSeek; | ||
} | ||
const BUFFER_SIZE = 8192; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix coding style issue:
2) src/Encoding/FilteredStream.php
---------- begin diff ----------
--- Original
+++ New
@@ -13,12 +13,11 @@
*/
abstract class FilteredStream implements StreamInterface
{
- const BUFFER_SIZE = 8192;
-
use StreamDecorator {
rewind as private doRewind;
seek as private doSeek;
}
+ const BUFFER_SIZE = 8192;
/**
* @var callable
----------- end diff -----------
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks a lot for the contribution, and thanks for your patience while we sorted this out. this looks ready to merge to me.
@php-http/core any further input or fine if merge and tag version 1.11.0 with this?
thanks a lot! i added documentation about this in php-http/documentation@aaeef9c and released this as https://github.com/php-http/message/releases/tag/1.11.0 if you have time to add a configuration option to the symfony bundle as well, that would be neat: https://github.com/php-http/HttplugBundle |
Closes #43.
What's in this PR?
hash('sha512')
to replacesha1
function.Checklist