Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[apex] AvoidHardcodingId false positives #776

Closed
parksungrin opened this issue Dec 5, 2017 · 7 comments
Closed

[apex] AvoidHardcodingId false positives #776

parksungrin opened this issue Dec 5, 2017 · 7 comments
Assignees
Labels
a:false-positive PMD flags a piece of code that is not problematic
Milestone

Comments

@parksungrin
Copy link

Please, prefix the report title with the language it applies to within brackets, such as [java] or [apex]. If not specific to a language, you can use [core]

Rule Set:
6.0.0 Snapshoot
AvoidHardcodingId

Description:
15 digit number is indicated as Salesforce Id.

objAssetDevice.IMEI__c = '359040082913024';

Code Sample demonstrating the issue:
359040082913024 is not Salesforce Id but below code indicated as violation of AvoidHardcodingId
objAssetDevice.IMEI__c = '359040082913024';

Running PMD through: [CLI | Ant | Maven | Gradle | Designer | Other]
CLI

@rsoesemann
Copy link
Member

@JAertgeerts didn't you add this rule. Can you make the regexp a bit smarter as describe here https://stackoverflow.com/questions/9742913/validating-a-salesforce-id?

@jsotuyod
Copy link
Member

jsotuyod commented Dec 5, 2017

@up2go-rsoesemann @JAertgeerts @parksungrin the rule is implemented based on this description which is actually a little more strict on 15 chars ids.

The rule is flawed for ids longer than 15 (it would allow, 16 and 17 chars "ids", and the 18 char ones are not validated).

However, for this scenario of a 15 char string, it would still match. I don't think we can safely detect this as a FP and avoid it...

@rsoesemann
Copy link
Member

The rule would need to incorporate also a checksum check as described here https://gist.github.com/jeriley/36b29f7c46527af4532aaf092c90dd56

A simple regex doesn't do the magic.

@jsotuyod
Copy link
Member

jsotuyod commented Dec 5, 2017

@up2go-rsoesemann that checksum applies only to 18 digit ids. 15 digit ids would still be flagged as long as the sixth position is a 0.

@alan-morey
Copy link

While not a solution for the false positive scenario where a 15 digit string is considered a Salesforce ID, the current pattern could be made stricter.

private static final Pattern PATTERN = Pattern.compile("^[a-zA-Z0-9]{5}[0][a-zA-Z0-9]{9,12}$", Pattern.CASE_INSENSITIVE);

The following pattern:

Pattern.compile("^[a-zA-Z0-9]{5}0[a-zA-Z0-9]{9}([a-zA-Z0-5]{3})?$");

Also is Pattern.CASE_INSENSITIVE flag redundant when the character classes are matching both upper and lower alpha?

@jsotuyod jsotuyod added this to the 6.1.0 milestone Dec 20, 2017
@jsotuyod
Copy link
Member

@alan-morey thanks, that's exactly what I was going after. The checksum check can be done too.

I'm flagging this for 6.1.0 release.

@adangel adangel modified the milestones: 6.1.0, 6.2.0 Feb 25, 2018
@adangel adangel modified the milestones: 6.2.0, 6.3.0 Mar 26, 2018
@jsotuyod jsotuyod self-assigned this Apr 10, 2018
jsotuyod added a commit to Monits/pmd that referenced this issue Apr 10, 2018
 - IDs are only 15, or 18 digits long
 - 18 digits long IDs are actually 15 digit IDs + checksum, which is now
validated
 - Resolves pmd#776
@jsotuyod jsotuyod changed the title [apex] AvoidHardcodingId indicate 15 digit number as Id [apex] AvoidHardcodingId false positives Apr 10, 2018
jsotuyod added a commit to Monits/pmd that referenced this issue Apr 10, 2018
@jsotuyod jsotuyod added has:pr a:false-positive PMD flags a piece of code that is not problematic labels Apr 10, 2018
adangel added a commit that referenced this issue Apr 24, 2018
Note: only solution right now is supressing the rule for the string.
@adangel
Copy link
Member

adangel commented Apr 24, 2018

Note: only solution for the specific string is right now to suppress the rule, like that:

       @SuppressWarnings('PMD.AvoidHardcodingId')
        String IMEI__c = '359040082913024';

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:false-positive PMD flags a piece of code that is not problematic
Projects
None yet
Development

No branches or pull requests

5 participants