Skip to content
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

Too long phpmyadmin version #141

Closed
nijel opened this issue May 9, 2017 · 6 comments
Closed

Too long phpmyadmin version #141

nijel opened this issue May 9, 2017 · 6 comments
Assignees
Labels

Comments

@nijel
Copy link
Contributor

nijel commented May 9, 2017

2017-05-09 08:15:42 Error: [PDOException] SQLSTATE[22001]: String data, right truncated: 1406 Data too long for column 'pma_version' at row 1 in /home/reports/error-reporting-server/vendor/cakephp/cakephp/src/Database/Statement/MysqlStatement.php on line 39
Request URL: /incidents/create
Client IP: [hidden]
Stack Trace:
#0 /home/reports/error-reporting-server/vendor/cakephp/cakephp/src/Database/Statement/MysqlStatement.php(39): PDOStatement->execute(NULL)
#1 /home/reports/error-reporting-server/vendor/cakephp/cakephp/src/Database/Connection.php(313): Cake\Database\Statement\MysqlStatement->execute()
#2 /home/reports/error-reporting-server/vendor/cakephp/cakephp/src/Database/Query.php(213): Cake\Database\Connection->run(Object(Cake\ORM\Query))
#3 /home/reports/error-reporting-server/vendor/cakephp/cakephp/src/ORM/Table.php(1916): Cake\Database\Query->execute()
#4 /home/reports/error-reporting-server/vendor/cakephp/cakephp/src/ORM/Table.php(1810): Cake\ORM\Table->_insert(Object(Cake\ORM\Entity), Array)
#5 /home/reports/error-reporting-server/vendor/cakephp/cakephp/src/ORM/Table.php(1723): Cake\ORM\Table->_processSave(Object(Cake\ORM\Entity), Object(ArrayObject))
#6 /home/reports/error-reporting-server/vendor/cakephp/cakephp/src/ORM/Table.php(1446): Cake\ORM\Table->Cake\ORM\{closure}()
#7 /home/reports/error-reporting-server/vendor/cakephp/cakephp/src/Database/Connection.php(680): Cake\ORM\Table->Cake\ORM\{closure}(Object(Cake\Database\Connection))
#8 /home/reports/error-reporting-server/vendor/cakephp/cakephp/src/ORM/Table.php(1447): Cake\Database\Connection->transactional(Object(Closure))
#9 /home/reports/error-reporting-server/vendor/cakephp/cakephp/src/ORM/Table.php(1724): Cake\ORM\Table->_executeTransaction(Object(Closure), true)
#10 /home/reports/error-reporting-server/src/Model/Table/IncidentsTable.php(187): Cake\ORM\Table->save(Object(Cake\ORM\Entity))
#11 /home/reports/error-reporting-server/src/Controller/IncidentsController.php(34): App\Model\Table\IncidentsTable->createIncidentFromBugReport(Array)
#12 /home/reports/error-reporting-server/vendor/cakephp/cakephp/src/Controller/Controller.php(440): App\Controller\IncidentsController->create()
#13 /home/reports/error-reporting-server/vendor/cakephp/cakephp/src/Http/ActionDispatcher.php(119): Cake\Controller\Controller->invokeAction()
#14 /home/reports/error-reporting-server/vendor/cakephp/cakephp/src/Http/ActionDispatcher.php(93): Cake\Http\ActionDispatcher->_invoke(Object(App\Controller\IncidentsController))
#15 /home/reports/error-reporting-server/vendor/cakephp/cakephp/src/Routing/Dispatcher.php(60): Cake\Http\ActionDispatcher->dispatch(Object(Cake\Http\ServerRequest), Object(Cake\Http\Response))
#16 /home/reports/error-reporting-server/webroot/index.php(36): Cake\Routing\Dispatcher->dispatch(Object(Cake\Http\ServerRequest), Object(Cake\Http\Response))
#17 {main}
@nijel nijel added the bug label May 9, 2017
@devenbansod
Copy link
Member

For this (and below mentioned ones), we can come up with a database migration (involving an ALTER TABLE ALTER COLUMN column_name ... statement).

Any suggestions welcome.

@nijel
Copy link
Contributor Author

nijel commented May 9, 2017

The question is whether this is desired or we should enforce some kind of validation. For example (here) the version probably could be sanitized to strip distro suffixes and include only information we really care about (see #102, though it mentioned sanitizing at rendering charts, but maybe it's better to do that earlier).

@devenbansod
Copy link
Member

Okay. Yes, that sounds good. We can strip the phpMyAdmin versions to just have numeric versions (example regex code at http://ideone.com/pisa9Z) while saving to the database (or should we do the change in phpMyAdmin while sending the error-report itself ?) and maybe while displaying the stats too.

But I am really not very sure how to handle the other truncations with respect to error message.

Also, I just realized that the the fields stacktrace and full_report are already text (which has a limit of 65,535 bytes). How frequent do such long reports truncation occurs? If they are very frequent, may be we can try and reduce some unused information (if there is any), while sending it from phpMyAdmin itself.

@nijel
Copy link
Contributor Author

nijel commented May 12, 2017

Changing it at phpMyAdmin side is not really needed, we need to validate the data at the error reporting server anyway, so I think it's best to do the cleanup there. It might be good idea to migrate existing data as well to have consistent data in the database and then no conversion on displaying stats would be needed.

As for the other fields, the most errors seem to happen for the error_message, see attached log. I'm not really sure if the truncations are valid data, but limiting stacktrace and full report to 64k seems reasonable, so I' just truncate these strings at PHP level to avoid error from MySQL.

truncations.txt

devenbansod added a commit to devenbansod/error-reporting-server that referenced this issue May 15, 2017
Fix phpmyadmin#141

Signed-off-by: Deven Bansod <devenbansod.bits@gmail.com>
@devenbansod
Copy link
Member

About the error_message, should we put debug logs before saving the incident to see what type of error messages are creating this truncations? If all these messages are just exceeding the current limit of 100, we can adjust the limit by a bit. Otherwise, we can truncate all error_message content to 100 characters before saving.

I think we use json_encode while saving and json_decode with stacktrace and full_report, while rendering them on view. So, if we truncate their content to 64k and it does not form a valid JSON, we would have to separately handle those errors as well. Any suggestions for a better approach?

@nijel
Copy link
Contributor Author

nijel commented May 16, 2017

I think we can start with logging and looking at the problematic reports and then decide what to do with them. So I'd suggest to check length of the field before saving and if that does not fit, log the content and do not save the report. We can then review the logs and see if those reports are valid and worth addressing, they are result of somebody inserting false data or the request is result of bug in client.

devenbansod added a commit to devenbansod/error-reporting-server that referenced this issue May 16, 2017
Fix phpmyadmin#141

Signed-off-by: Deven Bansod <devenbansod.bits@gmail.com>
@devenbansod devenbansod self-assigned this May 16, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants