mirror of https://github.com/apache/superset.git
fix(Query): Parse html string error responses to avoid displaying raw HTML as error message (#29321)
This commit is contained in:
Ross Mabbett
GitHub
parent
b5a72e21f7
commit
de6a518161
|
|
@ -23,10 +23,12 @@ import {
|
|||
t,
|
||||
SupersetError,
|
||||
ErrorTypeEnum,
|
||||
isProbablyHTML,
|
||||
isJsonString,
|
||||
} from '@superset-ui/core';
|
||||
|
||||
// The response always contains an error attribute, can contain anything from the
|
||||
// SupersetClientResponse object, and can contain a spread JSON blob
|
||||
// The response always contains an error attribute, can contain anything from
|
||||
// the SupersetClientResponse object, and can contain a spread JSON blob
|
||||
export type ClientErrorObject = {
|
||||
error: string;
|
||||
errors?: SupersetError[];
|
||||
|
|
@ -51,8 +53,74 @@ type ErrorType =
|
|||
|
||||
type ErrorTextSource = 'dashboard' | 'chart' | 'query' | 'dataset' | 'database';
|
||||
|
||||
export function parseErrorJson(responseObject: JsonObject): ClientErrorObject {
|
||||
let error = { ...responseObject };
|
||||
const ERROR_CODE_LOOKUP = {
|
||||
400: 'Bad request',
|
||||
401: 'Unauthorized',
|
||||
402: 'Payment required',
|
||||
403: 'Forbidden',
|
||||
404: 'Not found',
|
||||
405: 'Method not allowed',
|
||||
406: 'Not acceptable',
|
||||
407: 'Proxy authentication required',
|
||||
408: 'Request timeout',
|
||||
409: 'Conflict',
|
||||
410: 'Gone',
|
||||
411: 'Length required',
|
||||
412: 'Precondition failed',
|
||||
413: 'Payload too large',
|
||||
414: 'URI too long',
|
||||
415: 'Unsupported media type',
|
||||
416: 'Range not satisfiable',
|
||||
417: 'Expectation failed',
|
||||
418: "I'm a teapot",
|
||||
500: 'Server error',
|
||||
501: 'Not implemented',
|
||||
502: 'Bad gateway',
|
||||
503: 'Service unavailable',
|
||||
504: 'Gateway timeout',
|
||||
505: 'HTTP version not supported',
|
||||
506: 'Variant also negotiates',
|
||||
507: 'Insufficient storage',
|
||||
508: 'Loop detected',
|
||||
510: 'Not extended',
|
||||
511: 'Network authentication required',
|
||||
599: 'Network error',
|
||||
};
|
||||
|
||||
export function checkForHtml(str: string): boolean {
|
||||
return !isJsonString(str) && isProbablyHTML(str);
|
||||
}
|
||||
|
||||
export function parseStringResponse(str: string): string {
|
||||
if (checkForHtml(str)) {
|
||||
for (const [code, message] of Object.entries(ERROR_CODE_LOOKUP)) {
|
||||
const regex = new RegExp(`${code}|${message}`, 'i');
|
||||
if (regex.test(str)) {
|
||||
return t(message);
|
||||
}
|
||||
}
|
||||
return t('Unknown error');
|
||||
}
|
||||
return str;
|
||||
}
|
||||
|
||||
export function getErrorFromStatusCode(status: number): string | null {
|
||||
return ERROR_CODE_LOOKUP[status] || null;
|
||||
}
|
||||
|
||||
export function retrieveErrorMessage(
|
||||
str: string,
|
||||
errorObject: JsonObject,
|
||||
): string {
|
||||
const statusError =
|
||||
'status' in errorObject ? getErrorFromStatusCode(errorObject.status) : null;
|
||||
|
||||
// Prefer status code message over the response or HTML text
|
||||
return statusError || parseStringResponse(str);
|
||||
}
|
||||
|
||||
export function parseErrorJson(responseJson: JsonObject): ClientErrorObject {
|
||||
let error = { ...responseJson };
|
||||
// Backwards compatibility for old error renderers with the new error object
|
||||
if (error.errors && error.errors.length > 0) {
|
||||
error.error = error.description = error.errors[0].message;
|
||||
|
|
@ -67,7 +135,11 @@ export function parseErrorJson(responseObject: JsonObject): ClientErrorObject {
|
|||
t('Invalid input');
|
||||
}
|
||||
if (typeof error.message === 'string') {
|
||||
error.error = error.message;
|
||||
if (checkForHtml(error.message)) {
|
||||
error.error = retrieveErrorMessage(error.message, error);
|
||||
} else {
|
||||
error.error = error.message;
|
||||
}
|
||||
}
|
||||
}
|
||||
if (error.stack) {
|
||||
|
|
@ -95,11 +167,12 @@ export function getClientErrorObject(
|
|||
| { response: Response }
|
||||
| string,
|
||||
): Promise<ClientErrorObject> {
|
||||
// takes a SupersetClientResponse as input, attempts to read response as Json if possible,
|
||||
// and returns a Promise that resolves to a plain object with error key and text value.
|
||||
// takes a SupersetClientResponse as input, attempts to read response as Json
|
||||
// if possible, and returns a Promise that resolves to a plain object with
|
||||
// error key and text value.
|
||||
return new Promise(resolve => {
|
||||
if (typeof response === 'string') {
|
||||
resolve({ error: response });
|
||||
resolve({ error: parseStringResponse(response) });
|
||||
return;
|
||||
}
|
||||
|
||||
|
|
@ -149,20 +222,32 @@ export function getClientErrorObject(
|
|||
|
||||
const responseObject =
|
||||
response instanceof Response ? response : response.response;
|
||||
|
||||
if (responseObject && !responseObject.bodyUsed) {
|
||||
// attempt to read the body as json, and fallback to text. we must clone the
|
||||
// response in order to fallback to .text() because Response is single-read
|
||||
// attempt to read the body as json, and fallback to text. we must clone
|
||||
// the response in order to fallback to .text() because Response is
|
||||
// single-read
|
||||
responseObject
|
||||
.clone()
|
||||
.json()
|
||||
.then(errorJson => {
|
||||
const error = { ...responseObject, ...errorJson };
|
||||
// Destructuring instead of spreading to avoid loss of sibling properties to the body
|
||||
const { url, status, statusText, redirected, type } = responseObject;
|
||||
const responseSummary = { url, status, statusText, redirected, type };
|
||||
const error = {
|
||||
...errorJson,
|
||||
...responseSummary,
|
||||
};
|
||||
resolve(parseErrorJson(error));
|
||||
})
|
||||
.catch(() => {
|
||||
// fall back to reading as text
|
||||
responseObject.text().then((errorText: any) => {
|
||||
resolve({ ...responseObject, error: errorText });
|
||||
resolve({
|
||||
// Destructuring not necessary here
|
||||
...responseObject,
|
||||
error: retrieveErrorMessage(errorText, responseObject),
|
||||
});
|
||||
});
|
||||
});
|
||||
return;
|
||||
|
|
@ -177,7 +262,7 @@ export function getClientErrorObject(
|
|||
}
|
||||
resolve({
|
||||
...responseObject,
|
||||
error,
|
||||
error: parseStringResponse(error),
|
||||
});
|
||||
});
|
||||
}
|
||||
|
|
|
|||
|
|
@ -22,6 +22,8 @@ import {
|
|||
sanitizeHtmlIfNeeded,
|
||||
safeHtmlSpan,
|
||||
removeHTMLTags,
|
||||
isJsonString,
|
||||
getParagraphContents,
|
||||
} from './html';
|
||||
|
||||
describe('sanitizeHtml', () => {
|
||||
|
|
@ -113,3 +115,77 @@ describe('removeHTMLTags', () => {
|
|||
expect(output).toBe('Unclosed tag');
|
||||
});
|
||||
});
|
||||
|
||||
describe('isJsonString', () => {
|
||||
test('valid JSON object', () => {
|
||||
const jsonString = '{"name": "John", "age": 30, "city": "New York"}';
|
||||
expect(isJsonString(jsonString)).toBe(true);
|
||||
});
|
||||
|
||||
test('valid JSON array', () => {
|
||||
const jsonString = '[1, 2, 3, 4, 5]';
|
||||
expect(isJsonString(jsonString)).toBe(true);
|
||||
});
|
||||
|
||||
test('valid JSON string', () => {
|
||||
const jsonString = '"Hello, world!"';
|
||||
expect(isJsonString(jsonString)).toBe(true);
|
||||
});
|
||||
|
||||
test('invalid JSON with syntax error', () => {
|
||||
const jsonString = '{"name": "John", "age": 30, "city": "New York"';
|
||||
expect(isJsonString(jsonString)).toBe(false);
|
||||
});
|
||||
|
||||
test('empty string', () => {
|
||||
const jsonString = '';
|
||||
expect(isJsonString(jsonString)).toBe(false);
|
||||
});
|
||||
|
||||
test('non-JSON string', () => {
|
||||
const jsonString = '<p>Hello, <strong>World!</strong></p>';
|
||||
expect(isJsonString(jsonString)).toBe(false);
|
||||
});
|
||||
|
||||
test('non-JSON formatted number', () => {
|
||||
const jsonString = '12345abc';
|
||||
expect(isJsonString(jsonString)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('getParagraphContents', () => {
|
||||
test('should return an object with keys for each paragraph tag', () => {
|
||||
const htmlString =
|
||||
'<div><p>First paragraph.</p><p>Second paragraph.</p></div>';
|
||||
const result = getParagraphContents(htmlString);
|
||||
expect(result).toEqual({
|
||||
p1: 'First paragraph.',
|
||||
p2: 'Second paragraph.',
|
||||
});
|
||||
});
|
||||
|
||||
test('should return null if the string is not HTML', () => {
|
||||
const nonHtmlString = 'Just a plain text string.';
|
||||
expect(getParagraphContents(nonHtmlString)).toBeNull();
|
||||
});
|
||||
|
||||
test('should return null if there are no <p> tags in the HTML string', () => {
|
||||
const htmlStringWithoutP = '<div><span>No paragraph here.</span></div>';
|
||||
expect(getParagraphContents(htmlStringWithoutP)).toBeNull();
|
||||
});
|
||||
|
||||
test('should return an object with empty string for empty <p> tag', () => {
|
||||
const htmlStringWithEmptyP = '<div><p></p></div>';
|
||||
const result = getParagraphContents(htmlStringWithEmptyP);
|
||||
expect(result).toEqual({ p1: '' });
|
||||
});
|
||||
|
||||
test('should handle HTML strings with nested <p> tags correctly', () => {
|
||||
const htmlStringWithNestedP =
|
||||
'<div><p>First paragraph <span>with nested</span> content.</p></div>';
|
||||
const result = getParagraphContents(htmlStringWithNestedP);
|
||||
expect(result).toEqual({
|
||||
p1: 'First paragraph with nested content.',
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -44,10 +44,26 @@ export function sanitizeHtml(htmlString: string) {
|
|||
return xssFilter.process(htmlString);
|
||||
}
|
||||
|
||||
export function hasHtmlTagPattern(str: string): boolean {
|
||||
const htmlTagPattern =
|
||||
/<(html|head|body|div|span|a|p|h[1-6]|title|meta|link|script|style)/i;
|
||||
|
||||
return htmlTagPattern.test(str);
|
||||
}
|
||||
|
||||
export function isProbablyHTML(text: string) {
|
||||
return Array.from(
|
||||
new DOMParser().parseFromString(text, 'text/html').body.childNodes,
|
||||
).some(({ nodeType }) => nodeType === 1);
|
||||
const cleanedStr = text.trim().toLowerCase();
|
||||
|
||||
if (
|
||||
cleanedStr.startsWith('<!doctype html>') &&
|
||||
hasHtmlTagPattern(cleanedStr)
|
||||
) {
|
||||
return true;
|
||||
}
|
||||
|
||||
const parser = new DOMParser();
|
||||
const doc = parser.parseFromString(cleanedStr, 'text/html');
|
||||
return Array.from(doc.body.childNodes).some(({ nodeType }) => nodeType === 1);
|
||||
}
|
||||
|
||||
export function sanitizeHtmlIfNeeded(htmlString: string) {
|
||||
|
|
@ -70,3 +86,36 @@ export function safeHtmlSpan(possiblyHtmlString: string) {
|
|||
export function removeHTMLTags(str: string): string {
|
||||
return str.replace(/<[^>]*>/g, '');
|
||||
}
|
||||
|
||||
export function isJsonString(str: string): boolean {
|
||||
try {
|
||||
JSON.parse(str);
|
||||
return true;
|
||||
} catch (e) {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
export function getParagraphContents(
|
||||
str: string,
|
||||
): { [key: string]: string } | null {
|
||||
if (!isProbablyHTML(str)) {
|
||||
return null;
|
||||
}
|
||||
|
||||
const parser = new DOMParser();
|
||||
const doc = parser.parseFromString(str, 'text/html');
|
||||
const pTags = doc.querySelectorAll('p');
|
||||
|
||||
if (pTags.length === 0) {
|
||||
return null;
|
||||
}
|
||||
|
||||
const paragraphContents: { [key: string]: string } = {};
|
||||
|
||||
pTags.forEach((pTag, index) => {
|
||||
paragraphContents[`p${index + 1}`] = pTag.textContent || '';
|
||||
});
|
||||
|
||||
return paragraphContents;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -25,19 +25,59 @@ import {
|
|||
ErrorTypeEnum,
|
||||
} from '@superset-ui/core';
|
||||
|
||||
it('Returns a Promise', () => {
|
||||
test('Returns a Promise', () => {
|
||||
const response = getClientErrorObject('error');
|
||||
expect(response instanceof Promise).toBe(true);
|
||||
});
|
||||
|
||||
it('Returns a Promise that resolves to an object with an error key', async () => {
|
||||
test('Returns a Promise that resolves to an object with an error key', async () => {
|
||||
const error = 'error';
|
||||
|
||||
const errorObj = await getClientErrorObject(error);
|
||||
expect(errorObj).toMatchObject({ error });
|
||||
});
|
||||
|
||||
it('Handles Response that can be parsed as json', async () => {
|
||||
test('should handle HTML response with "500" or "server error"', async () => {
|
||||
const htmlString500 = '<div>500: Internal Server Error</div>';
|
||||
const clientErrorObject500 = await getClientErrorObject(htmlString500);
|
||||
expect(clientErrorObject500).toEqual({ error: 'Server error' });
|
||||
|
||||
const htmlStringServerError = '<div>Server error message</div>';
|
||||
const clientErrorObjectServerError = await getClientErrorObject(
|
||||
htmlStringServerError,
|
||||
);
|
||||
expect(clientErrorObjectServerError).toEqual({
|
||||
error: 'Server error',
|
||||
});
|
||||
});
|
||||
|
||||
test('should handle HTML response with "404" or "not found"', async () => {
|
||||
const htmlString404 = '<div>404: Page not found</div>';
|
||||
const clientErrorObject404 = await getClientErrorObject(htmlString404);
|
||||
expect(clientErrorObject404).toEqual({ error: 'Not found' });
|
||||
|
||||
const htmlStringNotFoundError = '<div>Not found message</div>';
|
||||
const clientErrorObjectNotFoundError = await getClientErrorObject(
|
||||
htmlStringNotFoundError,
|
||||
);
|
||||
expect(clientErrorObjectNotFoundError).toEqual({
|
||||
error: 'Not found',
|
||||
});
|
||||
});
|
||||
|
||||
test('should handle HTML response without common error code', async () => {
|
||||
const htmlString = '<!doctype html><div>Foo bar Lorem Ipsum</div>';
|
||||
const clientErrorObject = await getClientErrorObject(htmlString);
|
||||
expect(clientErrorObject).toEqual({ error: 'Unknown error' });
|
||||
|
||||
const htmlString2 = '<div><p>An error occurred</p></div>';
|
||||
const clientErrorObject2 = await getClientErrorObject(htmlString2);
|
||||
expect(clientErrorObject2).toEqual({
|
||||
error: 'Unknown error',
|
||||
});
|
||||
});
|
||||
|
||||
test('Handles Response that can be parsed as json', async () => {
|
||||
const jsonError = { something: 'something', error: 'Error message' };
|
||||
const jsonErrorString = JSON.stringify(jsonError);
|
||||
|
||||
|
|
@ -45,7 +85,7 @@ it('Handles Response that can be parsed as json', async () => {
|
|||
expect(errorObj).toMatchObject(jsonError);
|
||||
});
|
||||
|
||||
it('Handles backwards compatibility between old error messages and the new SIP-40 errors format', async () => {
|
||||
test('Handles backwards compatibility between old error messages and the new SIP-40 errors format', async () => {
|
||||
const jsonError = {
|
||||
errors: [
|
||||
{
|
||||
|
|
@ -63,14 +103,21 @@ it('Handles backwards compatibility between old error messages and the new SIP-4
|
|||
expect(errorObj.link).toEqual(jsonError.errors[0].extra.link);
|
||||
});
|
||||
|
||||
it('Handles Response that can be parsed as text', async () => {
|
||||
test('Handles Response that can be parsed as text', async () => {
|
||||
const textError = 'Hello I am a text error';
|
||||
|
||||
const errorObj = await getClientErrorObject(new Response(textError));
|
||||
expect(errorObj).toMatchObject({ error: textError });
|
||||
});
|
||||
|
||||
it('Handles TypeError Response', async () => {
|
||||
test('Handles Response that contains raw html be parsed as text', async () => {
|
||||
const textError = 'Hello I am a text error';
|
||||
|
||||
const errorObj = await getClientErrorObject(new Response(textError));
|
||||
expect(errorObj).toMatchObject({ error: textError });
|
||||
});
|
||||
|
||||
test('Handles TypeError Response', async () => {
|
||||
const error = new TypeError('Failed to fetch');
|
||||
|
||||
// @ts-ignore
|
||||
|
|
@ -78,7 +125,7 @@ it('Handles TypeError Response', async () => {
|
|||
expect(errorObj).toMatchObject({ error: 'Network error' });
|
||||
});
|
||||
|
||||
it('Handles timeout error', async () => {
|
||||
test('Handles timeout error', async () => {
|
||||
const errorObj = await getClientErrorObject({
|
||||
timeout: 1000,
|
||||
statusText: 'timeout',
|
||||
|
|
@ -110,14 +157,30 @@ it('Handles timeout error', async () => {
|
|||
});
|
||||
});
|
||||
|
||||
it('Handles plain text as input', async () => {
|
||||
test('Handles plain text as input', async () => {
|
||||
const error = 'error';
|
||||
|
||||
const errorObj = await getClientErrorObject(error);
|
||||
expect(errorObj).toMatchObject({ error });
|
||||
});
|
||||
|
||||
it('Handles error with status text and message', async () => {
|
||||
test('Handles error with status code', async () => {
|
||||
const status500 = new Response(null, { status: 500 });
|
||||
const status404 = new Response(null, { status: 404 });
|
||||
const status502 = new Response(null, { status: 502 });
|
||||
|
||||
expect(await getClientErrorObject(status500)).toMatchObject({
|
||||
error: 'Server error',
|
||||
});
|
||||
expect(await getClientErrorObject(status404)).toMatchObject({
|
||||
error: 'Not found',
|
||||
});
|
||||
expect(await getClientErrorObject(status502)).toMatchObject({
|
||||
error: 'Bad gateway',
|
||||
});
|
||||
});
|
||||
|
||||
test('Handles error with status text and message', async () => {
|
||||
const statusText = 'status';
|
||||
const message = 'message';
|
||||
|
||||
|
|
@ -135,7 +198,7 @@ it('Handles error with status text and message', async () => {
|
|||
});
|
||||
});
|
||||
|
||||
it('getClientErrorMessage', () => {
|
||||
test('getClientErrorMessage', () => {
|
||||
expect(getClientErrorMessage('error')).toEqual('error');
|
||||
expect(
|
||||
getClientErrorMessage('error', {
|
||||
|
|
@ -150,7 +213,7 @@ it('getClientErrorMessage', () => {
|
|||
).toEqual('error:\nclient error');
|
||||
});
|
||||
|
||||
it('parseErrorJson with message', () => {
|
||||
test('parseErrorJson with message', () => {
|
||||
expect(parseErrorJson({ message: 'error message' })).toEqual({
|
||||
message: 'error message',
|
||||
error: 'error message',
|
||||
|
|
@ -181,7 +244,49 @@ it('parseErrorJson with message', () => {
|
|||
});
|
||||
});
|
||||
|
||||
it('parseErrorJson with stacktrace', () => {
|
||||
test('parseErrorJson with HTML message', () => {
|
||||
expect(
|
||||
parseErrorJson({
|
||||
message: '<div>error message</div>',
|
||||
}),
|
||||
).toEqual({
|
||||
message: '<div>error message</div>',
|
||||
error: 'Unknown error',
|
||||
});
|
||||
expect(
|
||||
parseErrorJson({
|
||||
message: '<div>Server error</div>',
|
||||
}),
|
||||
).toEqual({
|
||||
message: '<div>Server error</div>',
|
||||
error: 'Server error',
|
||||
});
|
||||
});
|
||||
|
||||
test('parseErrorJson with HTML message and status code', () => {
|
||||
expect(
|
||||
parseErrorJson({
|
||||
status: 502,
|
||||
message: '<div>error message</div>',
|
||||
}),
|
||||
).toEqual({
|
||||
status: 502,
|
||||
message: '<div>error message</div>',
|
||||
error: 'Bad gateway',
|
||||
});
|
||||
expect(
|
||||
parseErrorJson({
|
||||
status: 999,
|
||||
message: '<div>Server error</div>',
|
||||
}),
|
||||
).toEqual({
|
||||
status: 999,
|
||||
message: '<div>Server error</div>',
|
||||
error: 'Server error',
|
||||
});
|
||||
});
|
||||
|
||||
test('parseErrorJson with stacktrace', () => {
|
||||
expect(
|
||||
parseErrorJson({ error: 'error message', stack: 'stacktrace' }),
|
||||
).toEqual({
|
||||
|
|
@ -204,7 +309,7 @@ it('parseErrorJson with stacktrace', () => {
|
|||
});
|
||||
});
|
||||
|
||||
it('parseErrorJson with CSRF', () => {
|
||||
test('parseErrorJson with CSRF', () => {
|
||||
expect(
|
||||
parseErrorJson({
|
||||
responseText: 'CSRF',
|
||||
|
|
@ -215,7 +320,7 @@ it('parseErrorJson with CSRF', () => {
|
|||
});
|
||||
});
|
||||
|
||||
it('getErrorText', async () => {
|
||||
test('getErrorText', async () => {
|
||||
expect(await getErrorText('error', 'dashboard')).toEqual(
|
||||
'Sorry, there was an error saving this dashboard: error',
|
||||
);
|
||||
|
|
|
|||
Loading…
Reference in New Issue