nevzheng commented on issue #13107: URL: https://github.com/apache/gravitino/issues/13107#issuecomment-5649297083
Bounded initial analysis: error-response builders in `server/`, `server-common/`, `iceberg/iceberg-rest-server/` and `lance/lance-rest-server/` (`src/main` only). Not a full audit; `catalogs/`, `core/` and clients were out of scope. Line numbers are from the #13057 branch at bc31ef559. **Paths in this issue** | Path | Status codes | `message` source | Caller | |---|---|---|---| | `AuthenticationFilter.sendAuthErrorResponse` (`AuthenticationFilter.java:133-160`) | 401/403/400/500 | Raw `exception.getMessage()` on every branch; 500 at `:153` | Authentication step, plus failures escaping the downstream chain | | `ErrorExceptionMapper` (`ErrorExceptionMapper.java:40`) | 500 | Prefix + `Error.toString()` | Authenticated | Same filter family, for comparison: | Path | Status codes | `message` source | |---|---|---| | `IcebergAuthenticationFilter.sendAuthErrorResponse` (`:48-58`) | 401/403/400/419/500 | Raw, via `convertToIcebergException` (`IcebergExceptionMapper.java:102-121`) | | `LanceAuthenticationFilter.sendAuthErrorResponse` (`:58-80`) | 401/403/400/500 | 4xx raw with fallback; 500 generic `"Authentication failed"` (`:76-79`) | **Same pattern elsewhere (context, possibly separate scope)** | Path | `message` source | |---|---| | `ExceptionHandlers.BaseExceptionHandler` and per-entity handlers (`ExceptionHandlers.java:1152-1186`) | Authored prefix + `reason [` + `getErrorMsg(e)` (`:1190-1200`, the exception message) | | `handleTestConnectionException` (`ExceptionHandlers.java:208-237`) | Raw `e.getMessage()`, returned in a 200 body | | `ParamExceptionMapper` (`:42-56`) | Authored prefix + `cause.getMessage()` | | `IcebergRESTUtils.errorResponse` (`:428-434`) | Raw `ex.getMessage()` | | `LanceExceptionMapper` (`:68-87`, `:127-133`) | Raw `ex.getMessage()` in `error` | Observations: - Within this issue's paths, the 4xx messages on the authentication step are mostly authored (for example `"The provided credentials did not support"`), so the exposure is concentrated in the unexpected-500 branch and `ErrorExceptionMapper`. - A spot check of `KerberosAuthenticator` shows it converts its own validation failures to `UnauthorizedException` (`:146-148`); other authenticators were not checked for runtime exceptions that would reach the 500 branch. - Among the servers, only `LanceAuthenticationFilter` already substitutes a generic 500 message. Open question: should the rule for these two paths also be applied to the "elsewhere" paths, or tracked separately? -- 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]
