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

change type of Hash.hash to MEDIUMTEXT instead of TEXT to allow longer hashes #867

Merged
merged 4 commits into from
Mar 21, 2023

Conversation

s3inlc
Copy link
Member

@s3inlc s3inlc commented Mar 15, 2023

closes issue #851
closes issue #841

@s3inlc s3inlc requested a review from zyronix March 15, 2023 09:39
@@ -10,4 +12,9 @@
require_once(dirname(__FILE__) . "/../../inc/defines/config.php");
require_once(dirname(__FILE__) . "/../../inc/defines/log.php");

if (!isset($PRESENT["v0.13.x_hash_length"])) {
Factory::getAgentFactory()->getDB()->query("ALTER TABLE `Hash` MODIFY `hash` MEDIUMTEXT NOT NULL;");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if the database type is set to longtext? This change would modify that and thus cause issue when upgrading. Maybe add a check to this to check if the type is TEXT.

@zyronix
Copy link
Member

zyronix commented Mar 20, 2023

@s3inlc I have fixed my own suggestion to double check that the column type is TEXT or smaller. I know that some people ahve already increased the size to LONGTEXT. This should prevent that from erroring or reverting back to MEDIUMTEXT. Could you verify my changes?

@zyronix zyronix linked an issue Mar 20, 2023 that may be closed by this pull request
@s3inlc
Copy link
Member Author

s3inlc commented Mar 21, 2023

Thanks, looks good. You're right, we should only increase if it's smaller.

@s3inlc s3inlc merged commit 267e965 into master Mar 21, 2023
@s3inlc s3inlc deleted the enhancement/hash-length branch March 21, 2023 20:08
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.

Hashes over 65535 in length triggers database error
2 participants