korbit-ai[bot] commented on code in PR #33212:
URL: https://github.com/apache/superset/pull/33212#discussion_r2054958664
##########
superset-frontend/src/utils/getBootstrapData.ts:
##########
@@ -25,8 +26,11 @@ export default function getBootstrapData(): BootstrapData {
if (cachedBootstrapData === null) {
const appContainer = document.getElementById('app');
const dataBootstrap = appContainer?.getAttribute('data-bootstrap');
- cachedBootstrapData = dataBootstrap
- ? JSON.parse(dataBootstrap)
+ const sanitizedDataBootstrap = dataBootstrap
+ ? DOMPurify.sanitize(dataBootstrap)
+ : null;
+ cachedBootstrapData = sanitizedDataBootstrap
+ ? JSON.parse(sanitizedDataBootstrap)
: DEFAULT_BOOTSTRAP_DATA;
Review Comment:
### JSON Structure Corruption Risk in Sanitization <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
DOMPurify.sanitize() may modify the JSON string structure when sanitizing
HTML entities, potentially causing JSON.parse() to fail with invalid JSON.
###### Why this matters
If the bootstrap data contains valid JSON with HTML entities or specific
characters that DOMPurify modifies, the JSON parsing will throw an exception
and fall back to DEFAULT_BOOTSTRAP_DATA, losing the intended configuration.
###### Suggested change ∙ *Feature Preview*
Ensure the JSON structure is preserved by sanitizing after parsing:
```typescript
cachedBootstrapData = dataBootstrap
? DOMPurify.sanitize(JSON.parse(dataBootstrap), { RETURN_OBJECT: true })
: DEFAULT_BOOTSTRAP_DATA;
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/be52c573-29d2-4830-b300-81ad40cf9399/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/be52c573-29d2-4830-b300-81ad40cf9399?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/be52c573-29d2-4830-b300-81ad40cf9399?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/be52c573-29d2-4830-b300-81ad40cf9399?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/be52c573-29d2-4830-b300-81ad40cf9399)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:68aa73d1-8fc6-4a58-8ef2-082a39e9b726 -->
[](68aa73d1-8fc6-4a58-8ef2-082a39e9b726)
##########
superset-frontend/src/utils/pathUtils.ts:
##########
@@ -24,5 +24,6 @@ import { applicationRoot } from 'src/utils/getBootstrapData';
* @param path A string path to a resource
*/
export function ensureAppRoot(path: string): string {
- return `${applicationRoot()}${path.startsWith('/') ? path : `/${path}`}`;
+ const fullPath = `${applicationRoot()}${path.startsWith('/') ? path :
`/${path}`}`;
+ return encodeURI(fullPath);
Review Comment:
### Incorrect URL encoding function <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
Using encodeURI() may not properly encode all necessary characters for a URL
path, particularly characters like '#', '?', '[', and ']' which are treated as
special characters in URLs.
###### Why this matters
This could lead to malformed URLs or security vulnerabilities when paths
contain special characters that should be encoded but aren't by encodeURI().
###### Suggested change ∙ *Feature Preview*
Use encodeURIComponent() instead of encodeURI() to ensure all special
characters are properly encoded:
```typescript
return encodeURIComponent(fullPath);
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/14398817-98be-4e71-80b7-8ecec4315982/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/14398817-98be-4e71-80b7-8ecec4315982?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/14398817-98be-4e71-80b7-8ecec4315982?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/14398817-98be-4e71-80b7-8ecec4315982?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/14398817-98be-4e71-80b7-8ecec4315982)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:6e8fd61f-7f41-4503-ab25-efa49c5ad107 -->
[](6e8fd61f-7f41-4503-ab25-efa49c5ad107)
--
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]
