-
Notifications
You must be signed in to change notification settings - Fork 83
fix(amazonq): unescape HTML entities and backslash-escaped angle brackets #2415
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
Changes from all commits
7bdb0d5
ed27ccd
7512f36
eff9712
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,46 @@ | ||
| import * as assert from 'assert' | ||
| import { unescapeHtml } from './textFormatting' | ||
|
|
||
| describe('textFormatting', () => { | ||
| describe('unescapeHtml', () => { | ||
| it('unescapes HTML entities', () => { | ||
| assert.strictEqual(unescapeHtml('<div>'), '<div>') | ||
| assert.strictEqual(unescapeHtml('"hello"'), '"hello"') | ||
| assert.strictEqual(unescapeHtml(''world''), "'world'") | ||
| assert.strictEqual(unescapeHtml('foo & bar'), 'foo & bar') | ||
| }) | ||
|
|
||
| it('unescapes backslash-escaped angle brackets', () => { | ||
| assert.strictEqual(unescapeHtml('\\<tag\\>'), '<tag>') | ||
| assert.strictEqual(unescapeHtml('a \\< b \\> c'), 'a < b > c') | ||
| }) | ||
|
|
||
| it('handles both HTML entities and backslash escaping together', () => { | ||
| const input = '[!@#$%^&*()_+\\-=\\[\\]{}|;':",./\\<\\>?]' | ||
| const expected = '[!@#$%^&*()_+\\-=\\[\\]{}|;\':\",./<>?]' | ||
| assert.strictEqual(unescapeHtml(input), expected) | ||
| }) | ||
|
|
||
| it('handles regex patterns with escaped characters', () => { | ||
| const input = "re.search(r'[!@#$%^&*()_+\\-=\\[\\]{}|;':",./\\<\\>?]', password)" | ||
| const expected = "re.search(r'[!@#$%^&*()_+\\-=\\[\\]{}|;\':\",./<>?]', password)" | ||
| assert.strictEqual(unescapeHtml(input), expected) | ||
| }) | ||
|
|
||
| it('returns unchanged text when no escaping is present', () => { | ||
| assert.strictEqual(unescapeHtml('hello world'), 'hello world') | ||
| assert.strictEqual(unescapeHtml('no special chars'), 'no special chars') | ||
| }) | ||
|
|
||
| it('handles empty string', () => { | ||
| assert.strictEqual(unescapeHtml(''), '') | ||
| }) | ||
|
|
||
| it('handles mixed content', () => { | ||
| assert.strictEqual( | ||
| unescapeHtml('Text with <html> and \\<escaped\\> brackets'), | ||
| 'Text with <html> and <escaped> brackets' | ||
| ) | ||
| }) | ||
| }) | ||
| }) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,6 +2,7 @@ import { CommandValidation, ExplanatoryParams, InvokeOutput, requiresPathAccepta | |
| import { EmptyPathError, EmptyDiffsError, FileNotExistsError, TextNotFoundError, MultipleMatchesError } from '../errors' | ||
| import { Features } from '@aws/language-server-runtimes/server-interface/server' | ||
| import { sanitize } from '@aws/lsp-core/out/util/path' | ||
| import { unescapeHtml } from '../textFormatting' | ||
| import * as os from 'os' | ||
|
|
||
| interface BaseParams extends ExplanatoryParams { | ||
|
|
@@ -138,9 +139,13 @@ const getReplaceContent = (params: ReplaceParams, fileContent: string) => { | |
| continue | ||
| } | ||
|
|
||
| // Unescape HTML entities in oldStr since the prompt was HTML-escaped before being sent to LLM | ||
| const unescapedOldStr = unescapeHtml(diff.oldStr) | ||
| const unescapedNewStr = unescapeHtml(diff.newStr) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. are we sure this will not introduce any regression? This is a risky change, what is prompting us to make this change? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fsReplace failures because LLm is reading sanitized prompt. When users are sending code in prompt using |
||
|
|
||
| // Normalize oldStr and newStr to match fileContent's line ending style | ||
| const normalizedOldStr = diff.oldStr.split(/\r\n|\r|\n/).join(lineEnding) | ||
| const normalizedNewStr = diff.newStr.split(/\r\n|\r|\n/).join(lineEnding) | ||
| const normalizedOldStr = unescapedOldStr.split(/\r\n|\r|\n/).join(lineEnding) | ||
| const normalizedNewStr = unescapedNewStr.split(/\r\n|\r|\n/).join(lineEnding) | ||
|
|
||
| // Use string indexOf and substring for safer replacement with special characters | ||
| const startIndex = fileContent.indexOf(normalizedOldStr) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can use
"unescape-html": "^1.1.0"alsoCheck this PR: https://github.com/aws/language-servers/pull/2360/files#diff-fbaaeff7c4b028ae726c46e4eea2adec92e5fd886d26d5eee21fba8dc67ecfe8L71