SARAMALI15792 commented on issue #10626:
URL: https://github.com/apache/gravitino/issues/10626#issuecomment-4168253810
# Investigation: Issue #10626 - Invalid metadata object type causes
authorization interceptor to return internal error
## Steps to Reproduce
When a REST API request includes an unsupported `metadataObjectType` path
parameter:
1. User sends request with invalid metadata object type (e.g.,
"INVALID_TYPE")
2. Request reaches authorization interceptor before REST handler
3. System returns 500 Internal Server Error instead of 400 Bad Request
## Current Behavior
**Error Response**: HTTP 500 - "Authorization failed due to system internal
error"
**What happens internally**:
1. `ParameterUtil.extractNameIdentifierFromParameters()` is called (line
67-96)
2. At line 86,
`MetadataObject.Type.valueOf(metadataObjectType.get().toUpperCase(Locale.ROOT))`
throws `IllegalArgumentException`
3. Exception bubbles up to
`GravitinoInterceptionService.MetadataAuthorizationMethodInterceptor.invoke()`
(line 140)
4. Caught by generic catch block at line 209
5. Returns: "Authorization failed due to system internal error" (line
218-219)
## Expected Behavior
Should return HTTP 400 Bad Request with clear message: "Invalid metadata
object type: [value]"
## Root Cause
**File**:
`server/src/main/java/org/apache/gravitino/server/web/filter/ParameterUtil.java`
**Line**: 86
```java
MetadataObject.Type type =
MetadataObject.Type.valueOf(metadataObjectType.get().toUpperCase(Locale.ROOT));
```
The `valueOf()` method throws `IllegalArgumentException` for invalid enum
values, but this exception is not caught locally. It propagates to the
authorization interceptor's generic exception handler, which treats all
exceptions as internal errors.
## Investigation Timeline
**2025-01-20**: Commit `075a5809b` - GravitinoInterceptionService introduced
(#7258)
- Established authorization interceptor pattern
- Generic exception handling returns internal error
**2025-02-15**: Commit `f2aaa9fb9` - Catalog Authorization added (#7450)
- Started using ParameterUtil for parameter extraction
**2025-03-10**: Commit `75f4bff96` - Fixed similar issue (#9271)
- Changed list operations to return 403 instead of 404 for missing metalake
- Shows pattern of improving error responses in authorization flow
**Current state**: Invalid enum parsing still returns 500 instead of 400
## Valid MetadataObject Types
From `api/src/main/java/org/apache/gravitino/MetadataObject.java` (lines
35-77):
- METALAKE
- CATALOG
- SCHEMA
- FILESET
- TABLE
- VIEW
- TOPIC
- COLUMN
- ROLE
- MODEL
- TAG
- POLICY
- JOB
- JOB_TEMPLATE
Any other value causes `IllegalArgumentException`.
## Proposed Solution
**Option 1: Catch in ParameterUtil (Recommended)**
Wrap the `valueOf()` call in try-catch at line 85-86 of `ParameterUtil.java`:
```java
try {
MetadataObject.Type type =
MetadataObject.Type.valueOf(metadataObjectType.get().toUpperCase(Locale.ROOT));
// ... rest of logic
} catch (IllegalArgumentException e) {
throw new IllegalArgumentException(
"Invalid metadata object type: " + metadataObjectType.get() +
". Valid types are: " + Arrays.toString(MetadataObject.Type.values()),
e);
}
```
**Why this is best**:
- Fails fast at the source
- Clear error message with valid options
- `IllegalArgumentException` is already handled by exception handlers
(returns 400)
- Consistent with existing error handling patterns in
`ExceptionHandlers.java`
**Option 2: Catch in GravitinoInterceptionService**
Add specific handling for parameter validation errors before line 209.
**Why Option 1 is better**:
- More maintainable (error handling near the source)
- Reusable if ParameterUtil is called from other places
- Clearer separation of concerns
## Testing Strategy
**Test 1: Reproduce bug with invalid type**
```bash
# Test with original code - expect 500
curl -X GET
"http://localhost:8090/api/metalakes/test/objects/INVALID_TYPE/catalog.schema.table/tags";
\
-H "Authorization: Bearer <token>"
# Expected current: 500 Internal Server Error
# Expected after fix: 400 Bad Request with clear message
```
**Test 2: Validate fix with valid type**
```bash
# Test with valid type - should work normally
curl -X GET
"http://localhost:8090/api/metalakes/test/objects/TABLE/catalog.schema.table/tags";
\
-H "Authorization: Bearer <token>"
# Expected: Normal authorization flow (200 or 403 depending on permissions)
```
**Test 3: Regression test**
Add unit test to verify all invalid types return 400:
- Test with completely invalid string ("INVALID")
- Test with lowercase valid type ("table" - should work after toUpperCase)
- Test with empty string
- Test with special characters
## Files to Modify
1.
**server/src/main/java/org/apache/gravitino/server/web/filter/ParameterUtil.java**
- Add try-catch around line 86
- Throw descriptive IllegalArgumentException
2. **Test file** (to be created or found):
- Add regression test for invalid metadata object types
- Verify 400 response with clear error message
## Why This Is Safe
- Only affects error handling path (invalid input)
- `IllegalArgumentException` already handled correctly by
`ExceptionHandlers.java` (returns 400)
- No changes to successful request flow
- Improves user experience by providing clear error messages
- Follows existing patterns in the codebase
## Related Issues/PRs
- #9271: Similar fix for metalake existence check (returns 403 instead of
500)
- #7258: Original GravitinoInterceptionService implementation
--
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]
