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

Escape quotes for decodeEscapedUnicodeCharacters #602

Merged
merged 2 commits into from
Jun 9, 2020

Conversation

Treno1
Copy link
Contributor

@Treno1 Treno1 commented Jun 8, 2020

Server sends me response as

{
    "text": "\u0022text\u0022"
}

My expectation was that decodeEscapedUnicodeCharacters will convert it to

{
    "text":"\"text\""
}

But result was
{"text":""text""}
with invalid json format errors.
Curl + jq handles that kind of situation perfectly.

So, small pr to fix that behaviour

@Huachao
Copy link
Owner

Huachao commented Jun 9, 2020

@Treno1 could you please provide the curl response output without jq? A public endpoint is better. I think a valid serialized JSON string should encode \u0022 as \\u0022.

@Treno1
Copy link
Contributor Author

Treno1 commented Jun 9, 2020

curl -XGET "https://pastebin.com/raw/bZJP0r5i"

@@ -254,7 +254,10 @@ export class HttpClient {
}

private decodeEscapedUnicodeCharacters(body: string): string {
return body.replace(/\\u([\d\w]{4})/gi, (_, g) => String.fromCharCode(parseInt(g, 16)));
return body.replace(/\\u([\d\w]{4})/gi, (_, g) => {
var char = String.fromCharCode(parseInt(g, 16));
Copy link
Owner

Choose a reason for hiding this comment

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

use const instead of var

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

return body.replace(/\\u([\d\w]{4})/gi, (_, g) => String.fromCharCode(parseInt(g, 16)));
return body.replace(/\\u([\d\w]{4})/gi, (_, g) => {
var char = String.fromCharCode(parseInt(g, 16));
return char == '\"' ? "\\\"" : char;
Copy link
Owner

Choose a reason for hiding this comment

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

use ===

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

return body.replace(/\\u([\d\w]{4})/gi, (_, g) => String.fromCharCode(parseInt(g, 16)));
return body.replace(/\\u([\d\w]{4})/gi, (_, g) => {
var char = String.fromCharCode(parseInt(g, 16));
return char == '\"' ? "\\\"" : char;
Copy link
Owner

Choose a reason for hiding this comment

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

no need to escape the double quote in a single quote, we can simply write like this

return char === '"' ? '\\"' : char;

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

@Huachao
Copy link
Owner

Huachao commented Jun 9, 2020

curl -XGET "https://pastebin.com/raw/bZJP0r5i"

Thanks, some minor suggestions.

Copy link
Owner

@Huachao Huachao left a comment

Choose a reason for hiding this comment

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

LGTM

@Huachao Huachao merged commit 61bb5f1 into Huachao:master Jun 9, 2020
@Huachao
Copy link
Owner

Huachao commented Jun 9, 2020

@Treno1 Meged, thanks

@Huachao
Copy link
Owner

Huachao commented Jun 12, 2020

@Treno1 you can verify this in the latest version 0.24.0

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.

2 participants