[vf] New Salesforce VisualForce language support #279
Conversation
jsotuyod
added
the
a:new-language
|
@jsotuyod Done! |
|
@sgorbaty language order is alphabetical, should be |
|
|
||
| final ASTText textValue = attr.getFirstDescendantOfType(ASTText.class); | ||
| if (textValue != null) { | ||
| if (Pattern.compile("\\{(\\w|,|\\.|'|:|\\s)*\\}").matcher(textValue.getImage()).matches()) { |
There was a problem hiding this comment.
consider moving the compiled pattern into a static final field. Regex compilation is a slow process to be done on the fly, specially for constant patterns.
|
|
||
| } | ||
|
|
||
| private boolean doesElContainAnyUnescapedIdentifiers(final ASTElExpression elExpression, List<ESCAPING> escapes) { |
There was a problem hiding this comment.
consider using EnumSet<ESCAPING> rather than a List
| return toReturn; | ||
| } | ||
|
|
||
| enum ESCAPING { |
There was a problem hiding this comment.
being this an enum (equivalent to a class) rather than a constant, consider renaming it to Escaping to comply better with naming conventions (@adangel we should check our checkstye config, since it missed this one)
There was a problem hiding this comment.
@jsotuyod I had a look at the checkstyle rules. It indeed doesn't detect this case - only uppercase enum name. We use the default TypeName check, and the regex just ensures, that the type name begins with an uppercase letter. But after that, any combination of upper/lower case letters are possible - including all uppercase...
There is however AbbreviationAsWordInName. This check could detect such cases. However, it will probably require net.sourceforge.pmd.PMD to become net.sourceforge.pmd.Pmd.... which looks more correct, but would be a very drastic change....
There was a problem hiding this comment.
@adangel that new check allows for a allowedAbbreviationLength param setting maximum consecutive letters in uppercase, which conveniently defaults to 3, so PMD and CPD would be ok (and technically, PMD is an acronym). We could play with it and see how many violations are found in our codebase / which ones we strongly disagree with, and move on from there.
| isEscaped = true; | ||
| continue; | ||
| default: |
There was a problem hiding this comment.
you are definitely missing a break here
|
This has been merged and will be included in PMD 5.6.0 and later. |
|
@sergeygorbaty I will add a few XPath rules as soon the Up2Go fork contains the merged in language module. @jsotuyod: can you trigger the update of my fork? |
|
@up2go-rsoesemann I can't update any fork for which I have no write repositories. Instructions to do that yourself can be found here |
|
Ahh, ok. @adangel has access and did this in the past using a script on a regular basis. |
|
@up2go-rsoesemann if it helps, I sync my fork with this script: #!/bin/bash
git fetch upstream
CURRENT_BRANCH=$(git rev-parse --abbrev-ref HEAD)
for branch in "pmd/5.5.x" "pmd/5.4.x" "master"; do
echo "Synching $branch"
git checkout $branch
git pull upstream $branch
git push
done
git checkout $CURRENT_BRANCHIt will sync 3 branches (master, pmd/5.5.x and pmd 5.4.x) and move back to which ever branch you were in before starting the sync. The remotes are assumed to be I could add code to abort on a dirty working copy, but so far I haven't. |
|
@up2go-rsoesemann I can't update Up2Go/pmd, since you have pushed changes there, that aren't on pmd/pmd yet. Once #281 is merged, these changes will be gone. In future, I'd suggest to create feature branches for pull requests, so that it's easier to keep the master branch updated with upstream while working on a feature... |
jsotuyod
changed the title
Hi @jsotuyod,
This is a brand new VF language support with a Stored XSS security rule.
It looks for Stored XSS in all (but style/script) incorrectly escaped merge fields.
FYI, the style check fails because I have no rights to upload a snapshot to Nexus.
The parser is covered by tests. The major difference compared to say JSP parser is that it also parses the island grammar of EL expression into AST nodes.
I've tested this thoroughly on my end but open to suggestions/feedback.
Cheers,
Sergey