Skip to content

Fixed dump issue with non-string values#204

Merged
dg merged 1 commit into
nette:masterfrom
bckp:patch-1
Sep 14, 2018
Merged

Fixed dump issue with non-string values#204
dg merged 1 commit into
nette:masterfrom
bckp:patch-1

Conversation

@bckp

@bckp bckp commented Sep 11, 2018

Copy link
Copy Markdown
Contributor

If you call ResultSet::dump with null as column value, it will raise TypeError, so $col should be retyped to string, as htmlspecialchars expects string.

  • bug fix
  • BC break? no

@bckp

bckp commented Sep 11, 2018

Copy link
Copy Markdown
Contributor Author

Or maybe add new Helper, that will translate column value to the proper type (boolean, string, number, null)

If you call dump on ResultSet and non-string value (like null) exists, dump will raise TypeError exception on PHP7.1 and newer (maybe even on older version).
This add new static dumpVar method, that will try to escape value and return always string.
@bckp

bckp commented Sep 14, 2018

Copy link
Copy Markdown
Contributor Author

@dg ping, if you have any ideas, what should be changed, just give me call :)

@dg

dg commented Sep 14, 2018

Copy link
Copy Markdown
Member

I'll make it little bit simpler, ok? ;-)

@dg dg merged commit 1d1d2a6 into nette:master Sep 14, 2018
dg pushed a commit that referenced this pull request Sep 14, 2018
If you call dump on ResultSet and non-string value (like null) exists, dump will raise TypeError exception on PHP7.1 and newer (maybe even on older version).
This add new static dumpVar method, that will try to escape value and return always string.
dg pushed a commit that referenced this pull request Sep 14, 2018
If you call dump on ResultSet and non-string value (like null) exists, dump will raise TypeError exception on PHP7.1 and newer (maybe even on older version).
This add new static dumpVar method, that will try to escape value and return always string.
@bckp

bckp commented Sep 14, 2018

Copy link
Copy Markdown
Contributor Author

Sure :-)

@bckp bckp deleted the patch-1 branch September 14, 2018 11:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants