Skip to content

Java: Deprecate StringLiteral.getRepresentedString() #7004

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

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
lgtm,codescanning
* The predicate `StringLiteral.getRepresentedString()` has been deprecated for removal in a future version because it is just an alias for `getValue()`. That predicate should be used instead.
16 changes: 14 additions & 2 deletions java/ql/lib/semmle/code/java/Expr.qll
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ class CompileTimeConstantExpr extends Expr {
*/
pragma[nomagic]
string getStringValue() {
result = this.(StringLiteral).getRepresentedString()
result = this.(StringLiteral).getValue()
or
result =
this.(AddExpr).getLeftOperand().(CompileTimeConstantExpr).getStringValue() +
Expand Down Expand Up @@ -745,9 +745,21 @@ class CharacterLiteral extends Literal, @characterliteral {
*/
class StringLiteral extends Literal, @stringliteral {
/**
* Gets the string represented by this string literal, that is, the content
* of the literal without enclosing quotes and with escape sequences translated.
*
* Unpaired Unicode surrogate characters (U+D800 to U+DFFF) are replaced with the
* replacement character U+FFFD.
*/
override string getValue() { result = super.getValue() }

/**
* DEPRECATED: This predicate will be removed in a future version because
* it is just an alias for `getValue()`; that predicate should be used instead.
*
* Gets the literal string without the quotes.
*/
string getRepresentedString() { result = this.getValue() }
deprecated string getRepresentedString() { result = this.getValue() }

/** Holds if this string literal is a text block (`""" ... """`). */
predicate isTextBlock() { this.getLiteral().matches("\"\"\"%") }
Expand Down
4 changes: 1 addition & 3 deletions java/ql/lib/semmle/code/java/JDKAnnotations.qll
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,7 @@ class SuppressWarningsAnnotation extends Annotation {
}

/** Gets the name of a warning suppressed by this annotation. */
string getASuppressedWarning() {
result = this.getASuppressedWarningLiteral().getRepresentedString()
}
string getASuppressedWarning() { result = this.getASuppressedWarningLiteral().getValue() }
}

/** A `@Target` annotation. */
Expand Down
6 changes: 3 additions & 3 deletions java/ql/lib/semmle/code/java/Reflection.qll
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ class ReflectiveClassIdentifierMethodAccess extends ReflectiveClassIdentifier, M
/**
* If the argument to this call is a `StringLiteral`, then return that string.
*/
string getTypeName() { result = this.getArgument(0).(StringLiteral).getRepresentedString() }
string getTypeName() { result = this.getArgument(0).(StringLiteral).getValue() }

override RefType getReflectivelyIdentifiedClass() {
// We only handle cases where the class is specified as a string literal to this call.
Expand Down Expand Up @@ -360,7 +360,7 @@ class ReflectiveMethodAccess extends ClassMethodAccess {
this.getInferredClassType().inherits(result)
) and
// Only consider instances where the method name is provided as a `StringLiteral`.
result.hasName(this.getArgument(0).(StringLiteral).getRepresentedString())
result.hasName(this.getArgument(0).(StringLiteral).getValue())
}
}

Expand Down Expand Up @@ -400,6 +400,6 @@ class ReflectiveFieldAccess extends ClassMethodAccess {
this.getInferredClassType().inherits(result)
)
) and
result.hasName(this.getArgument(0).(StringLiteral).getRepresentedString())
result.hasName(this.getArgument(0).(StringLiteral).getValue())
}
}
4 changes: 2 additions & 2 deletions java/ql/lib/semmle/code/java/StringFormat.qll
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ private predicate formatStringFragment(Expr fmt) {
private predicate formatStringValue(Expr e, string fmtvalue) {
formatStringFragment(e) and
(
e.(StringLiteral).getRepresentedString() = fmtvalue
e.(StringLiteral).getValue() = fmtvalue
or
e.getType() instanceof IntegralType and fmtvalue = "1" // dummy value
or
Expand Down Expand Up @@ -318,7 +318,7 @@ private predicate formatStringValue(Expr e, string fmtvalue) {
getprop.hasName("getProperty") and
getprop.getDeclaringType().hasQualifiedName("java.lang", "System") and
getprop.getNumberOfParameters() = 1 and
ma.getAnArgument().(StringLiteral).getRepresentedString() = prop and
ma.getAnArgument().(StringLiteral).getValue() = prop and
(prop = "line.separator" or prop = "file.separator" or prop = "path.separator") and
fmtvalue = "x" // dummy value
)
Expand Down
4 changes: 2 additions & 2 deletions java/ql/lib/semmle/code/java/UnitTests.qll
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ class TestNGTestMethod extends Method {
testAnnotation = this.getAnAnnotation() and
// The data provider must have the same name as the referenced data provider
result.getDataProviderName() =
testAnnotation.getValue("dataProvider").(StringLiteral).getRepresentedString()
testAnnotation.getValue("dataProvider").(StringLiteral).getValue()
|
// Either the data provider should be on the current class, or a supertype
this.getDeclaringType().getAnAncestor() = result.getDeclaringType()
Expand Down Expand Up @@ -258,7 +258,7 @@ class TestNGDataProviderMethod extends Method {
.(TestNGDataProviderAnnotation)
.getValue("name")
.(StringLiteral)
.getRepresentedString()
.getValue()
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -300,8 +300,8 @@ private predicate unsafeEscape(MethodAccess ma) {
// Removing `<script>` tags using a string-replace method is
// unsafe if such a tag is embedded inside another one (e.g. `<scr<script>ipt>`).
exists(StringReplaceMethod m | ma.getMethod() = m |
ma.getArgument(0).(StringLiteral).getRepresentedString() = "(<script>)" and
ma.getArgument(1).(StringLiteral).getRepresentedString() = ""
ma.getArgument(0).(StringLiteral).getValue() = "(<script>)" and
ma.getArgument(1).(StringLiteral).getValue() = ""
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,9 @@ class SpringComponentScan extends Annotation {
*/
string getBasePackages() {
// "value" and "basePackages" are synonymous, and are simple strings
result = this.getAValue("basePackages").(StringLiteral).getRepresentedString()
result = this.getAValue("basePackages").(StringLiteral).getValue()
or
result = this.getAValue("value").(StringLiteral).getRepresentedString()
result = this.getAValue("value").(StringLiteral).getValue()
or
exists(TypeLiteral typeLiteral |
// Base package classes are type literals whose package should be considered a base package.
Expand Down Expand Up @@ -201,7 +201,7 @@ class SpringComponent extends RefType {
.getType()
.hasQualifiedName("org.springframework.context.annotation", "Profile")
|
result = profileAnnotation.getAValue("value").(StringLiteral).getRepresentedString()
result = profileAnnotation.getAValue("value").(StringLiteral).getValue()
)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ private predicate boxedToString(Method method) {
* it is better to use a prepared query than to just put single quotes around the string.
*/
predicate endsInQuote(Expr expr) {
exists(string str | str = expr.(StringLiteral).getRepresentedString() | str.matches("%'"))
exists(string str | str = expr.(StringLiteral).getValue() | str.matches("%'"))
or
exists(Variable var | expr = var.getAnAccess() | endsInQuote(var.getAnAssignedValue()))
or
Expand Down
4 changes: 2 additions & 2 deletions java/ql/lib/semmle/code/java/security/HttpsUrls.qll
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,13 @@ private import semmle.code.java.frameworks.Networking
*/
class HttpStringLiteral extends StringLiteral {
HttpStringLiteral() {
exists(string s | this.getRepresentedString() = s |
exists(string s | this.getValue() = s |
s = "http"
or
s.matches("http://%") and
not s.substring(7, s.length()) instanceof PrivateHostName and
not TaintTracking::localExprTaint(any(StringLiteral p |
p.getRepresentedString() instanceof PrivateHostName
p.getValue() instanceof PrivateHostName
), this.getParent*())
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,5 +41,5 @@ private class DefaultInsecureBasicAuthSink extends InsecureBasicAuthSink {
* String pattern of basic authentication.
*/
private class BasicAuthString extends StringLiteral {
BasicAuthString() { exists(string s | this.getRepresentedString() = s | s.matches("Basic %")) }
BasicAuthString() { exists(string s | this.getValue() = s | s.matches("Basic %")) }
}
2 changes: 1 addition & 1 deletion java/ql/lib/semmle/code/java/security/RelativePaths.qll
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import java
* An element that starts with a relative path.
*/
predicate relativePath(Element tree, string command) {
exists(StringLiteral lit, string text | tree = lit and text = lit.getRepresentedString() |
exists(StringLiteral lit, string text | tree = lit and text = lit.getValue() |
text != "" and
text.regexpMatch(["[^/\\\\ \t]*", "[^/\\\\ \t]*[ \t].*"]) and
command = text.replaceAll("\t", " ").splitAt(" ", 0).replaceAll("\"", "")
Expand Down
2 changes: 1 addition & 1 deletion java/ql/lib/semmle/code/java/security/SecurityFlag.qll
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ abstract class FlagKind extends string {
flag.asExpr() = v and v.getType() instanceof FlagType
)
or
exists(StringLiteral s | s.getRepresentedString() = getAFlagName() | flag.asExpr() = s)
exists(StringLiteral s | s.getValue() = getAFlagName() | flag.asExpr() = s)
or
exists(MethodAccess ma | ma.getMethod().getName() = getAFlagName() |
flag.asExpr() = ma and
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,7 @@ class SensitiveMethodAccess extends SensitiveExpr, MethodAccess {
or
// This is particularly to pick up methods with an argument like "password", which
// may indicate a lookup.
exists(string s |
this.getAnArgument().(StringLiteral).getRepresentedString().toLowerCase() = s
|
exists(string s | this.getAnArgument().(StringLiteral).getValue().toLowerCase() = s |
s.matches(suspicious()) and
not s.matches(nonSuspicious())
)
Expand Down
4 changes: 2 additions & 2 deletions java/ql/src/Performance/InefficientEmptyStringTest.ql
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ where
mc.getQualifier().getType() instanceof TypeString and
mc.getMethod().hasName("equals") and
(
mc.getArgument(0).(StringLiteral).getRepresentedString() = "" or
mc.getQualifier().(StringLiteral).getRepresentedString() = ""
mc.getArgument(0).(StringLiteral).getValue() = "" or
mc.getQualifier().(StringLiteral).getValue() = ""
)
select mc, "Inefficient comparison to empty string, check for zero length instead."
8 changes: 4 additions & 4 deletions java/ql/src/Security/CWE/CWE-327/BrokenCryptoAlgorithm.ql
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,14 @@ import DataFlow
import PathGraph

private class ShortStringLiteral extends StringLiteral {
ShortStringLiteral() { getRepresentedString().length() < 100 }
ShortStringLiteral() { getValue().length() < 100 }
}

class BrokenAlgoLiteral extends ShortStringLiteral {
BrokenAlgoLiteral() {
getRepresentedString().regexpMatch(getInsecureAlgorithmRegex()) and
getValue().regexpMatch(getInsecureAlgorithmRegex()) and
// Exclude German and French sentences.
not getRepresentedString().regexpMatch(".*\\p{IsLowercase} des \\p{IsLetter}.*")
not getValue().regexpMatch(".*\\p{IsLowercase} des \\p{IsLetter}.*")
}
}

Expand All @@ -48,4 +48,4 @@ where
source.getNode().asExpr() = s and
conf.hasFlowPath(source, sink)
select c, source, sink, "Cryptographic algorithm $@ is weak and should not be used.", s,
s.getRepresentedString()
s.getValue()
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,14 @@ import semmle.code.java.dispatch.VirtualDispatch
import PathGraph

private class ShortStringLiteral extends StringLiteral {
ShortStringLiteral() { getRepresentedString().length() < 100 }
ShortStringLiteral() { getValue().length() < 100 }
}

class InsecureAlgoLiteral extends ShortStringLiteral {
InsecureAlgoLiteral() {
// Algorithm identifiers should be at least two characters.
getRepresentedString().length() > 1 and
exists(string s | s = getRepresentedString() |
getValue().length() > 1 and
exists(string s | s = getValue() |
not s.regexpMatch(getSecureAlgorithmRegex()) and
// Exclude results covered by another query.
not s.regexpMatch(getInsecureAlgorithmRegex())
Expand Down Expand Up @@ -72,4 +72,4 @@ where
conf.hasFlowPath(source, sink)
select c, source, sink,
"Cryptographic algorithm $@ may not be secure, consider using a different algorithm.", s,
s.getRepresentedString()
s.getValue()
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ private class HardcodedCharArray extends ArrayCreationExpr {
*/
class HardcodedExpr extends Expr {
HardcodedExpr() {
this.(StringLiteral).getRepresentedString() != "" or
this.(StringLiteral).getValue() != "" or
this instanceof HardcodedByteArray or
this instanceof HardcodedCharArray
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ predicate subjectToAtomicReferenceFieldUpdater(Field f) {
c.getMethod().getSourceDeclaration() = newUpdater and
isClassOf(c.getArgument(0).getType(), f.getDeclaringType()) and
isClassOf(c.getArgument(1).getType(), f.getType()) and
c.getArgument(2).(StringLiteral).getRepresentedString() = f.getName()
c.getArgument(2).(StringLiteral).getValue() = f.getName()
)
}

Expand All @@ -45,7 +45,7 @@ predicate lookedUpReflectively(Field f) {
exists(MethodAccess getDeclaredField |
isClassOf(getDeclaredField.getQualifier().getType(), f.getDeclaringType()) and
getDeclaredField.getMethod().hasName("getDeclaredField") and
getDeclaredField.getArgument(0).(StringLiteral).getRepresentedString() = f.getName()
getDeclaredField.getArgument(0).(StringLiteral).getValue() = f.getName()
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ class SpringViewManipulationConfig extends TaintTracking::Configuration {
exists(AddExpr e, StringLiteral sl |
node.asExpr() = e.getControlFlowNode().getASuccessor*() and
sl = e.getLeftOperand*() and
sl.getRepresentedString().matches(["redirect:%", "ajaxredirect:%", "forward:%"])
sl.getValue().matches(["redirect:%", "ajaxredirect:%", "forward:%"])
)
or
// Block flows like
Expand All @@ -79,7 +79,7 @@ class SpringViewManipulationConfig extends TaintTracking::Configuration {
sl = ca.getQualifier()
) and
ca = getAStringCombiningCall() and
sl.getRepresentedString().matches(["redirect:%", "ajaxredirect:%", "forward:%"])
sl.getValue().matches(["redirect:%", "ajaxredirect:%", "forward:%"])
|
exists(Call cc | DataFlow::localExprFlow(ca.getQualifier(), cc.getQualifier()) |
cc = node.asExpr()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ predicate hasShortECKeyPair(MethodAccess ma, string msg) {
kc.hasFlowPath(source, dest) and
DataFlow::localExprFlow(cie, ma.getArgument(0)) and
ma.getArgument(0).getType() instanceof ECGenParameterSpec and
getECKeySize(cie.getArgument(0).(StringLiteral).getRepresentedString()) < 256
getECKeySize(cie.getArgument(0).(StringLiteral).getValue()) < 256
) and
msg = "Key size should be at least 256 bits for EC encryption."
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ private class UnsafeBeanInitMethod extends Method {
exists(Annotation a | this.getAnAnnotation() = a |
a.getType().hasQualifiedName("org.springframework.context.annotation", "Bean") and
if a.getValue("name") instanceof StringLiteral
then identifier = a.getValue("name").(StringLiteral).getRepresentedString()
then identifier = a.getValue("name").(StringLiteral).getValue()
else identifier = this.getName()
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import DataFlow::PathGraph
class InsecureLdapUrlLiteral extends StringLiteral {
InsecureLdapUrlLiteral() {
// Match connection strings with the LDAP protocol and without private IP addresses to reduce false positives.
exists(string s | this.getRepresentedString() = s |
exists(string s | this.getValue() = s |
s.regexpMatch("(?i)ldap://[\\[a-zA-Z0-9].*") and
not s.substring(7, s.length()) instanceof PrivateHostName
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ class HostVerificationMethodAccess extends MethodAccess {
) and
this.getMethod().getNumberOfParameters() = 1 and
(
this.getArgument(0).(StringLiteral).getRepresentedString().charAt(0) != "." //string constant comparison e.g. uri.getHost().endsWith("example.com")
this.getArgument(0).(StringLiteral).getValue().charAt(0) != "." //string constant comparison e.g. uri.getHost().endsWith("example.com")
or
this.getArgument(0)
.(AddExpr)
Expand All @@ -63,15 +63,10 @@ class HostVerificationMethodAccess extends MethodAccess {
.getVariable()
.getAnAssignedValue()
.(StringLiteral)
.getRepresentedString()
.getValue()
.charAt(0) != "." //var1+var2, check var1 starts with "." e.g. String domainName = "example"; Uri.parse(url).getHost().endsWith(domainName+".com")
or
this.getArgument(0)
.(AddExpr)
.getLeftOperand()
.(StringLiteral)
.getRepresentedString()
.charAt(0) != "." //"."+var2, check string constant "." e.g. String domainName = "example.com"; Uri.parse(url).getHost().endsWith("www."+domainName)
this.getArgument(0).(AddExpr).getLeftOperand().(StringLiteral).getValue().charAt(0) != "." //"."+var2, check string constant "." e.g. String domainName = "example.com"; Uri.parse(url).getHost().endsWith("www."+domainName)
or
exists(MethodAccess ma, Method m, Field f |
this.getArgument(0) = ma and
Expand All @@ -87,7 +82,7 @@ class HostVerificationMethodAccess extends MethodAccess {
.getVariable()
.getAnAssignedValue()
.(StringLiteral)
.getRepresentedString()
.getValue()
.charAt(0) != "." //check variable starts with "." e.g. String domainName = "example.com"; Uri.parse(url).getHost().endsWith(domainName)
)
}
Expand Down
2 changes: 1 addition & 1 deletion java/ql/test/library-tests/Encryption/insecure.ql
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@ import default
import semmle.code.java.security.Encryption

from StringLiteral s
where s.getRepresentedString().regexpMatch(getInsecureAlgorithmRegex())
where s.getValue().regexpMatch(getInsecureAlgorithmRegex())
select s
2 changes: 1 addition & 1 deletion java/ql/test/library-tests/Encryption/secure.ql
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@ import default
import semmle.code.java.security.Encryption

from StringLiteral s
where s.getRepresentedString().regexpMatch(getSecureAlgorithmRegex())
where s.getValue().regexpMatch(getSecureAlgorithmRegex())
select s
Loading