On Thu, 9 Jun 2022 21:00:55 GMT, Sean Mullan <mul...@openjdk.org> wrote:

>> Hai-May Chao has updated the pull request incrementally with two additional 
>> commits since the last revision:
>> 
>>  - Inconsistent entries test
>>  - Inconsistent entries test
>
> src/java.base/share/classes/javax/crypto/CryptoPolicyParser.java line 202:
> 
>> 200:             if (!processedPermissions.isEmpty()) {
>> 201:                 throw new ParsingException(st.lineno(), "Inconsistent 
>> policy");
>> 202:             }
> 
> Instead of setting the `allPermEntryFound` flag, what if you instead put an 
> entry for `CryptoAllPermission.ALG_NAME` in `processedPermissions` here. Then 
> if there are more entries after this, I think `isConsistent` will catch it in 
> the following code:
> 
> 
>         if (processedPermissions.containsKey(CryptoAllPermission.ALG_NAME)) {
>             return false;
>         }

Yes, with the `allPermEntryFound` flag, the current fix would not require to 
put the `javax.crypto.CryptoAllPermission` entry in `processedPermissions`. So 
`processedPermissions` will be used to keep `javax.crypto.CryptoPermission` 
entries and is updated by `isConsistent()`, and no need to deal with 
`javax.crypto.CryptoAllPermission` entry. I’d like to keep it as-is if there is 
no objection.

-------------

PR: https://git.openjdk.org/jdk/pull/8985

Reply via email to