-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69618/#review211503
-----------------------------------------------------------
conf.getBoolean never returns NULL. See below
==============================================================================
public boolean getBoolean(String name, boolean defaultValue) {
String valueString = this.getTrimmed(name);
if (null != valueString && !valueString.isEmpty()) {
valueString = valueString.toLowerCase();
if ("true".equals(valueString)) {
return true;
} else {
return "false".equals(valueString) ? false : defaultValue;
}
} else {
return defaultValue;
}
}
==============================================================================
- Arjun Mishra
On Dec. 21, 2018, 10:16 a.m., hongtaq zhao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69618/
> -----------------------------------------------------------
>
> (Updated Dec. 21, 2018, 10:16 a.m.)
>
>
> Review request for sentry, Arjun Mishra, Anthony Young-Garner, kalyan kumar
> kalvagadda, Li Pi, Na Li, Steve Moist, and Sergio Pena.
>
>
> Bugs: SENTRY-2469
> https://issues.apache.org/jira/browse/SENTRY-2469
>
>
> Repository: sentry
>
>
> Description
> -------
>
> I have fixed issues in RoleServlet.java.
>
>
> RoleServlet at L62 needs to use either try resource or put it in the finally
> block to avoid resource leak
> if (conf.getBoolean(ServerConfig.SENTRY_WEB_ENABLE,
> ServerConfig.SENTRY_WEB_ENABLE_DEFAULT)) may return an NPE if conf.getBoolean
> returns a nullBoolean (not the primitive type)
> RoleServlet at L45, use Preconditions.checkNotNull or Objects.requreNonNull
> instead of assert
> RoleServlet at L55: String json = new Gson().toJson(roleMap);, it's better to
> reuse the Gson instance.
>
>
> Diffs
> -----
>
>
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/service/common/ServiceConstants.java
> 4508361b
>
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/RolesServlet.java
> 9ed2bf31
>
>
> Diff: https://reviews.apache.org/r/69618/diff/1/
>
>
> Testing
> -------
>
> builded and tested it.
>
>
> Thanks,
>
> hongtaq zhao
>
>
