Remove implicit ext-json dependency#4325
Conversation
|
The JSON extension is used in literally a couple of places of which only the usage for the JSON type is really justified. This doesn't look like a good reason to make it a core dependency. I'd rather rework the Exception class to not depend on it. As for the JSON type, its usage is optional, so it doesn't necessitate adding the dependency IMO (same as there's no dependency on any of the DB drivers). |
I agree, but as it stands, it is a core dependency at the moment, due to this usage. I'll try to refactor the Exception class then. Leaving this issue open until then. |
|
OK, I actually did it in this PR. Ready for review, @morozov! This would maybe benefit from being moved to a separate class, like |
| } | ||
|
|
||
| if (is_scalar($param)) { | ||
| if (is_string($param) && (! $this->isValidUTF8($param) || $this->containsNonPrintableASCIIChars($param))) { |
There was a problem hiding this comment.
After sleeping on it, I'm not sure if DBAL should be in the business of formatting prepared statement parameters at all.
The responsibility of the exception is to tell what went wrong by its error message and encapsulate the context for future handling or troubleshooting. By imprinting prepared statement parameters into the message, it may do a disservice: it makes it impossible to log the error message w/o logging the potentially sensitive information stored in the exception context (e.g. tokens, password hashes, personally identifiable information, etc).
Let's try to explore the approach of storing the parameters with the exception w/o making them part of the message. This way, the handling part will be free to process them in an appropriate way (e.g. ignore all parameters except for passwords and tokens) or ignore them.
|
This should probably target |
|
Closing in favor of #4387. |
Summary
The codebase contains calls to
json_encode(), yet this is not documented in the requirements.The
jsonextension can be disabled in PHP 7!