BC break? - Fix for issue #81 - silently skip invalid header values coming in from clients

[sgehrig]

silently skip invalid header values coming in from a client

instead of throwing exceptions on invalid headers from clients, the new code silently skips these header values and continues to parse the header for a best match

[willdurand]

Great!

[willdurand]

Wondering if this is a BC break to be honest. The "BC break" is actually the bug itself, hence it is a bugfix IMO.. I know the behavior of the lib will change because it will not throw an exception anymore, but that behavior was faulty.

[sgehrig]

You're right. I think it's easy to argue that it's not a BC break because, as you said, the original implementation had some flaws.

[sgehrig]

https://twitter.com/phpalcohol/status/770562475951550464:

phpalcohol: @couac does the change break your API or not (were the exceptions explicitly part of your API declaration)? @lsmith

I think this is a good argument.

[willdurand]

thanks @sgehrig!

[stof]

@sgehrig you should rebase your branch so that the commit diff contains only the relevant changes instead of adding changes from other commits in your patch.

Anyway, I think this one qualifies as a bug fix (rejecting the whole negotiation when the client submits an invalid header value among others does not loo like something to be considered as a feature)

[sgehrig]

@stof: I think the PR contained only changes done to fix this issue. It was based on master from somehow this morning - so I think it was the most recent master branch available. The other PR (#82) was a different way to tackle the problem - but it wasn't complete.

[dunglas]

This is indeed a BC break: https://travis-ci.org/api-platform/core/jobs/156291020

[willdurand]

@dunglas do you agree that the behavior was incorrect?

[dunglas]

Yes, this change will allow me to remove some useless code.
However it may impact other people.