You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ranger.apache.org by Pradeep Agrawal <pr...@freestoneinfotech.com> on 2016/07/27 17:26:37 UTC
Review Request 50511: RANGER-1124:Code changes to avoid potential
NPEs and
good coding practices in security-admin, kms, credentialbuilder, jisql,
agent-installer, emeddedwebserver
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50511/
-----------------------------------------------------------
Review request for ranger.
Bugs: RANGER-1124
https://issues.apache.org/jira/browse/RANGER-1124
Repository: ranger
Description
-------
Code changes to guard against potential NPEs and other potential run-time issues; good coding practices.
Diffs
-----
credentialbuilder/src/main/java/org/apache/ranger/credentialapi/CredentialReader.java ecede34
credentialbuilder/src/main/java/org/apache/ranger/credentialapi/buildks.java d8ffe2c
embeddedwebserver/src/main/java/org/apache/ranger/server/tomcat/EmbeddedServer.java a74f8d1
embeddedwebserver/src/main/java/org/apache/ranger/server/tomcat/StopEmbeddedServer.java ef80f43
jisql/src/main/java/org/apache/util/sql/Jisql.java 36c4340
jisql/src/main/java/org/apache/util/sql/MySQLPLRunner.java b2eda30
kms/src/main/java/org/apache/hadoop/crypto/key/RangerHSM.java 606706b
kms/src/main/java/org/apache/hadoop/crypto/key/RangerKeyStore.java abfab25
kms/src/main/java/org/apache/hadoop/crypto/key/RangerMasterKey.java d70ec4e
kms/src/main/java/org/apache/hadoop/crypto/key/kms/server/KMSJSONWriter.java 3674e7a
kms/src/main/java/org/apache/ranger/entity/XXDBBase.java a0d0120
plugin-kms/src/main/java/org/apache/ranger/services/kms/client/KMSClient.java 5337ee3
plugin-kms/src/main/java/org/apache/ranger/services/kms/client/KMSResourceMgr.java e61d0bc
security-admin/src/main/java/org/apache/ranger/biz/AssetMgr.java 5ecae8d
security-admin/src/main/java/org/apache/ranger/biz/KmsKeyMgr.java 693e959
security-admin/src/main/java/org/apache/ranger/biz/RangerBizUtil.java e0a9840
security-admin/src/main/java/org/apache/ranger/biz/RangerPolicyRetriever.java 3ba33d4
security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java 7947d4b
security-admin/src/main/java/org/apache/ranger/biz/ServiceMgr.java 0059884
security-admin/src/main/java/org/apache/ranger/biz/SessionMgr.java 2e9d6d5
security-admin/src/main/java/org/apache/ranger/biz/UserMgr.java a508926
security-admin/src/main/java/org/apache/ranger/biz/XUserMgr.java 86f9d96
security-admin/src/main/java/org/apache/ranger/common/ErrorMessageUtil.java 582580c
security-admin/src/main/java/org/apache/ranger/common/RangerProperties.java 72fde46
security-admin/src/main/java/org/apache/ranger/common/RangerServicePoliciesCache.java a68b215
security-admin/src/main/java/org/apache/ranger/common/RangerServiceTagsCache.java 66c1562
security-admin/src/main/java/org/apache/ranger/common/SearchField.java 1891edb
security-admin/src/main/java/org/apache/ranger/common/ServiceUtil.java c1baae8
security-admin/src/main/java/org/apache/ranger/common/db/BaseDao.java 13607d3
security-admin/src/main/java/org/apache/ranger/credentialapi/CredentialReader.java 429be27
security-admin/src/main/java/org/apache/ranger/db/XXPolicyDao.java e25540b
security-admin/src/main/java/org/apache/ranger/db/XXServiceVersionInfoDao.java 5291045
security-admin/src/main/java/org/apache/ranger/patch/PatchForServiceVersionInfo_J10004.java 380cca0
security-admin/src/main/java/org/apache/ranger/patch/PatchMigration_J10002.java 54148f1
security-admin/src/main/java/org/apache/ranger/patch/PatchPersmissionModel_J10003.java 3a3bed2
security-admin/src/main/java/org/apache/ranger/rest/AssetREST.java 3d649df
security-admin/src/main/java/org/apache/ranger/rest/PublicAPIs.java 1f465d5
security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java 1185e45
security-admin/src/main/java/org/apache/ranger/rest/ServiceRESTUtil.java e84a1aa
security-admin/src/main/java/org/apache/ranger/rest/TagREST.java 3dfb250
security-admin/src/main/java/org/apache/ranger/rest/XKeyREST.java b07bf9c
security-admin/src/main/java/org/apache/ranger/rest/XUserREST.java 226d3c7
security-admin/src/main/java/org/apache/ranger/security/handler/RangerAuthenticationProvider.java 3fa3436
security-admin/src/main/java/org/apache/ranger/security/web/authentication/RangerAuthenticationEntryPoint.java 6496698
security-admin/src/main/java/org/apache/ranger/security/web/filter/RangerSSOAuthenticationFilter.java d431bc1
security-admin/src/main/java/org/apache/ranger/security/web/filter/RangerSecurityContextFormationFilter.java 7314782
security-admin/src/main/java/org/apache/ranger/security/web/filter/SSOAuthentication.java 6fcadb7
security-admin/src/main/java/org/apache/ranger/service/AbstractBaseResourceService.java 6c7e1f0
security-admin/src/main/java/org/apache/ranger/service/RangerDataHistService.java 65ad5ad
security-admin/src/main/java/org/apache/ranger/service/RangerPolicyService.java 4b792de
security-admin/src/main/java/org/apache/ranger/service/RangerPolicyServiceBase.java 2649ff3
security-admin/src/main/java/org/apache/ranger/service/XResourceService.java 839bf59
security-admin/src/main/java/org/apache/ranger/service/XTrxLogService.java f28ccca
security-admin/src/main/java/org/apache/ranger/solr/SolrMgr.java 1b5793f
security-admin/src/main/java/org/apache/ranger/solr/SolrUtil.java e912cfb
security-admin/src/main/java/org/apache/ranger/view/VXAuditRecordList.java 42ff4d1
security-admin/src/test/java/org/apache/ranger/audit/TestAuditQueue.java 637e43f
Diff: https://reviews.apache.org/r/50511/diff/
Testing
-------
Built clean and ran unit tests
installed ranger and ranger-kms.
Tested create/update/delete operation on service, policy, users and groups.
Thanks,
Pradeep Agrawal
Re: Review Request 50511: RANGER-1124:Code changes to avoid potential
NPEs
and good coding practices in security-admin, kms, credentialbuilder, jisql,
agent-installer, emeddedwebserver
Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50511/#review143827
-----------------------------------------------------------
credentialbuilder/src/main/java/org/apache/ranger/credentialapi/CredentialReader.java (line 60)
<https://reviews.apache.org/r/50511/#comment209759>
Consider moving this within the 'for' loop below, to line #66.
security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java (line 883)
<https://reviews.apache.org/r/50511/#comment209826>
Why is it necessary to add "CollectionUtils.isNotEmpty(xxEnums)" here?
security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java (line 948)
<https://reviews.apache.org/r/50511/#comment209823>
Consider keeping this at line #950.
security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java (line 1953)
<https://reviews.apache.org/r/50511/#comment209821>
Consider moving retTemp to line #1963
security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java (line 2039)
<https://reviews.apache.org/r/50511/#comment209819>
catch exception in .close() and ignore.
security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java (line 2043)
<https://reviews.apache.org/r/50511/#comment209820>
catch exception in .flush and .close() and ignore.
security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java (line 3288)
<https://reviews.apache.org/r/50511/#comment209818>
policyItem0, policyItem1, policyItem2 - these variables should not be needed. Please review and remove these - it should make it easier to read the code.
security-admin/src/main/java/org/apache/ranger/biz/UserMgr.java (line 746)
<https://reviews.apache.org/r/50511/#comment209768>
Consider replacing this with: "if (resultSize == 0)"; it looks simper to read.
security-admin/src/main/java/org/apache/ranger/common/RangerProperties.java (line 42)
<https://reviews.apache.org/r/50511/#comment209777>
Please add "static" here.
security-admin/src/main/java/org/apache/ranger/credentialapi/CredentialReader.java (line 60)
<https://reviews.apache.org/r/50511/#comment209782>
Consider moving this to line #65, within "for" loop.
security-admin/src/main/java/org/apache/ranger/db/XXPolicyDao.java (line 126)
<https://reviews.apache.org/r/50511/#comment209785>
"userId!=null &&" - this should not be needed. Please review.
security-admin/src/main/java/org/apache/ranger/db/XXPolicyDao.java (line 138)
<https://reviews.apache.org/r/50511/#comment209786>
"groupId!=null &&" - this should not be needed. Please review.
security-admin/src/main/java/org/apache/ranger/rest/AssetREST.java (line 505)
<https://reviews.apache.org/r/50511/#comment209807>
Consider moving this to line #508.
security-admin/src/main/java/org/apache/ranger/rest/PublicAPIs.java (line 354)
<https://reviews.apache.org/r/50511/#comment209810>
Consider moving this to line #359.
security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java (line 1501)
<https://reviews.apache.org/r/50511/#comment209816>
changes in #1501 and #1502 should not be needed. Please review.
security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java (line 1507)
<https://reviews.apache.org/r/50511/#comment209813>
This fix does not look right; it changes the logic. Please review.
security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java
<https://reviews.apache.org/r/50511/#comment209817>
This fix changes the flow/logic and does not look right. Please review.
security-admin/src/main/java/org/apache/ranger/service/XTrxLogService.java (line 95)
<https://reviews.apache.org/r/50511/#comment209765>
Consider moving this to line #107.
security-admin/src/main/java/org/apache/ranger/service/XTrxLogService.java (line 103)
<https://reviews.apache.org/r/50511/#comment209764>
Consider moving this to line #106
security-admin/src/main/java/org/apache/ranger/service/XTrxLogService.java (line 321)
<https://reviews.apache.org/r/50511/#comment209766>
Consider moving these inside following 'for'.
- Madhan Neethiraj
On July 27, 2016, 5:26 p.m., Pradeep Agrawal wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50511/
> -----------------------------------------------------------
>
> (Updated July 27, 2016, 5:26 p.m.)
>
>
> Review request for ranger.
>
>
> Bugs: RANGER-1124
> https://issues.apache.org/jira/browse/RANGER-1124
>
>
> Repository: ranger
>
>
> Description
> -------
>
> Code changes to guard against potential NPEs and other potential run-time issues; good coding practices.
>
>
> Diffs
> -----
>
> credentialbuilder/src/main/java/org/apache/ranger/credentialapi/CredentialReader.java ecede34
> credentialbuilder/src/main/java/org/apache/ranger/credentialapi/buildks.java d8ffe2c
> embeddedwebserver/src/main/java/org/apache/ranger/server/tomcat/EmbeddedServer.java a74f8d1
> embeddedwebserver/src/main/java/org/apache/ranger/server/tomcat/StopEmbeddedServer.java ef80f43
> jisql/src/main/java/org/apache/util/sql/Jisql.java 36c4340
> jisql/src/main/java/org/apache/util/sql/MySQLPLRunner.java b2eda30
> kms/src/main/java/org/apache/hadoop/crypto/key/RangerHSM.java 606706b
> kms/src/main/java/org/apache/hadoop/crypto/key/RangerKeyStore.java abfab25
> kms/src/main/java/org/apache/hadoop/crypto/key/RangerMasterKey.java d70ec4e
> kms/src/main/java/org/apache/hadoop/crypto/key/kms/server/KMSJSONWriter.java 3674e7a
> kms/src/main/java/org/apache/ranger/entity/XXDBBase.java a0d0120
> plugin-kms/src/main/java/org/apache/ranger/services/kms/client/KMSClient.java 5337ee3
> plugin-kms/src/main/java/org/apache/ranger/services/kms/client/KMSResourceMgr.java e61d0bc
> security-admin/src/main/java/org/apache/ranger/biz/AssetMgr.java 5ecae8d
> security-admin/src/main/java/org/apache/ranger/biz/KmsKeyMgr.java 693e959
> security-admin/src/main/java/org/apache/ranger/biz/RangerBizUtil.java e0a9840
> security-admin/src/main/java/org/apache/ranger/biz/RangerPolicyRetriever.java 3ba33d4
> security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java 7947d4b
> security-admin/src/main/java/org/apache/ranger/biz/ServiceMgr.java 0059884
> security-admin/src/main/java/org/apache/ranger/biz/SessionMgr.java 2e9d6d5
> security-admin/src/main/java/org/apache/ranger/biz/UserMgr.java a508926
> security-admin/src/main/java/org/apache/ranger/biz/XUserMgr.java 86f9d96
> security-admin/src/main/java/org/apache/ranger/common/ErrorMessageUtil.java 582580c
> security-admin/src/main/java/org/apache/ranger/common/RangerProperties.java 72fde46
> security-admin/src/main/java/org/apache/ranger/common/RangerServicePoliciesCache.java a68b215
> security-admin/src/main/java/org/apache/ranger/common/RangerServiceTagsCache.java 66c1562
> security-admin/src/main/java/org/apache/ranger/common/SearchField.java 1891edb
> security-admin/src/main/java/org/apache/ranger/common/ServiceUtil.java c1baae8
> security-admin/src/main/java/org/apache/ranger/common/db/BaseDao.java 13607d3
> security-admin/src/main/java/org/apache/ranger/credentialapi/CredentialReader.java 429be27
> security-admin/src/main/java/org/apache/ranger/db/XXPolicyDao.java e25540b
> security-admin/src/main/java/org/apache/ranger/db/XXServiceVersionInfoDao.java 5291045
> security-admin/src/main/java/org/apache/ranger/patch/PatchForServiceVersionInfo_J10004.java 380cca0
> security-admin/src/main/java/org/apache/ranger/patch/PatchMigration_J10002.java 54148f1
> security-admin/src/main/java/org/apache/ranger/patch/PatchPersmissionModel_J10003.java 3a3bed2
> security-admin/src/main/java/org/apache/ranger/rest/AssetREST.java 3d649df
> security-admin/src/main/java/org/apache/ranger/rest/PublicAPIs.java 1f465d5
> security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java 1185e45
> security-admin/src/main/java/org/apache/ranger/rest/ServiceRESTUtil.java e84a1aa
> security-admin/src/main/java/org/apache/ranger/rest/TagREST.java 3dfb250
> security-admin/src/main/java/org/apache/ranger/rest/XKeyREST.java b07bf9c
> security-admin/src/main/java/org/apache/ranger/rest/XUserREST.java 226d3c7
> security-admin/src/main/java/org/apache/ranger/security/handler/RangerAuthenticationProvider.java 3fa3436
> security-admin/src/main/java/org/apache/ranger/security/web/authentication/RangerAuthenticationEntryPoint.java 6496698
> security-admin/src/main/java/org/apache/ranger/security/web/filter/RangerSSOAuthenticationFilter.java d431bc1
> security-admin/src/main/java/org/apache/ranger/security/web/filter/RangerSecurityContextFormationFilter.java 7314782
> security-admin/src/main/java/org/apache/ranger/security/web/filter/SSOAuthentication.java 6fcadb7
> security-admin/src/main/java/org/apache/ranger/service/AbstractBaseResourceService.java 6c7e1f0
> security-admin/src/main/java/org/apache/ranger/service/RangerDataHistService.java 65ad5ad
> security-admin/src/main/java/org/apache/ranger/service/RangerPolicyService.java 4b792de
> security-admin/src/main/java/org/apache/ranger/service/RangerPolicyServiceBase.java 2649ff3
> security-admin/src/main/java/org/apache/ranger/service/XResourceService.java 839bf59
> security-admin/src/main/java/org/apache/ranger/service/XTrxLogService.java f28ccca
> security-admin/src/main/java/org/apache/ranger/solr/SolrMgr.java 1b5793f
> security-admin/src/main/java/org/apache/ranger/solr/SolrUtil.java e912cfb
> security-admin/src/main/java/org/apache/ranger/view/VXAuditRecordList.java 42ff4d1
> security-admin/src/test/java/org/apache/ranger/audit/TestAuditQueue.java 637e43f
>
> Diff: https://reviews.apache.org/r/50511/diff/
>
>
> Testing
> -------
>
> Built clean and ran unit tests
> installed ranger and ranger-kms.
> Tested create/update/delete operation on service, policy, users and groups.
>
>
> Thanks,
>
> Pradeep Agrawal
>
>
Re: Review Request 50511: RANGER-1124:Code changes to avoid potential
NPEs
and good coding practices in security-admin, kms, credentialbuilder, jisql,
agent-installer, emeddedwebserver
Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50511/#review144066
-----------------------------------------------------------
Ship it!
Ship It!
- Madhan Neethiraj
On July 29, 2016, 2:15 a.m., Pradeep Agrawal wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50511/
> -----------------------------------------------------------
>
> (Updated July 29, 2016, 2:15 a.m.)
>
>
> Review request for ranger.
>
>
> Bugs: RANGER-1124
> https://issues.apache.org/jira/browse/RANGER-1124
>
>
> Repository: ranger
>
>
> Description
> -------
>
> Code changes to guard against potential NPEs and other potential run-time issues; good coding practices.
>
>
> Diffs
> -----
>
> kms/src/main/java/org/apache/hadoop/crypto/key/RangerKeyStore.java abfab25
> plugin-kms/src/main/java/org/apache/ranger/services/kms/client/KMSResourceMgr.java e61d0bc
> security-admin/src/main/java/org/apache/ranger/biz/KmsKeyMgr.java 693e959
> security-admin/src/main/java/org/apache/ranger/biz/RangerBizUtil.java e0a9840
> security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java 03fe840
> security-admin/src/main/java/org/apache/ranger/biz/ServiceMgr.java 0059884
> security-admin/src/main/java/org/apache/ranger/biz/SessionMgr.java 2e9d6d5
> security-admin/src/main/java/org/apache/ranger/biz/UserMgr.java a508926
> security-admin/src/main/java/org/apache/ranger/biz/XUserMgr.java 86f9d96
> security-admin/src/main/java/org/apache/ranger/common/SearchField.java 1891edb
> security-admin/src/main/java/org/apache/ranger/common/ServiceUtil.java c1baae8
> security-admin/src/main/java/org/apache/ranger/common/db/BaseDao.java 13607d3
> security-admin/src/main/java/org/apache/ranger/db/XXPolicyDao.java e25540b
> security-admin/src/main/java/org/apache/ranger/db/XXServiceVersionInfoDao.java 5291045
> security-admin/src/main/java/org/apache/ranger/entity/XXContextEnricherDef.java e035e58
> security-admin/src/main/java/org/apache/ranger/entity/XXPolicyBase.java 8564d43
> security-admin/src/main/java/org/apache/ranger/entity/XXPolicyConditionDef.java d738841
> security-admin/src/main/java/org/apache/ranger/entity/XXPolicyItemUserPerm.java 874ca20
> security-admin/src/main/java/org/apache/ranger/rest/AssetREST.java 3d649df
> security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java 1185e45
> security-admin/src/main/java/org/apache/ranger/rest/TagREST.java 3dfb250
> security-admin/src/main/java/org/apache/ranger/security/handler/RangerAuthenticationProvider.java 3fa3436
> security-admin/src/main/java/org/apache/ranger/security/web/authentication/RangerAuthenticationEntryPoint.java 6496698
> security-admin/src/main/java/org/apache/ranger/security/web/filter/RangerSSOAuthenticationFilter.java d431bc1
> security-admin/src/main/java/org/apache/ranger/security/web/filter/RangerSecurityContextFormationFilter.java 7314782
> security-admin/src/main/java/org/apache/ranger/service/RangerPolicyService.java 4b792de
> security-admin/src/main/java/org/apache/ranger/solr/SolrMgr.java 1b5793f
>
> Diff: https://reviews.apache.org/r/50511/diff/
>
>
> Testing
> -------
>
> Built clean and ran unit tests
> installed ranger and ranger-kms.
> Tested create/update/delete operation on service, policy, users and groups.
>
>
> Thanks,
>
> Pradeep Agrawal
>
>
Re: Review Request 50511: RANGER-1124:Code changes to avoid potential
NPEs
and good coding practices in security-admin, kms, credentialbuilder, jisql,
agent-installer, emeddedwebserver
Posted by Pradeep Agrawal <pr...@freestoneinfotech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50511/
-----------------------------------------------------------
(Updated July 29, 2016, 2:15 a.m.)
Review request for ranger.
Changes
-------
Addressed review comments.
Bugs: RANGER-1124
https://issues.apache.org/jira/browse/RANGER-1124
Repository: ranger
Description
-------
Code changes to guard against potential NPEs and other potential run-time issues; good coding practices.
Diffs (updated)
-----
kms/src/main/java/org/apache/hadoop/crypto/key/RangerKeyStore.java abfab25
plugin-kms/src/main/java/org/apache/ranger/services/kms/client/KMSResourceMgr.java e61d0bc
security-admin/src/main/java/org/apache/ranger/biz/KmsKeyMgr.java 693e959
security-admin/src/main/java/org/apache/ranger/biz/RangerBizUtil.java e0a9840
security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java 03fe840
security-admin/src/main/java/org/apache/ranger/biz/ServiceMgr.java 0059884
security-admin/src/main/java/org/apache/ranger/biz/SessionMgr.java 2e9d6d5
security-admin/src/main/java/org/apache/ranger/biz/UserMgr.java a508926
security-admin/src/main/java/org/apache/ranger/biz/XUserMgr.java 86f9d96
security-admin/src/main/java/org/apache/ranger/common/SearchField.java 1891edb
security-admin/src/main/java/org/apache/ranger/common/ServiceUtil.java c1baae8
security-admin/src/main/java/org/apache/ranger/common/db/BaseDao.java 13607d3
security-admin/src/main/java/org/apache/ranger/db/XXPolicyDao.java e25540b
security-admin/src/main/java/org/apache/ranger/db/XXServiceVersionInfoDao.java 5291045
security-admin/src/main/java/org/apache/ranger/entity/XXContextEnricherDef.java e035e58
security-admin/src/main/java/org/apache/ranger/entity/XXPolicyBase.java 8564d43
security-admin/src/main/java/org/apache/ranger/entity/XXPolicyConditionDef.java d738841
security-admin/src/main/java/org/apache/ranger/entity/XXPolicyItemUserPerm.java 874ca20
security-admin/src/main/java/org/apache/ranger/rest/AssetREST.java 3d649df
security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java 1185e45
security-admin/src/main/java/org/apache/ranger/rest/TagREST.java 3dfb250
security-admin/src/main/java/org/apache/ranger/security/handler/RangerAuthenticationProvider.java 3fa3436
security-admin/src/main/java/org/apache/ranger/security/web/authentication/RangerAuthenticationEntryPoint.java 6496698
security-admin/src/main/java/org/apache/ranger/security/web/filter/RangerSSOAuthenticationFilter.java d431bc1
security-admin/src/main/java/org/apache/ranger/security/web/filter/RangerSecurityContextFormationFilter.java 7314782
security-admin/src/main/java/org/apache/ranger/service/RangerPolicyService.java 4b792de
security-admin/src/main/java/org/apache/ranger/solr/SolrMgr.java 1b5793f
Diff: https://reviews.apache.org/r/50511/diff/
Testing
-------
Built clean and ran unit tests
installed ranger and ranger-kms.
Tested create/update/delete operation on service, policy, users and groups.
Thanks,
Pradeep Agrawal
Re: Review Request 50511: RANGER-1124:Code changes to avoid potential
NPEs
and good coding practices in security-admin, kms, credentialbuilder, jisql,
agent-installer, emeddedwebserver
Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50511/#review143941
-----------------------------------------------------------
Fix it, then Ship it!
security-admin/src/main/java/org/apache/ranger/security/web/filter/RangerSSOAuthenticationFilter.java (line 123)
<https://reviews.apache.org/r/50511/#comment209885>
synchronize(this) - may not be the best option.
Consider using "synchronize(httpRequest.getServletContext())"; perhaps not just the call to removeAttribute(), instead the entire block that access httpRequest.getServletContext() - from line #120 to #127
- Madhan Neethiraj
On July 28, 2016, 9:56 a.m., Pradeep Agrawal wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50511/
> -----------------------------------------------------------
>
> (Updated July 28, 2016, 9:56 a.m.)
>
>
> Review request for ranger.
>
>
> Bugs: RANGER-1124
> https://issues.apache.org/jira/browse/RANGER-1124
>
>
> Repository: ranger
>
>
> Description
> -------
>
> Code changes to guard against potential NPEs and other potential run-time issues; good coding practices.
>
>
> Diffs
> -----
>
> kms/src/main/java/org/apache/hadoop/crypto/key/RangerKeyStore.java abfab25
> plugin-kms/src/main/java/org/apache/ranger/services/kms/client/KMSResourceMgr.java e61d0bc
> security-admin/src/main/java/org/apache/ranger/biz/KmsKeyMgr.java 693e959
> security-admin/src/main/java/org/apache/ranger/biz/RangerBizUtil.java e0a9840
> security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java 7947d4b
> security-admin/src/main/java/org/apache/ranger/biz/ServiceMgr.java 0059884
> security-admin/src/main/java/org/apache/ranger/biz/SessionMgr.java 2e9d6d5
> security-admin/src/main/java/org/apache/ranger/biz/UserMgr.java a508926
> security-admin/src/main/java/org/apache/ranger/biz/XUserMgr.java 86f9d96
> security-admin/src/main/java/org/apache/ranger/common/SearchField.java 1891edb
> security-admin/src/main/java/org/apache/ranger/common/ServiceUtil.java c1baae8
> security-admin/src/main/java/org/apache/ranger/common/db/BaseDao.java 13607d3
> security-admin/src/main/java/org/apache/ranger/db/XXPolicyDao.java e25540b
> security-admin/src/main/java/org/apache/ranger/db/XXServiceVersionInfoDao.java 5291045
> security-admin/src/main/java/org/apache/ranger/entity/XXContextEnricherDef.java e035e58
> security-admin/src/main/java/org/apache/ranger/entity/XXPolicyBase.java 8564d43
> security-admin/src/main/java/org/apache/ranger/entity/XXPolicyConditionDef.java d738841
> security-admin/src/main/java/org/apache/ranger/entity/XXPolicyItemUserPerm.java 874ca20
> security-admin/src/main/java/org/apache/ranger/rest/AssetREST.java 3d649df
> security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java 1185e45
> security-admin/src/main/java/org/apache/ranger/rest/TagREST.java 3dfb250
> security-admin/src/main/java/org/apache/ranger/security/handler/RangerAuthenticationProvider.java 3fa3436
> security-admin/src/main/java/org/apache/ranger/security/web/authentication/RangerAuthenticationEntryPoint.java 6496698
> security-admin/src/main/java/org/apache/ranger/security/web/filter/RangerSSOAuthenticationFilter.java d431bc1
> security-admin/src/main/java/org/apache/ranger/security/web/filter/RangerSecurityContextFormationFilter.java 7314782
> security-admin/src/main/java/org/apache/ranger/service/RangerPolicyService.java 4b792de
> security-admin/src/main/java/org/apache/ranger/solr/SolrMgr.java 1b5793f
>
> Diff: https://reviews.apache.org/r/50511/diff/
>
>
> Testing
> -------
>
> Built clean and ran unit tests
> installed ranger and ranger-kms.
> Tested create/update/delete operation on service, policy, users and groups.
>
>
> Thanks,
>
> Pradeep Agrawal
>
>
Re: Review Request 50511: RANGER-1124:Code changes to avoid potential
NPEs
and good coding practices in security-admin, kms, credentialbuilder, jisql,
agent-installer, emeddedwebserver
Posted by Pradeep Agrawal <pr...@freestoneinfotech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50511/
-----------------------------------------------------------
(Updated July 28, 2016, 9:56 a.m.)
Review request for ranger.
Changes
-------
Addressed review comments and updated patch based on severity of issues.
Bugs: RANGER-1124
https://issues.apache.org/jira/browse/RANGER-1124
Repository: ranger
Description
-------
Code changes to guard against potential NPEs and other potential run-time issues; good coding practices.
Diffs (updated)
-----
kms/src/main/java/org/apache/hadoop/crypto/key/RangerKeyStore.java abfab25
plugin-kms/src/main/java/org/apache/ranger/services/kms/client/KMSResourceMgr.java e61d0bc
security-admin/src/main/java/org/apache/ranger/biz/KmsKeyMgr.java 693e959
security-admin/src/main/java/org/apache/ranger/biz/RangerBizUtil.java e0a9840
security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java 7947d4b
security-admin/src/main/java/org/apache/ranger/biz/ServiceMgr.java 0059884
security-admin/src/main/java/org/apache/ranger/biz/SessionMgr.java 2e9d6d5
security-admin/src/main/java/org/apache/ranger/biz/UserMgr.java a508926
security-admin/src/main/java/org/apache/ranger/biz/XUserMgr.java 86f9d96
security-admin/src/main/java/org/apache/ranger/common/SearchField.java 1891edb
security-admin/src/main/java/org/apache/ranger/common/ServiceUtil.java c1baae8
security-admin/src/main/java/org/apache/ranger/common/db/BaseDao.java 13607d3
security-admin/src/main/java/org/apache/ranger/db/XXPolicyDao.java e25540b
security-admin/src/main/java/org/apache/ranger/db/XXServiceVersionInfoDao.java 5291045
security-admin/src/main/java/org/apache/ranger/entity/XXContextEnricherDef.java e035e58
security-admin/src/main/java/org/apache/ranger/entity/XXPolicyBase.java 8564d43
security-admin/src/main/java/org/apache/ranger/entity/XXPolicyConditionDef.java d738841
security-admin/src/main/java/org/apache/ranger/entity/XXPolicyItemUserPerm.java 874ca20
security-admin/src/main/java/org/apache/ranger/rest/AssetREST.java 3d649df
security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java 1185e45
security-admin/src/main/java/org/apache/ranger/rest/TagREST.java 3dfb250
security-admin/src/main/java/org/apache/ranger/security/handler/RangerAuthenticationProvider.java 3fa3436
security-admin/src/main/java/org/apache/ranger/security/web/authentication/RangerAuthenticationEntryPoint.java 6496698
security-admin/src/main/java/org/apache/ranger/security/web/filter/RangerSSOAuthenticationFilter.java d431bc1
security-admin/src/main/java/org/apache/ranger/security/web/filter/RangerSecurityContextFormationFilter.java 7314782
security-admin/src/main/java/org/apache/ranger/service/RangerPolicyService.java 4b792de
security-admin/src/main/java/org/apache/ranger/solr/SolrMgr.java 1b5793f
Diff: https://reviews.apache.org/r/50511/diff/
Testing
-------
Built clean and ran unit tests
installed ranger and ranger-kms.
Tested create/update/delete operation on service, policy, users and groups.
Thanks,
Pradeep Agrawal