Java: Ratpack HTTP Framework Additional Modeling #7007

JLLeitschuh · 2021-10-29T15:43:11Z

Adds models for ratpack.func.Pair, and ratpack.exec.Result.
Improve models for ratpack.exec.Promise.

github-actions · 2021-10-29T15:45:45Z

⚠️ The head of this PR and the base branch were compared for differences in the framework coverage reports. The generated reports are available in the artifacts of this workflow run. The differences will be picked up by the nightly job after the PR gets merged.

Click to show differences in coverage

java

Generated file changes for java

Changes to framework-coverage-java.rst:

-    Others,"``cn.hutool.core.codec``, ``com.esotericsoftware.kryo.io``, ``com.esotericsoftware.kryo5.io``, ``com.fasterxml.jackson.core``, ``com.fasterxml.jackson.databind``, ``com.opensymphony.xwork2.ognl``, ``com.unboundid.ldap.sdk``, ``flexjson``, ``groovy.lang``, ``groovy.util``, ``jodd.json``, ``net.sf.saxon.s9api``, ``ognl``, ``org.apache.commons.codec``, ``org.apache.commons.jexl2``, ``org.apache.commons.jexl3``, ``org.apache.commons.ognl``, ``org.apache.directory.ldap.client.api``, ``org.apache.ibatis.jdbc``, ``org.apache.shiro.codec``, ``org.apache.shiro.jndi``, ``org.codehaus.groovy.control``, ``org.dom4j``, ``org.hibernate``, ``org.jooq``, ``org.mvel2``, ``org.xml.sax``, ``org.xmlpull.v1``, ``play.mvc``, ``ratpack.core.form``, ``ratpack.core.handling``, ``ratpack.core.http``, ``ratpack.exec``, ``ratpack.form``, ``ratpack.func``, ``ratpack.handling``, ``ratpack.http``, ``ratpack.util``",39,99,151,,,,14,18,,
+    Others,"``cn.hutool.core.codec``, ``com.esotericsoftware.kryo.io``, ``com.esotericsoftware.kryo5.io``, ``com.fasterxml.jackson.core``, ``com.fasterxml.jackson.databind``, ``com.opensymphony.xwork2.ognl``, ``com.unboundid.ldap.sdk``, ``flexjson``, ``groovy.lang``, ``groovy.util``, ``jodd.json``, ``net.sf.saxon.s9api``, ``ognl``, ``org.apache.commons.codec``, ``org.apache.commons.jexl2``, ``org.apache.commons.jexl3``, ``org.apache.commons.ognl``, ``org.apache.directory.ldap.client.api``, ``org.apache.ibatis.jdbc``, ``org.apache.shiro.codec``, ``org.apache.shiro.jndi``, ``org.codehaus.groovy.control``, ``org.dom4j``, ``org.hibernate``, ``org.jooq``, ``org.mvel2``, ``org.xml.sax``, ``org.xmlpull.v1``, ``play.mvc``, ``ratpack.core.form``, ``ratpack.core.handling``, ``ratpack.core.http``, ``ratpack.exec``, ``ratpack.form``, ``ratpack.func``, ``ratpack.handling``, ``ratpack.http``, ``ratpack.util``",39,239,151,,,,14,18,,
-    Totals,,175,5332,408,13,6,10,107,33,1,66
+    Totals,,175,5472,408,13,6,10,107,33,1,66

Changes to framework-coverage-java.csv:

- ratpack.exec,,,26,,,,,,,,,,,,,,,,,,,,,,26
+ ratpack.exec,,,58,,,,,,,,,,,,,,,,,,,,,,58
- ratpack.func,,,5,,,,,,,,,,,,,,,,,,,,,,5
+ ratpack.func,,,59,,,,,,,,,,,,,,,,,,,,,,59
- ratpack.util,,,5,,,,,,,,,,,,,,,,,,,,,,5
+ ratpack.util,,,59,,,,,,,,,,,,,,,,,,,,,,59

java/ql/lib/semmle/code/java/frameworks/ratpack/Ratpack.qll

smowton · 2021-11-17T11:24:19Z

Noting I haven't forgotten this bug and intend to look at it Friday or Monday

smowton · 2021-11-17T11:51:08Z

(Edit: apologies I had missed discussion between @atorralba and @aschackmull on this)

JLLeitschuh · 2021-11-17T13:57:23Z

I think the bulk of the code changes here are "done" but I'm waiting on a fix for the bug in order to request this to be merged.

Could someone add the hacktoberfest-accepted label to this PR for me?

aschackmull · 2021-11-19T15:13:37Z

Bug should be fixed now (#7187), so you should be able to get things working using just Field[..].

JLLeitschuh · 2021-11-22T20:54:03Z

First-time contributors need a maintainer to approve running workflows

😆

Bug should be fixed now (#7187), so you should be able to get things working using just Field[..].

Indeed! This does now work! The PR is updated! 😄

github-actions · 2021-11-23T08:23:40Z

⚠️ The head of this PR and the base branch were compared for differences in the framework coverage reports. The generated reports are available in the artifacts of this workflow run. The differences will be picked up by the nightly job after the PR gets merged.

Click to show differences in coverage

java

Generated file changes for java

Changes to framework-coverage-java.rst:

-    Others,"``cn.hutool.core.codec``, ``com.esotericsoftware.kryo.io``, ``com.esotericsoftware.kryo5.io``, ``com.fasterxml.jackson.core``, ``com.fasterxml.jackson.databind``, ``com.opensymphony.xwork2.ognl``, ``com.unboundid.ldap.sdk``, ``flexjson``, ``groovy.lang``, ``groovy.util``, ``jodd.json``, ``net.sf.saxon.s9api``, ``ognl``, ``org.apache.commons.codec``, ``org.apache.commons.jexl2``, ``org.apache.commons.jexl3``, ``org.apache.commons.ognl``, ``org.apache.directory.ldap.client.api``, ``org.apache.ibatis.jdbc``, ``org.apache.shiro.codec``, ``org.apache.shiro.jndi``, ``org.codehaus.groovy.control``, ``org.dom4j``, ``org.hibernate``, ``org.jooq``, ``org.mvel2``, ``org.xml.sax``, ``org.xmlpull.v1``, ``play.mvc``, ``ratpack.core.form``, ``ratpack.core.handling``, ``ratpack.core.http``, ``ratpack.exec``, ``ratpack.form``, ``ratpack.func``, ``ratpack.handling``, ``ratpack.http``, ``ratpack.util``",39,99,151,,,,14,18,,
+    Others,"``cn.hutool.core.codec``, ``com.esotericsoftware.kryo.io``, ``com.esotericsoftware.kryo5.io``, ``com.fasterxml.jackson.core``, ``com.fasterxml.jackson.databind``, ``com.opensymphony.xwork2.ognl``, ``com.unboundid.ldap.sdk``, ``flexjson``, ``groovy.lang``, ``groovy.util``, ``jodd.json``, ``net.sf.saxon.s9api``, ``ognl``, ``org.apache.commons.codec``, ``org.apache.commons.jexl2``, ``org.apache.commons.jexl3``, ``org.apache.commons.ognl``, ``org.apache.directory.ldap.client.api``, ``org.apache.ibatis.jdbc``, ``org.apache.shiro.codec``, ``org.apache.shiro.jndi``, ``org.codehaus.groovy.control``, ``org.dom4j``, ``org.hibernate``, ``org.jooq``, ``org.mvel2``, ``org.xml.sax``, ``org.xmlpull.v1``, ``play.mvc``, ``ratpack.core.form``, ``ratpack.core.handling``, ``ratpack.core.http``, ``ratpack.exec``, ``ratpack.form``, ``ratpack.func``, ``ratpack.handling``, ``ratpack.http``, ``ratpack.util``",39,177,151,,,,14,18,,
-    Totals,,175,5369,431,13,6,10,107,33,1,66
+    Totals,,175,5447,431,13,6,10,107,33,1,66

Changes to framework-coverage-java.csv:

- ratpack.exec,,,26,,,,,,,,,,,,,,,,,,,,,,,26
+ ratpack.exec,,,48,,,,,,,,,,,,,,,,,,,,,,,48
- ratpack.func,,,5,,,,,,,,,,,,,,,,,,,,,,,5
+ ratpack.func,,,33,,,,,,,,,,,,,,,,,,,,,,,33
- ratpack.util,,,5,,,,,,,,,,,,,,,,,,,,,,,5
+ ratpack.util,,,33,,,,,,,,,,,,,,,,,,,,,,,33

github-actions · 2021-11-23T13:29:06Z

⚠️ The head of this PR and the base branch were compared for differences in the framework coverage reports. The generated reports are available in the artifacts of this workflow run. The differences will be picked up by the nightly job after the PR gets merged.

Click to show differences in coverage

java

Generated file changes for java

Changes to framework-coverage-java.rst:

-    Others,"``cn.hutool.core.codec``, ``com.esotericsoftware.kryo.io``, ``com.esotericsoftware.kryo5.io``, ``com.fasterxml.jackson.core``, ``com.fasterxml.jackson.databind``, ``com.opensymphony.xwork2.ognl``, ``com.unboundid.ldap.sdk``, ``flexjson``, ``groovy.lang``, ``groovy.util``, ``jodd.json``, ``net.sf.saxon.s9api``, ``ognl``, ``org.apache.commons.codec``, ``org.apache.commons.jexl2``, ``org.apache.commons.jexl3``, ``org.apache.commons.ognl``, ``org.apache.directory.ldap.client.api``, ``org.apache.ibatis.jdbc``, ``org.apache.shiro.codec``, ``org.apache.shiro.jndi``, ``org.codehaus.groovy.control``, ``org.dom4j``, ``org.hibernate``, ``org.jooq``, ``org.mvel2``, ``org.xml.sax``, ``org.xmlpull.v1``, ``play.mvc``, ``ratpack.core.form``, ``ratpack.core.handling``, ``ratpack.core.http``, ``ratpack.exec``, ``ratpack.form``, ``ratpack.func``, ``ratpack.handling``, ``ratpack.http``, ``ratpack.util``",39,99,151,,,,14,18,,
+    Others,"``cn.hutool.core.codec``, ``com.esotericsoftware.kryo.io``, ``com.esotericsoftware.kryo5.io``, ``com.fasterxml.jackson.core``, ``com.fasterxml.jackson.databind``, ``com.opensymphony.xwork2.ognl``, ``com.unboundid.ldap.sdk``, ``flexjson``, ``groovy.lang``, ``groovy.util``, ``jodd.json``, ``net.sf.saxon.s9api``, ``ognl``, ``org.apache.commons.codec``, ``org.apache.commons.jexl2``, ``org.apache.commons.jexl3``, ``org.apache.commons.ognl``, ``org.apache.directory.ldap.client.api``, ``org.apache.ibatis.jdbc``, ``org.apache.shiro.codec``, ``org.apache.shiro.jndi``, ``org.codehaus.groovy.control``, ``org.dom4j``, ``org.hibernate``, ``org.jooq``, ``org.mvel2``, ``org.xml.sax``, ``org.xmlpull.v1``, ``play.mvc``, ``ratpack.core.form``, ``ratpack.core.handling``, ``ratpack.core.http``, ``ratpack.exec``, ``ratpack.form``, ``ratpack.func``, ``ratpack.handling``, ``ratpack.http``, ``ratpack.util``",39,177,151,,,,14,18,,
-    Totals,,175,5369,431,13,6,10,107,33,1,66
+    Totals,,175,5447,431,13,6,10,107,33,1,66

Changes to framework-coverage-java.csv:

- ratpack.exec,,,26,,,,,,,,,,,,,,,,,,,,,,,26
+ ratpack.exec,,,48,,,,,,,,,,,,,,,,,,,,,,,48
- ratpack.func,,,5,,,,,,,,,,,,,,,,,,,,,,,5
+ ratpack.func,,,33,,,,,,,,,,,,,,,,,,,,,,,33
- ratpack.util,,,5,,,,,,,,,,,,,,,,,,,,,,,5
+ ratpack.util,,,33,,,,,,,,,,,,,,,,,,,,,,,33

smowton · 2021-11-23T13:30:10Z

java/ql/lib/semmle/code/java/frameworks/ratpack/Ratpack.qll

+            "pushLeft;(Object);;Argument[-1];" + right + " of ReturnValue;value",
+            "pushRight;(Object);;Argument[-1];" + left + " of ReturnValue;value",


Missing: arg0 -> left/right of returnval

You were indeed correct. Thank you for spotting this! I'm impressed with how thoroughly you check these PR's. I would have missed this as a reviewer.

This should now be fixed. See below.

smowton · 2021-11-23T13:43:25Z

java/ql/lib/semmle/code/java/frameworks/ratpack/RatpackExec.qll

-          "next;;;Argument[-1];ReturnValue;value",
+          "nextOp;;;Element of Argument[-1];Parameter[0] of Argument[0];value",
+          "flatOp;;;Element of Argument[-1];Parameter[0] of Argument[0];value",
+          // `nextOpIf` accesses qualfier the 'Promise' value and also returns the qualifier


Suggested change

// `nextOpIf` accesses qualfier the 'Promise' value and also returns the qualifier

// `nextOpIf` accesses qualifier the 'Promise' value and also returns the qualifier

I blame GitHub copilot 😆

java/ql/lib/semmle/code/java/frameworks/ratpack/RatpackExec.qll

smowton · 2021-11-23T13:49:20Z

java/ql/lib/semmle/code/java/frameworks/ratpack/RatpackExec.qll

+          // `blockingOp` passes the value to the argumet
+          "blockingOp;;;Element of Argument[-1];Parameter[0] of Argument[0];value",
+          // `replace` returns the passed `Promise`
+          "replace;;;Argument[0];ReturnValue;value",


The doc and the source both read like you get a new promise back, that discards the outcome of this and replaces it with the outcome of arg0?

Not quite, it's not discarding the promise, because everything earlier in the chain still needs to execute.

Here's how the method is implemented on the interface:

/** * Replaces {@code this} promise with the provided promise for downstream subscribers. * <p> * This is simply a more convenient form of {@link #flatMap(Function)}, where the given promise is returned. * This method can be used when a subsequent operation on a promise isn't dependent on the actual promised value. * <p> * If the upstream promise fails, its error will propagate downstream and the given promise will never be subscribed to. * * <pre class="java">{@code * import ratpack.test.exec.ExecHarness; * import ratpack.exec.ExecResult; * import ratpack.exec.Promise; * * import static org.junit.Assert.assertEquals; * * public class Example { * private static String value; * * public static void main(String... args) throws Exception { * ExecResult<String> result = ExecHarness.yieldSingle(c -> * Promise.value("foo") * .next(v -> value = v) * .replace(Promise.value("bar")) * ); * * assertEquals("bar", result.getValue()); * assertEquals("foo", value); * } * } * }</pre> * * @param next the promise to replace {@code this} with * @param <O> the type of the value of the replacement promise * @return a promise * @since 1.1 */ default <O> Promise<O> replace(Promise<O> next) { return flatMap(in -> next); }

https://github.com/ratpack/ratpack/blob/1a5d97a011930335fdc97b20a638b6afb070a4a4/ratpack-exec/src/main/java/ratpack/exec/Promise.java#L767-L805

Right, but that's a fresh object, so we don't have value-flow from next to the return value

I mean, sure, but the semantics are the same from a CodeQL perspective, are they not?

Generally we would call aliases value-flow and anything else taint-flow

Okay, I've updated this to be replace;;;Element of Argument[0];Element of ReturnValue;value

java/ql/lib/semmle/code/java/frameworks/ratpack/RatpackExec.qll

Adds models for `ratpack.func.Pair`, and `ratpack.exec.Result`. Improve moels for `ratpack.exec.Promise`. Signed-off-by: Jonathan Leitschuh <Jonathan.Leitschuh@gmail.com>

github-actions · 2021-11-26T14:20:50Z

⚠️ The head of this PR and the base branch were compared for differences in the framework coverage reports. The generated reports are available in the artifacts of this workflow run. The differences will be picked up by the nightly job after the PR gets merged.

Click to show differences in coverage

java

Generated file changes for java

Changes to framework-coverage-java.rst:

-    Others,"``cn.hutool.core.codec``, ``com.esotericsoftware.kryo.io``, ``com.esotericsoftware.kryo5.io``, ``com.fasterxml.jackson.core``, ``com.fasterxml.jackson.databind``, ``com.opensymphony.xwork2.ognl``, ``com.unboundid.ldap.sdk``, ``flexjson``, ``groovy.lang``, ``groovy.util``, ``jodd.json``, ``net.sf.saxon.s9api``, ``ognl``, ``org.apache.commons.codec``, ``org.apache.commons.jexl2``, ``org.apache.commons.jexl3``, ``org.apache.commons.ognl``, ``org.apache.directory.ldap.client.api``, ``org.apache.ibatis.jdbc``, ``org.apache.shiro.codec``, ``org.apache.shiro.jndi``, ``org.codehaus.groovy.control``, ``org.dom4j``, ``org.hibernate``, ``org.jooq``, ``org.mvel2``, ``org.xml.sax``, ``org.xmlpull.v1``, ``play.mvc``, ``ratpack.core.form``, ``ratpack.core.handling``, ``ratpack.core.http``, ``ratpack.exec``, ``ratpack.form``, ``ratpack.func``, ``ratpack.handling``, ``ratpack.http``, ``ratpack.util``",39,99,151,,,,14,18,,
+    Others,"``cn.hutool.core.codec``, ``com.esotericsoftware.kryo.io``, ``com.esotericsoftware.kryo5.io``, ``com.fasterxml.jackson.core``, ``com.fasterxml.jackson.databind``, ``com.opensymphony.xwork2.ognl``, ``com.unboundid.ldap.sdk``, ``flexjson``, ``groovy.lang``, ``groovy.util``, ``jodd.json``, ``net.sf.saxon.s9api``, ``ognl``, ``org.apache.commons.codec``, ``org.apache.commons.jexl2``, ``org.apache.commons.jexl3``, ``org.apache.commons.ognl``, ``org.apache.directory.ldap.client.api``, ``org.apache.ibatis.jdbc``, ``org.apache.shiro.codec``, ``org.apache.shiro.jndi``, ``org.codehaus.groovy.control``, ``org.dom4j``, ``org.hibernate``, ``org.jooq``, ``org.mvel2``, ``org.xml.sax``, ``org.xmlpull.v1``, ``play.mvc``, ``ratpack.core.form``, ``ratpack.core.handling``, ``ratpack.core.http``, ``ratpack.exec``, ``ratpack.form``, ``ratpack.func``, ``ratpack.handling``, ``ratpack.http``, ``ratpack.util``",39,181,151,,,,14,18,,
-    Totals,,175,5369,431,13,6,10,107,33,1,66
+    Totals,,175,5451,431,13,6,10,107,33,1,66

Changes to framework-coverage-java.csv:

- ratpack.exec,,,26,,,,,,,,,,,,,,,,,,,,,,,26
+ ratpack.exec,,,48,,,,,,,,,,,,,,,,,,,,,,,48
- ratpack.func,,,5,,,,,,,,,,,,,,,,,,,,,,,5
+ ratpack.func,,,35,,,,,,,,,,,,,,,,,,,,,,,35
- ratpack.util,,,5,,,,,,,,,,,,,,,,,,,,,,,5
+ ratpack.util,,,35,,,,,,,,,,,,,,,,,,,,,,,35

JLLeitschuh · 2021-11-29T15:02:06Z

Java QLDoc Checks (GitHub Actions)

I see that this is failing, but I can't see the failures because that repository is not public.

atorralba · 2021-11-29T15:30:46Z

Java QLDoc Checks (GitHub Actions)

I see that this is failing, but I can't see the failures because that repository is not public.

I re-launched it because it seemed a transient error. Now it's all green.

JLLeitschuh requested a review from a team as a code owner October 29, 2021 15:43

github-actions bot added documentation Java labels Oct 29, 2021

JLLeitschuh commented Oct 29, 2021

View reviewed changes

java/ql/lib/semmle/code/java/frameworks/ratpack/Ratpack.qll Outdated Show resolved Hide resolved

smowton added the hacktoberfest-accepted label Nov 17, 2021

JLLeitschuh force-pushed the feat/JLL/improve_ratpack_support branch from c4f2d27 to c079914 Compare November 22, 2021 18:45

smowton reviewed Nov 23, 2021

View reviewed changes

JLLeitschuh and others added 2 commits November 25, 2021 12:55

Java: Ratpack HTTP Framework Additional Modeling

1ddf5fb

Adds models for `ratpack.func.Pair`, and `ratpack.exec.Result`. Improve moels for `ratpack.exec.Promise`. Signed-off-by: Jonathan Leitschuh <Jonathan.Leitschuh@gmail.com>

Copyedit change note

36bb84d

JLLeitschuh force-pushed the feat/JLL/improve_ratpack_support branch from 5cc1d98 to 36bb84d Compare November 25, 2021 17:56

smowton approved these changes Nov 26, 2021

View reviewed changes

smowton merged commit 27f40e0 into github:main Nov 29, 2021

JLLeitschuh deleted the feat/JLL/improve_ratpack_support branch November 29, 2021 18:39

		"pushLeft;(Object);;Argument[-1];" + right + " of ReturnValue;value",
		"pushRight;(Object);;Argument[-1];" + left + " of ReturnValue;value",

	// `nextOpIf` accesses qualfier the 'Promise' value and also returns the qualifier
	// `nextOpIf` accesses qualifier the 'Promise' value and also returns the qualifier

Java: Ratpack HTTP Framework Additional Modeling #7007

Java: Ratpack HTTP Framework Additional Modeling #7007

Uh oh!

Conversation

JLLeitschuh commented Oct 29, 2021

Uh oh!

github-actions bot commented Oct 29, 2021

java

Generated file changes for java

Uh oh!

Uh oh!

smowton commented Nov 17, 2021

Uh oh!

smowton commented Nov 17, 2021

Uh oh!

JLLeitschuh commented Nov 17, 2021

Uh oh!

aschackmull commented Nov 19, 2021

Uh oh!

JLLeitschuh commented Nov 22, 2021

Uh oh!

github-actions bot commented Nov 23, 2021

java

Generated file changes for java

Uh oh!

github-actions bot commented Nov 23, 2021

java

Generated file changes for java

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions bot commented Nov 26, 2021

java

Generated file changes for java

Uh oh!

JLLeitschuh commented Nov 29, 2021

Uh oh!

atorralba commented Nov 29, 2021

Uh oh!

Uh oh!