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

Add two buildin function ( decode and encode) #7622

Merged
merged 20 commits into from
Sep 10, 2018

Conversation

lanjingquan
Copy link
Contributor

Add two buildin function ( decode and encode):

  1. add crypt.go to util/encrypt
  2. modify buildin_encryption.go

What problem does this PR solve?

What is changed and how it works?

Check List

Tests

  • Unit test
  • Integration test
  • Manual test (add detailed scripts or steps below)
  • No code

Code changes

  • Has exported function/method change
  • Has exported variable/fields change
  • Has interface methods change
  • Has persistent data change

Side effects

  • Possible performance regression
  • Increased code complexity
  • Breaking backward compatibility

Related changes

  • Need to cherry-pick to the release branch
  • Need to update the documentation
  • Need to update the tidb-ansible repository
  • Need to be included in the release note

Add two buildin function ( decode and encode):
1) add crypt.go to util/encrypt
2) modify buildin_encryption.go
@sre-bot
Copy link
Contributor

sre-bot commented Sep 5, 2018

Hi contributor, thanks for your PR.

This patch needs to be approved by someone of admins. They should reply with "/ok-to-test" to accept this PR for running test automatically.

@CLAassistant
Copy link

CLAassistant commented Sep 5, 2018

CLA assistant check
All committers have signed the CLA.

mysql> select decode("pingancap","pass1234");
+--------------------------------+
| decode("pingancap","pass1234") |
+--------------------------------+
| .�[P�n���                           |
+--------------------------------+
1 row in set (0.32 sec)

mysql> select encode(decode("pingancap","pass1234"),"pass1234");
+---------------------------------------------------+
| encode(decode("pingancap","pass1234"),"pass1234") |
+---------------------------------------------------+
| pingancap                                         |
+---------------------------------------------------+
1 row in set (0.00 sec)

mysql>
@morgo
Copy link
Contributor

morgo commented Sep 5, 2018

Note that these functions are deprecated in 5.7 and removed in MySQL 8.0, as the crypto is now considered weak.

mysql> select decode("pingancap","pass1234");
+--------------------------------+
| decode("pingancap","pass1234") |
+--------------------------------+
| .�[P�n���                           |
+--------------------------------+
1 row in set (0.32 sec)

mysql> select encode(decode("pingancap","pass1234"),"pass1234");
+---------------------------------------------------+
| encode(decode("pingancap","pass1234"),"pass1234") |
+---------------------------------------------------+
| pingancap                                         |
+---------------------------------------------------+
1 row in set (0.00 sec)

mysql>
@zz-jason zz-jason added type/enhancement The issue or PR belongs to an enhancement. contribution This PR is from a community contributor. component/expression labels Sep 5, 2018
return "", true, errors.Trace(err)
}

decodeStr, _ := encrypt.SQLDecode(dataStr, passwordStr)
Copy link
Member

Choose a reason for hiding this comment

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

I think we should check the returned error

@zz-jason
Copy link
Member

zz-jason commented Sep 5, 2018

@lanjingquan Please fix CI.

@lanjingquan
Copy link
Contributor Author

how to run ci check again after i fix it ?

@mccxj
Copy link
Contributor

mccxj commented Sep 6, 2018

@lanjingquan when you push new code, ci will rerun automatically. You can cilck the detail link to check what is going wrong.

@winkyao
Copy link
Contributor

winkyao commented Sep 6, 2018

Do you port the code from MySQL Item_func_encode?

@zz-jason
Copy link
Member

zz-jason commented Sep 6, 2018

@lanjingquan You can run make dev in your local machine.

@lanjingquan
Copy link
Contributor Author

@winkyao I looked at the implementation code of MySQL and transferred them from C++ to Go to ensure that the same results could be obtained.

seed1 := nr & ((uint32(1) << 31) - uint32(1))
seed2 := nr2 & ((uint32(1) << 31) - uint32(1))

fmt.Println(seed1)
Copy link
Member

Choose a reason for hiding this comment

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

remove the debug code?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done! Thanks!

@zz-jason
Copy link
Member

zz-jason commented Sep 7, 2018

@XuHuaiyu PTAL

}

// SQLCrypt use to store initialization results
type SQLCrypt struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

We'd better not export struct SQLCrypt if it's not used outside module util/crypt

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK

isError bool
}{
{"", "", "", false},
{"pingcap", "1234567890123456", "2C35B5A4ADF391", false},
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

isError bool
}{
{"", "", "", false},
{"pingcap", "1234567890123456", "pingcap", false},
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

}{
{"", "", ""},
{"pingcap", "1234567890123456", "2C35B5A4ADF391"},
{"pingcap", "asdfjasfwefjfjkj", "351CC412605905"},
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add some cases for NULL inputs and other conditions, like:

mysql> select hex(decode("数据库", "asdfjasfwefjfjkj"));
+----------------------------------------------+
| hex(decode("数据库", "asdfjasfwefjfjkj"))    |
+----------------------------------------------+
| 8A5F1D65A3BECDD210                           |
+----------------------------------------------+
1 row in set, 1 warning (0.00 sec)

mysql> select hex(decode(12355.5555, "asdfjasfwefjfjkj"));
+---------------------------------------------+
| hex(decode(12355.5555, "asdfjasfwefjfjkj")) |
+---------------------------------------------+
| E2359202FA4CDD5AB4F2                        |
+---------------------------------------------+
1 row in set, 1 warning (0.00 sec)

mysql> select hex(decode("分布式データベース", "asdfjasfwefjfjkj"));
+----------------------------------------------------------------+
| hex(decode("分布式データベース", "asdfjasfwefjfjkj"))          |
+----------------------------------------------------------------+
| A9F2E51CEA86CC75EB4A579CA8133E9B346D5B4B610FB575637F34         |
+----------------------------------------------------------------+
1 row in set, 1 warning (0.00 sec)

mysql> select hex(decode("数据库", null));
+--------------------------------+
| hex(decode("数据库", null))    |
+--------------------------------+
| NULL                           |
+--------------------------------+
1 row in set (0.00 sec)

mysql> select hex(decode(null, "asdfjasfwefjfjkj"));
+---------------------------------------+
| hex(decode(null, "asdfjasfwefjfjkj")) |
+---------------------------------------+
| NULL                                  |
+---------------------------------------+
1 row in set (0.00 sec)

mysql> select hex(decode("pingcap", "密匙"));
+----------------------------------+
| hex(decode("pingcap", "密匙"))   |
+----------------------------------+
| CE5C02A5010010                   |
+----------------------------------+
1 row in set, 1 warning (0.01 sec)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

{"pingcap#%$%^", "*^%YTu1234567", "8634B9C55FF55E5B6328F449", false},
{"pingcap", "", "4A77B524BD2C5C", false},
{"分布式データベース","pass1234@#$%%^^&","80CADC8D328B3026D04FB285F36FED04BBCA0CC685BF78B1E687CE",false},
{"分布式データベース","分布式7782734adgwy1242","0E24CFEF272EE32B6E0BFBDB89F29FB43B4B30DAA95C3F914444BC",false},
Copy link
Contributor

Choose a reason for hiding this comment

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

Pls fix format

{"pingcap#%$%^", "*^%YTu1234567", "pingcap#%$%^", false},
{"pingcap", "", "pingcap", false},
{"分布式データベース","pass1234@#$%%^^&","分布式データベース",false},
{"分布式データベース","分布式7782734adgwy1242","分布式データベース",false},
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

{"pingcap#%$%^", "*^%YTu1234567", "8634B9C55FF55E5B6328F449"},
{"pingcap", "", "4A77B524BD2C5C"},
{"分布式データベース","pass1234@#$%%^^&","80CADC8D328B3026D04FB285F36FED04BBCA0CC685BF78B1E687CE"},
{"分布式データベース","分布式7782734adgwy1242","0E24CFEF272EE32B6E0BFBDB89F29FB43B4B30DAA95C3F914444BC"},
Copy link
Contributor

Choose a reason for hiding this comment

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

Pls fix fmt

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok

Copy link
Contributor

@spongedu spongedu left a comment

Choose a reason for hiding this comment

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

Thanks a lot! LGTM. @zz-jason PTAL

Copy link
Member

@zz-jason zz-jason left a comment

Choose a reason for hiding this comment

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

LGTM

@zz-jason zz-jason added the status/LGT2 Indicates that a PR has LGTM 2. label Sep 10, 2018
@zz-jason
Copy link
Member

/run-all-tests

@zz-jason zz-jason merged commit 8b1feeb into pingcap:master Sep 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/expression contribution This PR is from a community contributor. status/LGT2 Indicates that a PR has LGTM 2. type/enhancement The issue or PR belongs to an enhancement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants