Skip to content

Commit

Permalink
feat(designer): No longer escapes characters in token expressions (#6131
Browse files Browse the repository at this point in the history 
)

* cherrypick changes

* small commit

* casting changes

* added tests

* remove console log

* fix tests

* remove console log
  • Loading branch information
@Eric-B-Wu
Eric-B-Wu authored Nov 16, 2024
1 parent 8283cea commit a3f6e31
Show file tree
Hide file tree
Showing 7 changed files with 174 additions and 71 deletions.
20 changes: 15 additions & 5 deletions libs/designer-ui/src/lib/editor/base/utils/keyvalueitem.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { isEmpty } from '../../../dictionary/expandeddictionary';
import type { ValueSegment } from '../../models/parameter';
import { createLiteralValueSegment, insertQutationForStringType } from './helper';
import { convertSegmentsToString } from './parsesegments';
import { isNumber, isBoolean, escapeString } from '@microsoft/logic-apps-shared';
import { escapeString } from '@microsoft/logic-apps-shared';

export interface KeyValueItem {
id: string;
Expand All @@ -24,7 +24,7 @@ export const convertKeyValueItemToSegments = (items: KeyValueItem[], keyType?: s

for (let index = 0; index < itemsToConvert.length; index++) {
const { key, value } = itemsToConvert[index];
// todo: we should have some way of handle formatting better

const convertedKeyType = convertValueType(key, keyType);
const updatedKey = key.map((segment) => {
return {
Expand All @@ -41,6 +41,7 @@ export const convertKeyValueItemToSegments = (items: KeyValueItem[], keyType?: s
};
});

// wrap key and value with quotation marks if they are string type
insertQutationForStringType(parsedItems, convertedKeyType);
parsedItems.push(...updatedKey);
insertQutationForStringType(parsedItems, convertedKeyType);
Expand All @@ -60,8 +61,17 @@ export const convertValueType = (value: ValueSegment[], type?: string): string |
return type;
}
const stringSegments = convertSegmentsToString(value).trim();
const isExpressionOrObject = (stringSegments.startsWith('@{') || stringSegments.startsWith('{')) && stringSegments.endsWith('}');
const isKnownType = isExpressionOrObject || isNumber(stringSegments) || isBoolean(stringSegments) || /^\[.*\]$/.test(stringSegments);
if (isNonString(stringSegments)) {
return type;
}
return constants.SWAGGER.TYPE.STRING;
};

return isKnownType ? type : constants.SWAGGER.TYPE.STRING;
const isNonString = (value: string): boolean => {
try {
const parsedValue = JSON.parse(value);
return parsedValue === null || Array.isArray(parsedValue) || typeof parsedValue !== 'string';
} catch {
return false;
}
};
4 changes: 2 additions & 2 deletions libs/designer/src/lib/core/utils/parameters/__test__/helper.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -411,7 +411,7 @@ describe('core/utils/parameters/helper', () => {
];

expect(parameterValueToJSONString(parameterValue, /* applyCasting */ false, /* forValidation */ true)).toBe(
'{"newUnb3_1":"@xpath(xml(triggerBody()), \'string(/*[local-name()=\\"DynamicsSOCSV\\"])\')"}'
'{\n "newUnb3_1": @{xpath(xml(triggerBody()), \'string(/*[local-name()="DynamicsSOCSV"])\')}\n}'
);
});

Expand Down Expand Up @@ -485,7 +485,7 @@ describe('core/utils/parameters/helper', () => {
];

expect(parameterValueToJSONString(parameterValue, /* applyCasting */ false, /* forValidation */ true)).toBe(
'{"newUnb3_1":"@{xpath(xml(triggerBody()), \'string(/*[local-name()=\\"DynamicsSOCSV\\"])\')}"}'
'{\n "newUnb3_1": "@{xpath(xml(triggerBody()), \'string(/*[local-name()="DynamicsSOCSV"])\')}"\n}'
);
});

Expand Down
48 changes: 23 additions & 25 deletions libs/designer/src/lib/core/utils/parameters/helper.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ import {
canStringBeConverted,
isStringLiteral,
splitAtIndex,
unescapeString,
} from '@microsoft/logic-apps-shared';
import type {
AuthProps,
Expand Down Expand Up @@ -787,8 +788,8 @@ function toAuthenticationViewModel(value: any): { type: AuthenticationType; auth
type: value.type,
authenticationValue: {
basic: {
basicUsername: loadParameterValueFromString(value.username),
basicPassword: loadParameterValueFromString(value.password),
basicUsername: loadParameterValueFromString(value.username, { parameterType: constants.SWAGGER.TYPE.STRING }),
basicPassword: loadParameterValueFromString(value.password, { parameterType: constants.SWAGGER.TYPE.STRING }),
},
},
};
Expand All @@ -797,8 +798,8 @@ function toAuthenticationViewModel(value: any): { type: AuthenticationType; auth
type: value.type,
authenticationValue: {
clientCertificate: {
clientCertificatePfx: loadParameterValueFromString(value.pfx),
clientCertificatePassword: loadParameterValueFromString(value.password),
clientCertificatePfx: loadParameterValueFromString(value.pfx, { parameterType: constants.SWAGGER.TYPE.STRING }),
clientCertificatePassword: loadParameterValueFromString(value.password, { parameterType: constants.SWAGGER.TYPE.STRING }),
},
},
};
Expand All @@ -808,14 +809,14 @@ function toAuthenticationViewModel(value: any): { type: AuthenticationType; auth
type: value.type,
authenticationValue: {
aadOAuth: {
oauthTenant: loadParameterValueFromString(value.tenant),
oauthAudience: loadParameterValueFromString(value.audience),
oauthAuthority: loadParameterValueFromString(value.authority),
oauthClientId: loadParameterValueFromString(value.clientId),
oauthTenant: loadParameterValueFromString(value.tenant, { parameterType: constants.SWAGGER.TYPE.STRING }),
oauthAudience: loadParameterValueFromString(value.audience, { parameterType: constants.SWAGGER.TYPE.STRING }),
oauthAuthority: loadParameterValueFromString(value.authority, { parameterType: constants.SWAGGER.TYPE.STRING }),
oauthClientId: loadParameterValueFromString(value.clientId, { parameterType: constants.SWAGGER.TYPE.STRING }),
oauthType: loadOauthType(value),
oauthTypeSecret: loadParameterValueFromString(value.secret),
oauthTypeCertificatePfx: loadParameterValueFromString(value.pfx),
oauthTypeCertificatePassword: loadParameterValueFromString(value.password),
oauthTypeSecret: loadParameterValueFromString(value.secret, { parameterType: constants.SWAGGER.TYPE.STRING }),
oauthTypeCertificatePfx: loadParameterValueFromString(value.pfx, { parameterType: constants.SWAGGER.TYPE.STRING }),
oauthTypeCertificatePassword: loadParameterValueFromString(value.password, { parameterType: constants.SWAGGER.TYPE.STRING }),
},
},
};
Expand All @@ -835,7 +836,7 @@ function toAuthenticationViewModel(value: any): { type: AuthenticationType; auth
type: value.type,
authenticationValue: {
msi: {
msiAudience: loadParameterValueFromString(value.audience),
msiAudience: loadParameterValueFromString(value.audience, { parameterType: constants.SWAGGER.TYPE.STRING }),
msiIdentity: value.identity,
},
},
Expand Down Expand Up @@ -954,7 +955,7 @@ export function convertToTokenExpression(value: any): string {
return value.toString();
}

export function convertToValueSegments(value: any, shouldUncast: boolean, parameterType: string): ValueSegment[] {
export function convertToValueSegments(value: any, shouldUncast: boolean, parameterType?: string): ValueSegment[] {
try {
const convertor = new ValueSegmentConvertor({
shouldUncast,
Expand Down Expand Up @@ -3590,6 +3591,7 @@ export function parameterValueToJSONString(parameterValue: ValueSegment[], apply
let tokenExpression: string = expression.value;

if (isTokenValueSegment(expression)) {
const stringifiedTokenExpression = tokenExpression;
// Note: Stringify the token expression to escape double quotes and other characters which must be escaped in JSON.
if (shouldInterpolate) {
if (applyCasting) {
Expand All @@ -3602,20 +3604,15 @@ export function parameterValueToJSONString(parameterValue: ValueSegment[], apply
);
}

const stringifiedTokenExpression = JSON.stringify(tokenExpression).slice(1, -1);
tokenExpression = `@{${stringifiedTokenExpression}}`;
} else {
// Add quotes around tokens. Tokens directly after a literal need a leading quote, and those before another literal need an ending quote.
const lastExpressionWasLiteral = i > 0 && updatedParameterValue[i - 1].type !== ValueSegmentType.TOKEN;
const nextExpressionIsLiteral =
i < updatedParameterValue.length - 1 && updatedParameterValue[i + 1].type !== ValueSegmentType.TOKEN;

const stringifiedTokenExpression = JSON.stringify(tokenExpression).slice(1, -1);
tokenExpression = `@${stringifiedTokenExpression}`;
// eslint-disable-next-line no-useless-escape
tokenExpression = lastExpressionWasLiteral ? `\"${tokenExpression}` : tokenExpression;
// eslint-disable-next-line no-useless-escape
tokenExpression = nextExpressionIsLiteral ? `${tokenExpression}\"` : `${tokenExpression}`;
tokenExpression = lastExpressionWasLiteral ? `"${tokenExpression}` : tokenExpression;
tokenExpression = nextExpressionIsLiteral ? `${tokenExpression}"` : `${tokenExpression}`;
}

parameterValueString += tokenExpression;
Expand Down Expand Up @@ -3743,13 +3740,14 @@ function parameterValueToStringWithoutCasting(value: ValueSegment[], forValidati
const shouldInterpolateTokens = (value.length > 1 || shouldInterpolateSingleToken) && value.some(isTokenValueSegment);

return value
.map((expression) => {
let expressionValue = forValidation ? expression.value || null : expression.value;
if (isTokenValueSegment(expression)) {
expressionValue = shouldInterpolateTokens ? `@{${expressionValue}}` : `@${expressionValue}`;
.map((segment) => {
const { value: segmentValue } = segment;
if (isTokenValueSegment(segment)) {
const token = forValidation ? segmentValue || null : unescapeString(segmentValue);
return shouldInterpolateTokens ? `@{${token}}` : `@${token}`;
}

return expressionValue;
return forValidation ? segmentValue || null : segmentValue;
})
.join('');
}
Expand Down
11 changes: 7 additions & 4 deletions libs/designer/src/lib/core/utils/parameters/segment.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
isNullOrUndefined,
startsWith,
UnsupportedException,
escapeString,
} from '@microsoft/logic-apps-shared';

/**
Expand Down Expand Up @@ -57,7 +58,7 @@ export class ValueSegmentConvertor {
* @arg {any} value - The value.
* @return {ValueSegment[]}
*/
public convertToValueSegments(value: any, parameterType: string = constants.SWAGGER.TYPE.ANY): ValueSegment[] {
public convertToValueSegments(value: any, parameterType?: string): ValueSegment[] {
if (isNullOrUndefined(value)) {
return [createLiteralValueSegment('')];
}
Expand Down Expand Up @@ -107,11 +108,12 @@ export class ValueSegmentConvertor {
return [this._createLiteralValueSegment(section)];
}

private _convertStringToValueSegments(value: string, parameterType: string): ValueSegment[] {
private _convertStringToValueSegments(value: string, parameterType?: string): ValueSegment[] {
if (isTemplateExpression(value)) {
const expression = ExpressionParser.parseTemplateExpression(value);
return this._convertTemplateExpressionToValueSegments(expression);
}

const isSpecialValue = ['true', 'false', 'null'].includes(value) || /^-?\d+$/.test(value);
const stringValue = parameterType === constants.SWAGGER.TYPE.ANY && isSpecialValue ? `"${value}"` : value;
return [this._createLiteralValueSegment(stringValue)];
Expand Down Expand Up @@ -186,10 +188,11 @@ export class ValueSegmentConvertor {
return dynamicContentTokenSegment;
}
// Note: We need to get the expression value if this is a sub expression resulted from uncasting.
const value =
const value = escapeString(
expression.startPosition === 0
? expression.expression
: expression.expression.substring(expression.startPosition, expression.endPosition);
: expression.expression.substring(expression.startPosition, expression.endPosition)
);
return this._createExpressionTokenValueSegment(value, expression);
}

Expand Down
14 changes: 4 additions & 10 deletions libs/designer/src/lib/core/utils/validation.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -418,16 +418,10 @@ const validateFloatingActionMenuOutputsEditor = (editorViewModel: FloatingAction
};

function shouldValidateJSON(expressions: ValueSegment[]): boolean {
const shouldValidate = true;

if (shouldValidate && expressions.length) {
const firstSegmentValue = expressions[0].token?.value;
if (firstSegmentValue) {
return startsWith(firstSegmentValue, '@@') || startsWith(firstSegmentValue, '@{') || !startsWith(firstSegmentValue, '@');
}
}

return shouldValidate;
const firstSegmentValue = expressions[0]?.token?.value;
return firstSegmentValue
? firstSegmentValue.startsWith('@@') || firstSegmentValue.startsWith('@{') || !firstSegmentValue.startsWith('@')
: true;
}

export function parameterHasOnlyTokenBinding(parameterValue: ValueSegment[]): boolean {
Expand Down
110 changes: 89 additions & 21 deletions libs/logic-apps-shared/src/utils/src/lib/helpers/__test__/stringFunctions.spec.ts
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { escapeString, idDisplayCase, labelCase, canStringBeConverted } from '../stringFunctions';
import { escapeString, idDisplayCase, labelCase, canStringBeConverted, unescapeString } from '../stringFunctions';
import { describe, it, expect } from 'vitest';
describe('label_case', () => {
it('should replace _ with spaces', () => {
Expand Down Expand Up @@ -28,26 +28,6 @@ describe('idDisplayCase', () => {
});
});

describe('escapeString', () => {
it('should correctly escape backslashes', () => {
expect(escapeString('\\')).toEqual('\\\\');
expect(escapeString('Test\\Test')).toEqual('Test\\\\Test');
});

it('should correctly escape newline characters', () => {
expect(escapeString('\n')).toEqual('\\n');
expect(escapeString('Test\nTest')).toEqual('Test\\nTest');
});

it('should correctly escape backslashes and newline characters together', () => {
expect(escapeString('\\\n')).toEqual('\\\\\\n');
expect(escapeString('Test\\\nTest')).toEqual('Test\\\\\\nTest');
});

it('should handle an empty string', () => {
expect(escapeString('')).toEqual('');
});
});
describe('canStringBeConverted', () => {
it('should return false for non-string inputs', () => {
expect(canStringBeConverted(123 as any)).toBe(false);
Expand Down Expand Up @@ -89,3 +69,91 @@ describe('canStringBeConverted', () => {
expect(canStringBeConverted('{"a", "b", "c"}')).toBe(false);
});
});

describe('unescapeString', () => {
it('unescapes newline characters', () => {
const input = 'Hello\\nWorld';
const expectedOutput = 'Hello\nWorld';
const result = unescapeString(input);
expect(result).toBe(expectedOutput);
});

it('unescapes carriage return characters', () => {
const input = 'Hello\\rWorld';
const expectedOutput = 'Hello\rWorld';
const result = unescapeString(input);
expect(result).toBe(expectedOutput);
});

it('unescapes tab characters', () => {
const input = 'Hello\\tWorld';
const expectedOutput = 'Hello\tWorld';
const result = unescapeString(input);
expect(result).toBe(expectedOutput);
});

it('unescapes vertical tab characters', () => {
const input = 'Hello\\vWorld';
const expectedOutput = 'Hello\vWorld';
const result = unescapeString(input);
expect(result).toBe(expectedOutput);
});

it('returns the same string if there are no escape sequences', () => {
const input = 'Hello World';
const expectedOutput = 'Hello World';
const result = unescapeString(input);
expect(result).toBe(expectedOutput);
});
});

describe('escapeString', () => {
it('escapes newline characters', () => {
const input = 'Hello\nWorld';
const expectedOutput = 'Hello\\nWorld';
const result = escapeString(input);
expect(result).toBe(expectedOutput);
});

it('escapes carriage return characters', () => {
const input = 'Hello\rWorld';
const expectedOutput = 'Hello\\rWorld';
const result = escapeString(input);
expect(result).toBe(expectedOutput);
});

it('escapes tab characters', () => {
const input = 'Hello\tWorld';
const expectedOutput = 'Hello\\tWorld';
const result = escapeString(input);
expect(result).toBe(expectedOutput);
});

it('escapes vertical tab characters', () => {
const input = 'Hello\vWorld';
const expectedOutput = 'Hello\\vWorld';
const result = escapeString(input);
expect(result).toBe(expectedOutput);
});

it('returns the same string if there are no special characters', () => {
const input = 'Hello World';
const expectedOutput = 'Hello World';
const result = escapeString(input);
expect(result).toBe(expectedOutput);
});

it('should correctly escape newline characters', () => {
expect(escapeString('\n')).toEqual('\\n');
expect(escapeString('Test\nTest')).toEqual('Test\\nTest');
});

it('should correctly escape backslashes and newline characters together', () => {
expect(escapeString('\\\n')).toEqual('\\\\n');
expect(escapeString('Test\\\nTest')).toEqual('Test\\\\nTest');
});

it('should handle an empty string', () => {
expect(escapeString('')).toEqual('');
});
});
Loading

0 comments on commit a3f6e31

Please sign in to comment.