Skip to content

Commit

Permalink
deprecate utf8Encode and utf8Decode async calls and rename methods (a…
Browse files Browse the repository at this point in the history
…mpproject#13091)

* deprecate async calls and rename methods

* additional fixes
  • Loading branch information
alabiaga authored and William Chou committed Jan 29, 2018
1 parent d42cd14 commit 0f3fe05
Show file tree
Hide file tree
Showing 14 changed files with 55 additions and 108 deletions.
2 changes: 1 addition & 1 deletion extensions/amp-a4a/0.1/amp-a4a.js
Original file line number Diff line number Diff line change
Expand Up @@ -1481,7 +1481,7 @@ export class AmpA4A extends AMP.BaseElement {
'releaseType': this.releaseType_,
});
const checkStillCurrent = this.verifyStillCurrent();
return utf8Decode(creativeBody).then(creative => {
return Promise.resolve(utf8Decode(creativeBody)).then(creative => {
checkStillCurrent();
let srcPath;
let name = '';
Expand Down
6 changes: 3 additions & 3 deletions extensions/amp-a4a/0.1/test/test-signature-verifier.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import * as sinon from 'sinon';

import {dev, user} from '../../../../src/log';
import {base64EncodeFromBytes} from '../../../../src/utils/base64';
import {utf8EncodeSync} from '../../../../src/utils/bytes';
import {utf8Encode} from '../../../../src/utils/bytes';
import {SignatureVerifier, VerificationStatus} from '../signature-verifier';

const networkFailure = {throws: new TypeError('Failed to fetch')};
Expand All @@ -43,8 +43,8 @@ describes.fakeWin('SignatureVerifier', {amp: true}, env => {
.once();
};

const creative1 = utf8EncodeSync('Hello world!');
const creative2 = utf8EncodeSync('This is a <em>test</em> creative.');
const creative1 = utf8Encode('Hello world!');
const creative2 = utf8Encode('This is a <em>test</em> creative.');

// TODO(bradfrizzell, #12476): Make this test work with sinon 4.0.
it.skip('should make no network requests when crypto is unavailable', () => {
Expand Down
6 changes: 3 additions & 3 deletions extensions/amp-access/0.1/jwt.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
import {
base64UrlDecodeToBytes,
} from '../../../src/utils/base64';
import {stringToBytes, utf8DecodeSync} from '../../../src/utils/bytes';
import {stringToBytes, utf8Decode} from '../../../src/utils/bytes';
import {pemToBytes} from '../../../src/utils/pem';
import {tryParseJson} from '../../../src/json';

Expand Down Expand Up @@ -127,8 +127,8 @@ export class JwtHelper {
const headerUtf8Bytes = base64UrlDecodeToBytes(parts[0]);
const payloadUtf8Bytes = base64UrlDecodeToBytes(parts[1]);
return {
header: tryParseJson(utf8DecodeSync(headerUtf8Bytes), invalidToken),
payload: tryParseJson(utf8DecodeSync(payloadUtf8Bytes), invalidToken),
header: tryParseJson(utf8Decode(headerUtf8Bytes), invalidToken),
payload: tryParseJson(utf8Decode(payloadUtf8Bytes), invalidToken),
verifiable: `${parts[0]}.${parts[1]}`,
sig: parts[2],
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ export class AmpAdNetworkAdzerkImpl extends AmpA4A {
}
// Shorthand for: reject promise if current promise chain is out of date.
const checkStillCurrent = this.verifyStillCurrent();
return utf8Decode(bytes).then(body => {
return Promise.resolve(utf8Decode(bytes)).then(body => {
checkStillCurrent();
this.ampCreativeJson_ = /** @type {!AmpTemplateCreativeDef} */
(tryParseJson(body) || {});
Expand All @@ -122,7 +122,8 @@ export class AmpAdNetworkAdzerkImpl extends AmpA4A {
.then(parsedTemplate => {
this.creativeMetadata_ = /** @type {!CreativeMetaDataDef} */
(super.getAmpAdMetadata(parsedTemplate));
return utf8Encode(this.creativeMetadata_.minifiedCreative);
return Promise.resolve(
utf8Encode(this.creativeMetadata_.minifiedCreative));
})
.catch(error => {
dev().warn(TAG, 'Error fetching/expanding template',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import {
import {AmpMustache} from '../../../amp-mustache/0.1/amp-mustache';
import {createElementWithAttributes} from '../../../../src/dom';
import {Xhr} from '../../../../src/service/xhr-impl';
import {utf8EncodeSync, utf8Decode} from '../../../../src/utils/bytes';
import {utf8Encode, utf8Decode} from '../../../../src/utils/bytes';

describes.fakeWin('amp-ad-network-adzerk-impl', {amp: true}, env => {
let win, doc;
Expand Down Expand Up @@ -126,15 +126,15 @@ describes.fakeWin('amp-ad-network-adzerk-impl', {amp: true}, env => {
text: () => template,
}));
return impl.maybeValidateAmpCreative(
utf8EncodeSync(JSON.stringify(adResponseBody)).buffer,
utf8Encode(JSON.stringify(adResponseBody)).buffer,
{
get: name => {
expect(name).to.equal(AMP_TEMPLATED_CREATIVE_HEADER_NAME);
return 'amp-mustache';
},
},
() => {})
.then(buffer => utf8Decode(buffer))
.then(buffer => Promise.resolve(utf8Decode(buffer)))
.then(creative => {
expect(impl.getAmpAdMetadata()).to.jsonEqual({
minifiedCreative: creative,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1205,7 +1205,7 @@ export class AmpAdNetworkDoubleclickImpl extends AmpA4A {
/** @type {?../../../src/service/xhr-impl.FetchResponse} */
({
headers,
arrayBuffer: () => utf8Encode(creative),
arrayBuffer: () => Promise.resolve(utf8Encode(creative)),
});
// Pop head off of the array of resolvers as the response
// should match the order of blocks declared in the ad url.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,32 +148,28 @@ describes.realWin('DoubleClick Fast Fetch Fluid', realWinConfig, env => {

it('should style iframe/slot correctly on multi-size creative', () => {
multiSizeImpl.buildCallback();
return utf8Encode('foo').then(creative => {
multiSizeImpl.sentinel = 'sentinel';
multiSizeImpl.adPromise_ = Promise.resolve();
multiSizeImpl.creativeBody_ = creative;
multiSizeImpl.returnedSize_ = {width: 250, height: 100};
return multiSizeImpl.layoutCallback().then(() => {
const iframeStyleString = multiSizeImpl.iframe.getAttribute('style');
const slotStyleString = multiSizeImpl.element.getAttribute('style');
expect(slotStyleString).to.match(/width: 250px/);
expect(iframeStyleString).to.match(/position: relative/);
expect(multiSizeImpl.element.getAttribute('height')).to.be.null;
});
multiSizeImpl.sentinel = 'sentinel';
multiSizeImpl.adPromise_ = Promise.resolve();
multiSizeImpl.creativeBody_ = utf8Encode('foo');
multiSizeImpl.returnedSize_ = {width: 250, height: 100};
return multiSizeImpl.layoutCallback().then(() => {
const iframeStyleString = multiSizeImpl.iframe.getAttribute('style');
const slotStyleString = multiSizeImpl.element.getAttribute('style');
expect(slotStyleString).to.match(/width: 250px/);
expect(iframeStyleString).to.match(/position: relative/);
expect(multiSizeImpl.element.getAttribute('height')).to.be.null;
});
});

it('should have an iframe child with initial size 0x0', () => {
impl.buildCallback();
return utf8Encode('foo').then(creative => {
impl.sentinel = 'sentinel';
impl.adPromise_ = Promise.resolve();
impl.creativeBody_ = creative;
return impl.layoutCallback().then(() => {
const styleString = impl.iframe.getAttribute('style');
expect(styleString).to.match(/width: 0px/);
expect(styleString).to.match(/height: 0px/);
});
impl.sentinel = 'sentinel';
impl.adPromise_ = Promise.resolve();
impl.creativeBody_ = utf8Encode('foo');
return impl.layoutCallback().then(() => {
const styleString = impl.iframe.getAttribute('style');
expect(styleString).to.match(/width: 0px/);
expect(styleString).to.match(/height: 0px/);
});
});

Expand Down Expand Up @@ -202,15 +198,13 @@ describes.realWin('DoubleClick Fast Fetch Fluid', realWinConfig, env => {
sandbox.spy(impl, 'connectFluidMessagingChannel');
const onFluidResizeSpy = sandbox.spy(impl, 'onFluidResize_');
impl.attemptChangeHeight = () => Promise.resolve();
return utf8Encode(rawCreative).then(creative => {
impl.sentinel = 'sentinel';
impl.initiateAdRequest();
return impl.adPromise_.then(() => {
impl.creativeBody_ = creative;
return impl.layoutCallback().then(() => {
expect(connectFluidMessagingChannelSpy).to.be.calledOnce;
expect(onFluidResizeSpy).to.be.calledOnce;
});
impl.sentinel = 'sentinel';
impl.initiateAdRequest();
return impl.adPromise_.then(() => {
impl.creativeBody_ = utf8Encode(rawCreative);
return impl.layoutCallback().then(() => {
expect(connectFluidMessagingChannelSpy).to.be.calledOnce;
expect(onFluidResizeSpy).to.be.calledOnce;
});
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ describes.realWin('amp-ad-network-doubleclick-impl', config , env => {
method: 'GET',
credentials: 'include',
}).returns(Promise.resolve({
arrayBuffer: () => utf8Encode(creative),
arrayBuffer: () => Promise.resolve(utf8Encode(creative)),
bodyUsed: false,
headers: new FetchResponseHeaders({
getResponseHeader(name) {
Expand Down
4 changes: 2 additions & 2 deletions extensions/amp-iframe/0.1/amp-iframe.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import {removeElement, closestBySelector} from '../../../src/dom';
import {removeFragment, parseUrl, isSecureUrl} from '../../../src/url';
import {Services} from '../../../src/services';
import {user, dev} from '../../../src/log';
import {utf8EncodeSync} from '../../../src/utils/bytes.js';
import {utf8Encode} from '../../../src/utils/bytes.js';
import {urls} from '../../../src/config';
import {moveLayoutRect} from '../../../src/layout-rect';
import {setStyle} from '../../../src/style';
Expand Down Expand Up @@ -202,7 +202,7 @@ export class AmpIframe extends AMP.BaseElement {
this.element);

return 'data:text/html;charset=utf-8;base64,' +
base64EncodeFromBytes(utf8EncodeSync(srcdoc));
base64EncodeFromBytes(utf8Encode(srcdoc));
}

/** @override */
Expand Down
4 changes: 2 additions & 2 deletions src/service/crypto-impl.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
import {registerServiceBuilder, getService} from '../service';
import {dev} from '../log';
import {Services} from '../services';
import {stringToBytes, utf8EncodeSync} from '../utils/bytes';
import {stringToBytes, utf8Encode} from '../utils/bytes';
import {base64UrlEncodeFromBytes} from '../utils/base64';

/** @const {string} */
Expand Down Expand Up @@ -165,7 +165,7 @@ export class Crypto {
dev().assert(this.isPkcsAvailable());
// Safari 10 and earlier want this as an ArrayBufferView.
const keyData = this.isLegacyWebkit_
? utf8EncodeSync(JSON.stringify(/** @type {!JsonObject} */ (jwk)))
? utf8Encode(JSON.stringify(/** @type {!JsonObject} */ (jwk)))
: /** @type {!webCrypto.JsonWebKey} */ (jwk);
return /** @type {!Promise<!webCrypto.CryptoKey>} */ (
this.subtle.importKey('jwk', keyData, this.pkcsAlgo, true, ['verify'])
Expand Down
4 changes: 2 additions & 2 deletions src/service/xhr-impl.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import {
} from '../url';
import {parseJson} from '../json';
import {isArray, isObject} from '../types';
import {utf8EncodeSync} from '../utils/bytes';
import {utf8Encode} from '../utils/bytes';
import {getMode} from '../mode';
import {dict, map} from '../utils/object';
import {fromIterator} from '../utils/array';
Expand Down Expand Up @@ -776,7 +776,7 @@ export class FetchResponse {
*/
arrayBuffer() {
return /** @type {!Promise<!ArrayBuffer>} */ (
this.drainText_().then(utf8EncodeSync));
this.drainText_().then(utf8Encode));
}
}

Expand Down
50 changes: 2 additions & 48 deletions src/utils/bytes.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,71 +16,25 @@

import {dev} from '../log';

/**
* Interpret a byte array as a UTF-8 string.
* @param {!BufferSource} bytes
* @return {!Promise<string>}
*/
export function utf8Decode(bytes) {
if (typeof TextDecoder !== 'undefined') {
return Promise.resolve(new TextDecoder('utf-8').decode(bytes));
}
return new Promise((resolve, reject) => {
const reader = new FileReader();
reader.onerror = () => {
reject(reader.error);
};
reader.onloadend = () => {
resolve(reader.result);
};
reader.readAsText(new Blob([bytes]));
});
}

// TODO(aghassemi, #6139): Remove the async version of utf8 encoding and rename
// the sync versions to the canonical utf8Decode/utf8Encode.
/**
* Interpret a byte array as a UTF-8 string.
* @param {!BufferSource} bytes
* @return {string}
*/
export function utf8DecodeSync(bytes) {
export function utf8Decode(bytes) {
if (typeof TextDecoder !== 'undefined') {
return new TextDecoder('utf-8').decode(bytes);
}
const asciiString = bytesToString(new Uint8Array(bytes.buffer || bytes));
return decodeURIComponent(escape(asciiString));
}

/**
* Turn a string into UTF-8 bytes.
* @param {string} string
* @return {!Promise<!Uint8Array>}
*/
export function utf8Encode(string) {
if (typeof TextEncoder !== 'undefined') {
return Promise.resolve(new TextEncoder('utf-8').encode(string));
}
return new Promise((resolve, reject) => {
const reader = new FileReader();
reader.onerror = () => {
reject(reader.error);
};
reader.onloadend = () => {
// Because we used readAsArrayBuffer, we know the result must be an
// ArrayBuffer.
resolve(new Uint8Array(/** @type {ArrayBuffer} */ (reader.result)));
};
reader.readAsArrayBuffer(new Blob([string]));
});
}

/**
* Turn a string into UTF-8 bytes.
* @param {string} string
* @return {!Uint8Array}
*/
export function utf8EncodeSync(string) {
export function utf8Encode(string) {
if (typeof TextEncoder !== 'undefined') {
return new TextEncoder('utf-8').encode(string);
}
Expand Down
12 changes: 6 additions & 6 deletions test/functional/utils/test-base64.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ import {
} from '../../../src/utils/base64';
import {
stringToBytes,
utf8DecodeSync,
utf8EncodeSync,
utf8Decode,
utf8Encode,
} from '../../../src/utils/bytes';

describe('base64 <> utf-8 encode/decode', () => {
Expand Down Expand Up @@ -59,11 +59,11 @@ describe('base64 <> utf-8 encode/decode', () => {
describe('base64Encode/base64Decode', () => {
testCases.forEach(testCase => {
it(testCase, () => {
const utf8Bytes = utf8EncodeSync(testCase);
const utf8Bytes = utf8Encode(testCase);
const encoded = base64EncodeFromBytes(utf8Bytes);
expect(encoded).not.to.equal(testCase);
const decodedUtf8Bytes = base64DecodeToBytes(encoded);
const decoded = utf8DecodeSync(decodedUtf8Bytes);
const decoded = utf8Decode(decodedUtf8Bytes);
expect(decoded).to.equal(testCase);
});
});
Expand All @@ -72,12 +72,12 @@ describe('base64 <> utf-8 encode/decode', () => {
describe('base64UrlEncode/base64UrlDecode', () => {
testCases.forEach(testCase => {
it(testCase, () => {
const utf8Bytes = utf8EncodeSync(testCase);
const utf8Bytes = utf8Encode(testCase);
const encoded = base64UrlEncodeFromBytes(utf8Bytes);
expect(encoded).not.to.equal(testCase);
expect(encoded).not.to.match(/[+/=]/g);
const decodedUtf8Bytes = base64UrlDecodeToBytes(encoded);
const decoded = utf8DecodeSync(decodedUtf8Bytes);
const decoded = utf8Decode(decodedUtf8Bytes);
expect(decoded).to.equal(testCase);
});
});
Expand Down
6 changes: 2 additions & 4 deletions test/functional/utils/test-bytes.js
Original file line number Diff line number Diff line change
Expand Up @@ -120,15 +120,13 @@ describe('utf8', function() {

it('should encode given string into utf-8 byte array', () => {
for (let i = 0; i < strings.length; i++) {
utf8Encode(strings[i]).then(byteArray => expect(byteArray).to.deep
.equal(new Uint8Array(bytes[i])));
expect(utf8Encode(strings[i])).to.deep.equal(new Uint8Array(bytes[i]));
}
});

it('should decode given utf-8 bytes into string', () => {
for (let i = 0; i < bytes.length; i++) {
utf8Decode(new Uint8Array(bytes[i])).then(string => expect(string).to
.equal(strings[i]));
expect(utf8Decode(new Uint8Array(bytes[i]))).to.equal(strings[i]);
}
});
});
Expand Down

0 comments on commit 0f3fe05

Please sign in to comment.