Repository: apache/jena (java)

Issue:
Support for `UNDEF` in `ParameterizedSparqlString` VALUES methods
### Version

5.4

### Feature

## Description  
As I understand, in SPARQL 1.1, the `UNDEF` keyword in a `VALUES` clause represents an unbound binding (i.e. `null`). However, when I'm calling any of the following methods in  `org.apache.jena.query.ParameterizedSparqlString` with a `null` element, a `NullPointerException` is thrown:  
```java
setValues(String valueName, Collection<? extends org.apache.jena.rdf.model.RDFNode> items)
setValues(String valueName, org.apache.jena.rdf.model.RDFNode item)
setValues(Map<String, Collection<? extends org.apache.jena.rdf.model.RDFNode>> itemsMap)
```
There is also no existing `RDFNode` implementation that represents the SPARQL keyword `UNDEF`. According to the SPARQL 1.1 specification, `UNDEF` must be allowed in a VALUES clause to represent an unbound binding.

## Actual Behavior
- A `NullPointerException` occurs
- No way to explicitly represent the `UNDEF` keyword via the API

## Expected Behavior
- No `NullPointerException` is thrown
- The generated query is valid SPARQL:
```sparql
VALUES (?x ?y) {
  (:uri1 1)
  (:uri2 UNDEF)
}
```

## Possible Solutions
1. Introduce an `UndefRDFNode`
    - Create a class `UndefRDFNode` implements `RDFNode` whose `toString()` returns "UNDEF".
    - `ParameterizedSparqlString` detects instances of `UndefRDFNode` and render them as `UNDEF`.
2. Treat `null` as `UNDEF`
    - Modify all `setValues` methods to catch `null` entries and output `UNDEF` instead of throwing an exception.
    - Pros: No new class required.
    - Cons: Overloading `null` may be confusing in other contexts.
     
3. Reuse an existing Node type
    - Investigate if `Node.ANY` or another core Node can be repurposed to mean unbound.
    - Please note: `Node.ANY` is intended for pattern matching, not unbound results, so semantics differ.

## Tests
Add unit tests for each overload of `setValues` method to verify that the correct results are returned:
- Single-variable `UNDEF` case
- Mixed bindings, e.g. `( :uri1 1 )`, `( :uri2 UNDEF )`
- Multi-variable rows containing `UNDEF`

## References
- Apache JIRA: [JENA-1750](https://issues.apache.org/jira/browse/JENA-1750)
- SPARQL 1.1 Query Language: [VALUES clause](https://www.w3.org/TR/sparql11-query/#inline-data)

### Are you interested in contributing a solution yourself?

None

Pull request description:
Permit injecting UNDEF in ParameterizedSparqlString (GH-3268)
This commit adds the ability for `ParameterizedSparqlString` to support injecting the `UNDEF` keyword into `VALUES` clauses as needed.  A new `undef()` method is added to the class that provides access to a special instance level constant `RDFNode` that is used to indicate that the caller wants to inject `UNDEF`.

Under the hood this is just a wrapper around a `Node` constant but due to the `setValues()`/`setRowValues()` part of the API working in `RDFNode`'s rather than `Node`'s went with this approach.  I considered refactoring further to allow that part of the API to take `Node` and `RDFNode` interchangeably **but** given the limitations of Java type erasure generics (several of the methods in that part of the API use `Collection<? extends RDFNode>` and similar) that would effectively introduce a breaking API change so probably best done for a major release in a separate PR.

GitHub issue resolved #3268 

----

 - [x] Tests are included.
 - [ ] Documentation change and updates are provided for the [Apache Jena website](https://github.com/apache/jena-site/)
 - [x] Commits have been squashed to remove intermediate development commit messages.
 - [x] Key commit messages start with the issue number (GH-xxxx)

By submitting this pull request, I acknowledge that I am making a contribution to the Apache Software Foundation under the terms and conditions of the [Contributor's Agreement](https://www.apache.org/licenses/contributor-agreements.html).

----

See the [Apache Jena "Contributing" guide](https://github.com/apache/jena/blob/main/CONTRIBUTING.md).


Patch:
diff --git a/jena-arq/src/main/java/org/apache/jena/query/ParameterizedSparqlString.java b/jena-arq/src/main/java/org/apache/jena/query/ParameterizedSparqlString.java
index 93cc4d3727..a1ee3ee681 100644
--- a/jena-arq/src/main/java/org/apache/jena/query/ParameterizedSparqlString.java
+++ b/jena-arq/src/main/java/org/apache/jena/query/ParameterizedSparqlString.java
@@ -120,7 +120,7 @@ import org.apache.jena.update.UpdateRequest;
  * While this class was in part designed to prevent SPARQL injection it is by no
  * means foolproof because it works purely at the textual level. The current
  * version of the code addresses some possible attack vectors that the
- * developers have identified but we do not claim to be sufficiently devious to
+ * developers have identified, but we do not claim to be sufficiently devious to
  * have thought of and prevented every possible attack vector.
  * </p>
  * <p>
@@ -134,7 +134,10 @@ import org.apache.jena.update.UpdateRequest;
  */
 public class ParameterizedSparqlString implements PrefixMapping {
 
+    private static final Node UNDEF = NodeFactory.createURI("urn:x-arq:undef");
+
     private Model model = ModelFactory.createDefaultModel();
+    private final RDFNode undef = model.wrapAsResource(UNDEF);
 
     private StringBuilder cmd = new StringBuilder();
     private String baseUri;
@@ -626,6 +629,16 @@ public class ParameterizedSparqlString implements PrefixMapping {
         return this.baseUri;
     }
 
+    /**
+     * Gets a special {@link RDFNode} value that can be used with the various {@link #setValues(Map)}-like methods to
+     * inject the {@code UNDEF} keyword into a parameterised {@code VALUES} clause.
+     *
+     * @return Special undefined node value
+     */
+    public RDFNode undef() {
+        return this.undef;
+    }
+
     /**
      * Helper method which does the validation of the parameters
      *
@@ -1290,7 +1303,7 @@ public class ParameterizedSparqlString implements PrefixMapping {
         Pattern p = Pattern.compile("\"[?$]" + var + "\"|'[?$]" + var + "'");
 
         if (p.matcher(command).find() && n.isLiteral()) {
-            throw new ARQException("Command string is vunerable to injection attack, variable ?" + var
+            throw new ARQException("Command string is vulnerable to injection attack, variable ?" + var
                     + " appears surrounded directly by quotes and is bound to a literal which provides a SPARQL injection attack vector");
         }
 
@@ -1305,7 +1318,7 @@ public class ParameterizedSparqlString implements PrefixMapping {
 
             if (n.isLiteral()) {
                 if (delims.isInsideLiteral(posMatch.start(1), posMatch.end(1))) {
-                    throw new ARQException("Command string is vunerable to injection attack, variable ?" + var
+                    throw new ARQException("Command string is vulnerable to injection attack, variable ?" + var
                             + " appears inside of a literal and is bound to a literal which provides a SPARQL injection attack vector");
                 }
             }
@@ -1336,7 +1349,7 @@ public class ParameterizedSparqlString implements PrefixMapping {
         // Check each occurrence of the variable for safety
         if (n.isLiteral()) {
             if (delims.isInsideLiteral(position, position)) {
-                throw new ARQException("Command string is vunerable to injection attack, a positional paramter (index "
+                throw new ARQException("Command string is vulnerable to injection attack, a positional parameter (index "
                         + index
                         + ") appears inside of a literal and is bound to a literal which provides a SPARQL injection attack vector");
             }
@@ -1358,6 +1371,10 @@ public class ParameterizedSparqlString implements PrefixMapping {
     }
 
     protected final String stringForNode(Node n, SerializationContext context) {
+        if (n == UNDEF) {
+            return "UNDEF";
+        }
+
         String str = FmtUtils.stringForNode(n, context);
         if (n.isLiteral() && str.contains("'")) {
             // Should escape ' to avoid a possible injection vulnerability
@@ -1369,7 +1386,7 @@ public class ParameterizedSparqlString implements PrefixMapping {
     /**
      * <p>
      * This method is where the actual work happens, the original command text
-     * is always preserved and we just generated a temporary command string by
+     * is always preserved, and we just generate a temporary command string by
      * prepending the defined Base URI and namespace prefixes at the start of
      * the command and injecting the set parameters into a copy of that base
      * command string and return the resulting command.
@@ -1849,7 +1866,10 @@ public class ParameterizedSparqlString implements PrefixMapping {
      * See setRowValues to assign multiple values to multiple variables.<br>
      * Using "valueName" with list(prop_A, obj_A) on query "VALUES (?p ?o)
      * {?valueName}" * would produce "VALUES (?p ?o) {(prop_A obj_A)}".
-     *
+     * <p>
+     * Note that if you want to inject the SPARQL keyword {@code UNDEF} for any of the values then you
+     * <strong>MUST</strong> use the special {@link #undef()} value in the items parameter as appropriate.
+     * </p>
      *
      * @param valueName
      * @param items
@@ -1871,6 +1891,10 @@ public class ParameterizedSparqlString implements PrefixMapping {
      * Assign a VALUES valueName with a single item.<br>
      * Using "valueName" with Literal obj_A on query "VALUES ?o {?valueName}"
      * would produce * "VALUES ?o {obj_A}".
+     * <p>
+     * Note that if you want to inject the SPARQL keyword {@code UNDEF} for the value then you <strong>MUST</strong> use
+     * the special {@link #undef()} value as the item parameter.
+     * </p>
      *
      * @param valueName
      * @param item
@@ -1880,10 +1904,14 @@ public class ParameterizedSparqlString implements PrefixMapping {
     }
 
     /**
-     * ** Sets a map of VALUES valueNames and their items.<br>
+     * Sets a map of VALUES valueNames and their items.<br>
      * Can be used to assign multiple values to a single variable or single
      * value to multiple variables (if using a List) in the SPARQL query.<br>
      * See setRowValues to assign multiple values to multiple variables.
+     * <p>
+     * Note that if you want to inject the SPARQL keyword {@code UNDEF} for any of the values then you
+     * <strong>MUST</strong> use the special {@link #undef()} value in the itemsMap parameter as appropriate.
+     * </p>
      *
      * @param itemsMap
      */
@@ -1896,6 +1924,10 @@ public class ParameterizedSparqlString implements PrefixMapping {
      * Using "valuesName" with list(list(prop_A, obj_A), list(prop_B, obj_B)) on
      * query "VALUES (?p ?o) {?valuesName}" would produce "VALUES (?p ?o)
      * {(prop_A obj_A) * (prop_B obj_B)}".
+     * <p>
+     * Note that if you want to inject the SPARQL keyword {@code UNDEF} for any of the values then you
+     * <strong>MUST</strong> use the special {@link #undef()} value in the rowItems parameter as appropriate.
+     * </p>
      *
      * @param valueName
      * @param rowItems
@@ -2008,8 +2040,6 @@ public class ParameterizedSparqlString implements PrefixMapping {
 
         /**
          * Tidy up valueName if doesn't start with a ? or $.
-         *
-         * @param valueName
          * @return
          */
         private String createTarget() {


Test patch:
diff --git a/jena-arq/src/test/java/org/apache/jena/query/TestParameterizedSparqlString.java b/jena-arq/src/test/java/org/apache/jena/query/TestParameterizedSparqlString.java
index de470ee8b1..cfd3932dda 100644
--- a/jena-arq/src/test/java/org/apache/jena/query/TestParameterizedSparqlString.java
+++ b/jena-arq/src/test/java/org/apache/jena/query/TestParameterizedSparqlString.java
@@ -1674,7 +1674,6 @@ public class TestParameterizedSparqlString {
         queryStr.asQuery();
     }
 
-
     @Test
     public void test_param_string_non_injection_03() {
         String prefixes="PREFIX : <http://purl.bdrc.io/ontology/core/>\n" +
@@ -1982,8 +1981,7 @@ public class TestParameterizedSparqlString {
 
         String exp = "SELECT * WHERE { VALUES ?o {(\"test\")} ?s ?p ?o }";
         String res = pss.toString();
-        //System.out.println("Exp: " + exp);
-        //System.out.println("Res: " + res);
+
         Assert.assertEquals(exp, res);
     }
 
@@ -1996,8 +1994,20 @@ public class TestParameterizedSparqlString {
 
         String exp = "SELECT * WHERE { VALUES $o {(\"test\")} $s $p $o }";
         String res = pss.toString();
-        //System.out.println("Exp: " + exp);
-        //System.out.println("Res: " + res);
+
+        Assert.assertEquals(exp, res);
+    }
+
+    @Test
+    public void test_set_values_undef() {
+        // Tests a single value being added using '$' variable syntax - always adding parenthesis.
+        String str = "SELECT * WHERE { VALUES $o {$objs} $s $p $o }";
+        ParameterizedSparqlString pss = new ParameterizedSparqlString(str);
+        pss.setValues("objs", pss.undef());
+
+        String exp = "SELECT * WHERE { VALUES $o {(UNDEF)} $s $p $o }";
+        String res = pss.toString();
+
         Assert.assertEquals(exp, res);
     }
 
@@ -2010,8 +2020,7 @@ public class TestParameterizedSparqlString {
 
         String exp = "SELECT * WHERE { ?o {?objs} ?s ?p ?o }";
         String res = pss.toString();
-        //System.out.println("Exp: " + exp);
-        //System.out.println("Res: " + res);
+
         Assert.assertEquals(exp, res);
     }
 
@@ -2024,8 +2033,7 @@ public class TestParameterizedSparqlString {
 
         String exp = "SELECT * WHERE { VALUES ?o ?objs ?s ?p ?o }";
         String res = pss.toString();
-        //System.out.println("Exp: " + exp);
-        //System.out.println("Res: " + res);
+
         Assert.assertEquals(exp, res);
     }
 
@@ -2038,8 +2046,7 @@ public class TestParameterizedSparqlString {
 
         String exp = "SELECT * WHERE { VALUES ?o {?objs} ?s ?p ?o }";
         String res = pss.toString();
-        //System.out.println("Exp: " + exp);
-        //System.out.println("Res: " + res);
+
         Assert.assertEquals(exp, res);
     }
 
@@ -2055,8 +2062,7 @@ public class TestParameterizedSparqlString {
 
         String exp = "SELECT * WHERE { VALUES (?o) {(\"obj_A\") (\"obj_B\")} ?s ?p ?o }";
         String res = pss.toString();
-        //System.out.println("Exp: " + exp);
-        //System.out.println("Res: " + res);
+
         Assert.assertEquals(exp, res);
     }
 
@@ -2071,8 +2077,6 @@ public class TestParameterizedSparqlString {
         String exp = "SELECT * WHERE { VALUES (?o) {} ?s ?p ?o }";
         String res = pss.toString();
 
-        //System.out.println("Exp: " + exp);
-        //System.out.println("Res: " + res);
         Assert.assertEquals(exp, res);
     }
 
@@ -2088,8 +2092,23 @@ public class TestParameterizedSparqlString {
 
         String exp = "SELECT * WHERE { VALUES (?p ?o) {(<http://example.org/prop_A> \"obj_A\")} ?s ?p ?o }";
         String res = pss.toString();
-        //System.out.println("Exp: " + exp);
-        //System.out.println("Res: " + res);
+
+        Assert.assertEquals(exp, res);
+    }
+
+    @Test
+    public void test_set_values_multiple_variables_including_undef() {
+        // Tests two values for same variable.
+        String str = "SELECT * WHERE { VALUES (?p ?o) {?vars} ?s ?p ?o }";
+        ParameterizedSparqlString pss = new ParameterizedSparqlString(str);
+        List<RDFNode> vars = new ArrayList<>();
+        vars.add(ResourceFactory.createProperty("http://example.org/prop_A"));
+        vars.add(pss.undef());
+        pss.setValues("vars", vars);
+
+        String exp = "SELECT * WHERE { VALUES (?p ?o) {(<http://example.org/prop_A> UNDEF)} ?s ?p ?o }";
+        String res = pss.toString();
+
         Assert.assertEquals(exp, res);
     }
 
@@ -2133,13 +2152,13 @@ public class TestParameterizedSparqlString {
 
         List<RDFNode> props = new ArrayList<>();
         props.add(ResourceFactory.createProperty("http://example.org/prop_A"));
+        props.add(pss.undef());
         props.add(ResourceFactory.createProperty("http://example.org/prop_B"));
         pss.setValues("props", props);
 
-        String exp = "SELECT * WHERE { VALUES ?p {(<http://example.org/prop_A>) (<http://example.org/prop_B>)} VALUES ?o {(\"obj_A\") (\"obj_B\")} ?s ?p ?o }";
+        String exp = "SELECT * WHERE { VALUES ?p {(<http://example.org/prop_A>) (UNDEF) (<http://example.org/prop_B>)} VALUES ?o {(\"obj_A\") (\"obj_B\")} ?s ?p ?o }";
         String res = pss.toString();
-        //System.out.println("Exp: " + exp);
-        //System.out.println("Res: " + res);
+
         Assert.assertEquals(exp, res);
     }
 
@@ -2164,8 +2183,7 @@ public class TestParameterizedSparqlString {
 
         String exp = "SELECT * WHERE { VALUES (?p ?o) {(<http://example.org/prop_A> \"obj_A\") (<http://example.org/prop_B> \"obj_B\")} ?s ?p ?o }";
         String res = pss.toString();
-        //System.out.println("Exp: " + exp);
-        //System.out.println("Res: " + res);
+
         Assert.assertEquals(exp, res);
     }
 
@@ -2188,8 +2206,6 @@ public class TestParameterizedSparqlString {
         String[] res = ParameterizedSparqlString.extractTargetVars(cmd, valueName);
         String[] exp = new String[]{"o"};
 
-        //System.out.println("Exp: " + String.join(",", exp));
-        //System.out.println("Res: " + String.join(",", res));
         Assert.assertArrayEquals(exp, res);
     }
 
@@ -2201,8 +2217,6 @@ public class TestParameterizedSparqlString {
         String[] res = ParameterizedSparqlString.extractTargetVars(cmd, valueName);
         String[] exp = new String[]{"p", "o"};
 
-        ///System.out.println("Exp: " + String.join(",", exp));
-        //System.out.println("Res: " + String.join(",", res));
         Assert.assertArrayEquals(exp, res);
     }
 
@@ -2214,8 +2228,6 @@ public class TestParameterizedSparqlString {
         String[] res = ParameterizedSparqlString.extractTargetVars(cmd, valueName);
         String[] exp = new String[]{"o"};
 
-        //System.out.println("Exp: " + String.join(",", exp));
-        //System.out.println("Res: " + String.join(",", res));
         Assert.assertArrayEquals(exp, res);
     }
 
@@ -2227,8 +2239,6 @@ public class TestParameterizedSparqlString {
         String[] res = ParameterizedSparqlString.extractTargetVars(cmd, valueName);
         String[] exp = new String[]{};
 
-        //System.out.println("Exp: " + String.join(",", exp));
-        //System.out.println("Res: " + String.join(",", res));
         Assert.assertArrayEquals(exp, res);
     }
 
@@ -2240,8 +2250,6 @@ public class TestParameterizedSparqlString {
         String[] res = ParameterizedSparqlString.extractTargetVars(cmd, valueName);
         String[] exp = new String[]{};
 
-        //System.out.println("Exp: " + String.join(",", exp));
-        //System.out.println("Res: " + String.join(",", res));
         Assert.assertArrayEquals(exp, res);
     }
 
@@ -2253,8 +2261,6 @@ public class TestParameterizedSparqlString {
         String[] res = ParameterizedSparqlString.extractTargetVars(cmd, valueName);
         String[] exp = new String[]{};
 
-        //System.out.println("Exp: " + String.join(",", exp));
-        //System.out.println("Res: " + String.join(",", res));
         Assert.assertArrayEquals(exp, res);
     }
 
@@ -2266,8 +2272,6 @@ public class TestParameterizedSparqlString {
         String[] res = ParameterizedSparqlString.extractTargetVars(cmd, valueName);
         String[] exp = new String[]{};
 
-        //System.out.println("Exp: " + String.join(",", exp));
-        //System.out.println("Res: " + String.join(",", res));
         Assert.assertArrayEquals(exp, res);
     }
 
@@ -2279,8 +2283,6 @@ public class TestParameterizedSparqlString {
         String[] res = ParameterizedSparqlString.extractTargetVars(cmd, valueName);
         String[] exp = new String[]{};
 
-        //System.out.println("Exp: " + String.join(",", exp));
-        //System.out.println("Res: " + String.join(",", res));
         Assert.assertArrayEquals(exp, res);
     }
 
@@ -2292,9 +2294,6 @@ public class TestParameterizedSparqlString {
         String[] res = ParameterizedSparqlString.extractTargetVars(cmd, valueName);
         String[] exp = new String[]{};
 
-        //System.out.println("Exp: " + String.join(",", exp));
-        //System.out.println("Res: " + String.join(",", res));
         Assert.assertArrayEquals(exp, res);
     }
-
 }