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

Hexstring converter to base64 #951

Merged
merged 7 commits into from
Sep 26, 2017
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 81 additions & 0 deletions core/tools.js
Original file line number Diff line number Diff line change
Expand Up @@ -1517,6 +1517,87 @@
return domEvent.button;
},

/**
* Convert hex string to array containing 1 byte in each cell. Bytes are represent as an Integer numbers.
*
* @since 4.8.0
* @param {String} hexString Contains input string which represent bytes, e.g. `"08A11D8ADA2B"`.
* @returns {Array} Bytes stored in a form of Integer numbers, e.g. `[ 8, 161, 29, 138, 218, 43 ]`.
*/
convertHexStringToBytes: function( hexString ) {
var bytesArray = [],
bytesArrayLength = hexString.length / 2,
i;

for ( i = 0; i < bytesArrayLength; i++ ) {
bytesArray.push( parseInt( hexString.substr( i * 2, 2 ), 16 ) );
}
return bytesArray;
},

/**
* Convert bytes array into base64 code.
*
* Bytes are `8bit` numbers, where base64 use `6bit` to store data. That's why we process 3 Bytes into 4 characters representing base64.
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this information in any way needed for the person using this API? I think not - this is an implementation detail of base64 and are not relevant for the person looking for this method.

Same goes for the "Algorithm" part. It's great that you have added it, but you did simply in a wrong place.

You don't want to put all of this information in our API docs, as there should be just summary. Instead it should be added as a inline comment insde the function, so that only programmers who work on this internals know it.

So I'll ask you to just do that, and we'll keep this great value of having these comments. 🙂

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed, first of all - kudos for explaining this in details, great job.

However, I'd recommend moving the detailed explanation of the algorithm to (internal) comments and leaving the public API docs more straightforward/basic. A nice example how we usually do it is the Magicline plugin with excellent comments by @oleq: https://github.com/ckeditor/ckeditor-dev/blob/major/plugins/magicline/plugin.js#L851

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Credit where it's due! Learn from the master, guys! :)

*
* Algorithm:
*
* 1. Take `3 * 8bit`.
* 2. If there is less than 3 byte, fill empty bits with zeros.
* 3. Transform `3 * 8bit` into `4 * 6bit` numbers.
* 4. Translate those numbers to proper character related to base64.
* 5. If extra zeros bytes were added fill them with `=` sign.
*
* Example:
*
* * Bytes Array: [ 8, 161, 29, 138, 218, 43 ] -> binary: `0000 1000 1010 0001 0001 1101 1000 1010 1101 1010 0010 1011`.
* * Binary: `0000 10|00 1010| 0001 00|01 1101| 1000 10|10 1101| 1010 00|10 1011` ← `|` (pipe) shows where base64 will cut bits during transformation.
* * Now we have 6bit numbers (written in decimal values), which are translated to indexes in `base64characters` array.<br />
* Decimal: `2 10 4 29 34 45 40 43` → base64: `CKEditor`.
*
* @since 4.8.0
* @param {Array} bytesArray An array which stores 1 byte in each cell as Integer number.
* @returns {String} Base64 string which represents input bytes.
Copy link
Contributor

Choose a reason for hiding this comment

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

Base64-encoded string.

*/
convertBytesToBase64: function( bytesArray ) {
var base64characters = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/',
base64string = '',
bytesArrayLength = bytesArray.length,
i;

for ( i = 0; i < bytesArrayLength; i += 3 ) {
var array3 = bytesArray.slice( i, i + 3 ),
array3length = array3.length,
array4 = [],
j;

if ( array3length < 3 ) {
for ( j = array3length; j < 3; j++ ) {
array3[ j ] = 0;
}
}

// 0xFC -> 11111100 || 0x03 -> 00000011 || 0x0F -> 00001111 || 0xC0 -> 11000000 || 0x3F -> 00111111
array4[ 0 ] = ( array3[ 0 ] & 0xFC ) >> 2;
array4[ 1 ] = ( ( array3[ 0 ] & 0x03 ) << 4 ) | ( array3[ 1 ] >> 4 );
array4[ 2 ] = ( ( array3[ 1 ] & 0x0F ) << 2 ) | ( ( array3[ 2 ] & 0xC0 ) >> 6 );
array4[ 3 ] = array3[ 2 ] & 0x3F;

for ( j = 0; j < 4; j++ ) {
// Example: if array3length == 1, then we need to add 2 equal sign at the end of base64.
// array3[ 0 ] is used to calculate array4[ 0 ] and array4[ 1 ], so there will be regular values,
// next two one has to be replaced with `=`, because array3[ 1 ] and array3[ 2 ] wasn't present in input string.
if ( j <= array3length ) {
base64string += base64characters.charAt( array4[ j ] );
} else {
base64string += '=';
}
}

}
return base64string;
},

/**
* A set of functions for operations on styles.
*
Expand Down
82 changes: 82 additions & 0 deletions tests/core/tools.js
Original file line number Diff line number Diff line change
Expand Up @@ -775,6 +775,88 @@
[ CKEDITOR.MOUSE_BUTTON_MIDDLE, isIe8 ? 4 : CKEDITOR.MOUSE_BUTTON_MIDDLE ],
[ CKEDITOR.MOUSE_BUTTON_RIGHT, isIe8 ? 2 : CKEDITOR.MOUSE_BUTTON_RIGHT ]
] );
},

// #662
'test hexstring to bytes converter': function() {
var testCases = [
{
hex: '00',
bytes: [ 0 ]
},
{
hex: '000000',
bytes: [ 0, 0, 0 ]
},
{
hex: '011001',
bytes: [ 1, 16, 1 ]
},
{
hex: '0123456789ABCDEF',
bytes: [ 1, 35, 69, 103, 137, 171, 205, 239 ]
},
{
hex: 'FFFFFFFF',
bytes: [ 255, 255, 255, 255 ]
},
{
hex: 'fc0fc0',
bytes: [ 252, 15, 192 ]
},
{
hex: '08A11D8ADA2B',
bytes: [ 8, 161, 29, 138, 218, 43 ]
}
];
CKEDITOR.tools.array.forEach( testCases, function( test ) {
assert.isTrue( CKEDITOR.tools.arrayCompare( test.bytes, CKEDITOR.tools.convertHexStringToBytes( test.hex ) ) );
Copy link
Contributor

Choose a reason for hiding this comment

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

There are dedicated assertions for array assertion (arrayAssert), see arrayAssert.itemsAreEqual.

} );
},

// #662
'test bytes to base64 converter': function() {
var testCases = [
{
bytes: [ 0 ],
base64: 'AA=='
},
{
bytes: [ 0, 0, 0 ],
base64: 'AAAA'
},
{
bytes: [ 1, 16, 1 ],
base64: 'ARAB'
},
{
bytes: [ 1, 35, 69, 103, 137, 171, 205, 239 ],
base64: 'ASNFZ4mrze8='
},
{
bytes: [ 255, 255, 255 ],
base64: '////'
},
{
bytes: [ 252, 15, 192 ],
base64: '/A/A'
},
{
bytes: [ 8, 161, 29, 138, 218, 43 ],
base64: 'CKEditor'
Copy link
Contributor

Choose a reason for hiding this comment

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

❤️

},
{
// jscs:disable
bytes: [ 0, 16, 131, 16, 81, 135, 32, 146, 139, 48, 211, 143, 65, 20, 147, 81, 85, 151, 97, 150, 155, 113, 215, 159, 130, 24, 163, 146, 89, 167, 162, 154, 171, 178, 219, 175, 195, 28, 179, 211, 93, 183, 227, 158, 187, 243, 223, 191 ],
// jscs:enable
base64: 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/'
}
];

CKEDITOR.tools.array.forEach( testCases, function( test ) {
assert.areSame( test.base64, CKEDITOR.tools.convertBytesToBase64( test.bytes ) );
} );
}

} );
} )();