bito-code-review[bot] commented on PR #42489:
URL: https://github.com/apache/superset/pull/42489#issuecomment-5228440725
<!-- Bito Reply -->
The flagged issue is correct. The current implementation of `checkForHtml`
uses `str.trimStart().startsWith('<')` to detect HTML, which incorrectly flags
prose messages that happen to contain HTML tags (like database syntax errors)
as HTML error pages. To resolve this, you should restrict the detection to
stronger indicators of an HTML document, such as a `<!DOCTYPE` declaration or a
root `<html>` tag, rather than just any opening angle bracket.
Here is a suggested fix for
`superset-frontend/packages/superset-ui-core/src/query/getClientErrorObject.ts`:
```typescript
export function checkForHtml(str: string): boolean {
// Check for document-level markup rather than just any tag.
const trimmed = str.trimStart();
const isDocument = /^<(doctype|html|head|body)/i.test(trimmed);
return !isJsonString(str) && isDocument && isProbablyHTML(str);
}
```
There are no other comments on this PR to address.
**superset-frontend/packages/superset-ui-core/src/query/getClientErrorObject.ts**
```
export function checkForHtml(str: string): boolean {
// Check for document-level markup rather than just any tag.
const trimmed = str.trimStart();
const isDocument = /^<(doctype|html|head|body)/i.test(trimmed);
return !isJsonString(str) && isDocument && isProbablyHTML(str);
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
