codeant-ai-for-open-source[bot] commented on code in PR #40466:
URL: https://github.com/apache/superset/pull/40466#discussion_r3324578390
##########
superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/OAuth2ClientField.tsx:
##########
@@ -42,22 +46,40 @@ export const OAuth2ClientField = ({
changeMethods,
db,
default_value: defaultValue,
+ isPublic = true,
}: FieldPropTypes) => {
- const encryptedExtra = JSON.parse(db?.masked_encrypted_extra || '{}');
- const [oauth2ClientInfo, setOauth2ClientInfo] = useState<OAuth2ClientInfo>({
- id: encryptedExtra.oauth2_client_info?.id || '',
- secret: encryptedExtra.oauth2_client_info?.secret || '',
- authorization_request_uri:
- encryptedExtra.oauth2_client_info?.authorization_request_uri ||
- defaultValue?.authorization_request_uri ||
- '',
- token_request_uri:
- encryptedExtra.oauth2_client_info?.token_request_uri ||
- defaultValue?.token_request_uri ||
- '',
- scope:
- encryptedExtra.oauth2_client_info?.scope || defaultValue?.scope || '',
- });
+ const deriveOauth2ClientInfo = (): OAuth2ClientInfo => {
+ const encryptedExtra = JSON.parse(db?.masked_encrypted_extra || '{}');
+ return {
+ id: encryptedExtra.oauth2_client_info?.id || '',
+ secret: encryptedExtra.oauth2_client_info?.secret || '',
+ authorization_request_uri:
+ encryptedExtra.oauth2_client_info?.authorization_request_uri ||
+ defaultValue?.authorization_request_uri ||
+ '',
+ token_request_uri:
+ encryptedExtra.oauth2_client_info?.token_request_uri ||
+ defaultValue?.token_request_uri ||
+ '',
+ scope:
+ encryptedExtra.oauth2_client_info?.scope || defaultValue?.scope || '',
+ };
+ };
Review Comment:
**Suggestion:** `masked_encrypted_extra` is parsed and then dereferenced
without validating the parsed shape. If the backend returns malformed JSON
(throws) or the valid JSON literal `null`, this component crashes during render
(`Cannot read properties of null`). Parse defensively and default to an empty
object when parsing fails or when the parsed value is not a plain object. [null
pointer]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Editing OAuth2-enabled database can crash connection modal.
- ⚠️ Prevents updating Google Sheets OAuth2 client settings.
- ⚠️ Unhandled JSON parse errors degrade admin UX.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open the database connection modal, which renders `DatabaseModal` from
`superset-frontend/src/features/databases/DatabaseModal/index.tsx:70-82`;
this uses
`DatabaseConnectionForm` (import at line 12) to render the dynamic form for
a database.
2. In `DatabaseConnectionForm`
(`superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/index.tsx:87-137`),
note that fields are rendered from `FORM_FIELD_MAP`, and
`FORM_FIELD_MAP.oauth2_client_info = OAuth2ClientField` in
`constants.ts:72-80`, so any
database engine whose parameters schema exposes `oauth2_client_info` will
render
`OAuth2ClientField`.
3. Construct or load a `DatabaseObject` (type defined in
`superset-frontend/src/features/databases/types.ts:67-124`) whose
`masked_encrypted_extra`
property is the JSON literal string `"null"` (for example, by adding a new
Jest test in
`OAuth2ClientField.test.tsx` that sets
`defaultProps.db.masked_encrypted_extra = 'null'`
before calling `render(<OAuth2ClientField {...defaultProps} />)` as in lines
74-79).
4. When React renders `OAuth2ClientField` (`OAuth2ClientField.tsx:45-50`),
the state
initializer `useState(deriveOauth2ClientInfo)` invokes
`deriveOauth2ClientInfo` (line 51),
which executes `JSON.parse(db?.masked_encrypted_extra || '{}')` (line 52);
for `"null"`
this returns `null`, and the following line
`encryptedExtra.oauth2_client_info?.id` (line
54) attempts to access a property on `null`, causing a runtime error
(`Cannot read
properties of null (reading 'oauth2_client_info')`) and crashing the form
render.
```
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=eb0d8b702f7849e4bc1cf00a4cc5c604&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=eb0d8b702f7849e4bc1cf00a4cc5c604&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/src/features/databases/DatabaseModal/DatabaseConnectionForm/OAuth2ClientField.tsx
**Line:** 51:67
**Comment:**
*Null Pointer: `masked_encrypted_extra` is parsed and then dereferenced
without validating the parsed shape. If the backend returns malformed JSON
(throws) or the valid JSON literal `null`, this component crashes during render
(`Cannot read properties of null`). Parse defensively and default to an empty
object when parsing fails or when the parsed value is not a plain object.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40466&comment_hash=f8fb332d848c2383eb767df4ec0a9611fe087a2971472627446908d1768d28f7&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40466&comment_hash=f8fb332d848c2383eb767df4ec0a9611fe087a2971472627446908d1768d28f7&reaction=dislike'>👎</a>
--
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]