Fix mysqlnd protocol errors #16421
Fix mysqlnd protocol errors #16421
Conversation
|
Does this "match" libmysql-client behavior? I'm asking wrt PDO_MySQL which still may use either. |
It's an implementation detail so I don't think it matters. I am not even sure if we can compare them like that. Obviously, libmysql can't raise PHP E_WARNINGs so this PR might bring their behaviour more aligned. Libmysql raises client errors in similar situations too https://github.com/mysql/mysql-server/blob/trunk/libmysql/libmysql.cc#L3984 It's the intended way for clients (either libmysql or mysqlnd) to raise errors specific to their implementation. |
| @@ -863,9 +859,7 @@ php_mysqlnd_ok_read(MYSQLND_CONN_DATA * conn, void * _packet) | |||
|
|
|||
| DBG_RETURN(PASS); | |||
| premature_end: | |||
| DBG_ERR_FMT("OK packet %zu bytes shorter than expected", p - begin - packet->header.size); | |||
There was a problem hiding this comment.
why are you getting rid of all DBG_ERR_FMT?
There was a problem hiding this comment.
I guess that they will get logged from error (haven't checked though) but that bytes info can be useful there in some cases.
There was a problem hiding this comment.
Not all, just a couple that I saw now. Why are you asking? Are you using them? I think we should remove all of them, but it's a lot of work and since it's dead code, there's little harm in keeping them in the code. If someone ever needs to debug this piece of code, and they use this debug feature, they can add these lines themselves.
There was a problem hiding this comment.
In case people don't know. DBG_ERR_FMT is a macro that can be enabled with some compiler option and when it is on, mysqlnd will spew out debugging information to CLI. If it's off, then the compiler simply discards these lines.
kamil-tekiela
force-pushed
the
Fix-mysqlnd-protocol-errors
branch
from
6412634 to
11a272f
Compare
kamil-tekiela
force-pushed
the
Fix-mysqlnd-protocol-errors
branch
from
11a272f to
240d885
Compare
|
@bukka I have changed 3 more warnings from your recent PR. Is this PR ok now? |
I will try, but it may be very difficult. I don't know how to trigger many of these errors or whether it's even possible. |
|
Sure, ping me once you are done I will take a look if I maybe get an idea how to cover some of the missing ones. It's not a big deal if the difficult ones are not covered. |
The warnings are removed similarly to what was done in 2856afc
mysqlnd should have been raising client errors when the packet is malformed. In all places where it is possible without refactoring, I have changed the warnings to client errors. I have tested most of them to see if they are properly propagated.
There are still more places where warnings need to be converted into client errors.