[JENKINS-47736] Introduced ClassFilter.setDefault #208

jglick · 2017-10-30T18:12:40Z

JENKINS-47736

@reviewbybees

oleg-nenashev · 2017-11-20T04:52:17Z

@jglick It requires the JEP review, right? What is the current state of this process?

jglick · 2017-11-27T16:38:24Z

I am not really sure.

jglick · 2017-11-28T13:35:29Z

@oleg-nenashev FYI

…
00:11:50.494 [linux] Testing factory JNLP4-plaintext
00:37:30.425 [linux] Cancelling nested steps due to timeout
…

oleg-nenashev

All the changes introduced here are reasonable even without JEP-200. The most interesting part of this JEP is changing the default behavior in the core. Blacklist filters just move the de-facto existing restrictions from the core.

Assuming the documentation gets polished a bit, I see no strict need to wait till JEP-200 is fully approved. So just a minor change requirement regarding the documentation.

oleg-nenashev · 2017-12-18T23:58:20Z

src/main/java/hudson/remoting/ClassFilter.java

    public static final String FILE_OVERRIDE_LOCATION_PROPERTY = "hudson.remoting.ClassFilter.DEFAULTS_OVERRIDE_LOCATION";

    private static final Logger LOGGER = Logger.getLogger(ClassFilter.class.getName());

-    protected boolean isBlacklisted(String name) {
+    public boolean isBlacklisted(String name) {


Some Javadoc here would be helpful

oleg-nenashev · 2017-12-18T23:58:36Z

src/main/java/hudson/remoting/ClassFilter.java

        return false;
    }

-    protected boolean isBlacklisted(Class c) {
+    public boolean isBlacklisted(Class c) {


oleg-nenashev · 2017-12-18T23:59:56Z

src/main/java/hudson/remoting/ClassFilter.java

@@ -15,6 +15,7 @@
 import java.util.regex.PatternSyntaxException;

 import javax.annotation.CheckForNull;
+import javax.annotation.Nonnull;

 /**
 * Restricts what classes can be received through remoting.


IIUC with JEP-200 the same ClassFilter will be also applied to XStream. I recommend documenting it here

It already was applied to XStream, but yes it should be documented anyway.

oleg-nenashev · 2017-12-19T00:01:33Z

src/main/java/hudson/remoting/ClassFilter.java

+    /**
+     * Changes the effective value of {@link #DEFAULT}.
+     * @param filter a new default to set; may or may not delegate to {@link STANDARD}
+     */


@since TODO

I think addressed.

oleg-nenashev

🐝 . As discussed in JEP-200, I will probably merge it ahead Remoting 3.16 even if the JEP is not approved. This part just enhances API and does not break anything.

jglick · 2017-12-22T16:37:24Z

It mostly just adds clearer APIs while leaving the deprecated appendDefaultFilter with the same behavior. My concern would be a few additions to the default blacklist. SignedObject is already being added by core, but Method and json-lib would be behavioral changes if integrated. In particular this last change would cause a functional regression until jenkinsci/dockerhub-notification-plugin#16 is released. CC @rsandell who seems to be active in this plugin.

Since there is no advantage to releasing the new API ahead of the core PR which uses it, I would still suggest doing them together, but up to you ultimately. @reviewbybees done anyway

oleg-nenashev · 2017-12-26T08:02:21Z

Agreed, the plugins need to be released ahead at least

oleg-nenashev · 2017-12-28T16:23:20Z

I have merged the PR with the current master and deployed the new snapshot: remoting-3.16-20171228.162243-1

rsandell · 2018-01-04T15:56:19Z

dockerhub-notification-2.2.1 released today.

jglick · 2018-01-08T14:43:15Z

Saw that, thanks @rsandell.

oleg-nenashev · 2018-01-09T11:26:55Z

Windows build is unstable due to the INFRA issue AFAICT:

[INFO] --- maven-jar-plugin:3.0.2:jar (default-jar) @ remoting ---

[INFO] Downloading: https://repo.maven.apache.org/maven2/org/apache/maven/maven-archiver/3.1.1/maven-archiver-3.1.1.pom

Jan 09, 2018 9:40:02 AM org.apache.maven.wagon.providers.http.httpclient.impl.execchain.RetryExec execute

INFO: I/O exception (java.net.SocketException) caught when processing request to {s}->https://repo.maven.apache.org:443: Connection reset

Jan 09, 2018 9:40:02 AM org.apache.maven.wagon.providers.http.httpclient.impl.execchain.RetryExec execute

INFO: Retrying request to {s}->https://repo.maven.apache.org:443

Jan 09, 2018 9:40:21 AM org.apache.maven.wagon.providers.http.httpclient.impl.execchain.RetryExec execute

INFO: I/O exception (java.net.SocketException) caught when processing request to {s}->https://repo.maven.apache.org:443: Connection reset

Jan 09, 2018 9:40:21 AM org.apache.maven.wagon.providers.http.httpclient.impl.execchain.RetryExec execute

INFO: Retrying request to {s}->https://repo.maven.apache.org:443

Jan 09, 2018 9:40:40 AM org.apache.maven.wagon.providers.http.httpclient.impl.execchain.RetryExec execute

INFO: I/O exception (java.net.SocketException) caught when processing request to {s}->https://repo.maven.apache.org:443: Connection reset

Jan 09, 2018 9:40:40 AM org.apache.maven.wagon.providers.http.httpclient.impl.execchain.RetryExec execute

INFO: Retrying request to {s}->https://repo.maven.apache.org:443

Windows tests are green, so I do not care much: [windows] [WARNING] Tests run: 515, Failures: 0, Errors: 0, Skipped: 6

oleg-nenashev · 2018-01-09T16:08:40Z

JEP-200 has been accepted. 🚢 🇮🇹

@SInCE

* Changelog: Noting 3.16 * Javadoc: Add @SInCE to the ClassFilter#setDefault() meethod * Changelog: We already know the target Jenkins weekly release * Changelog: Fix the reference to #208

[JENKINS-47736] Introduced ClassFilter.setDefault.

079a803

jglick added the work-in-progress label Oct 30, 2017

jglick mentioned this pull request Oct 30, 2017

[JENKINS-47736] Switch Remoting/XStream blacklist to a whitelist jenkinsci/jenkins#3120

Merged

jglick requested a review from oleg-nenashev October 30, 2017 19:07

Merge branch 'master' into whitelist-JENKINS-47736

f6cddd0

Merge branch 'master' into whitelist-JENKINS-47736

56ff2d7

Merge branch 'master' into whitelist-JENKINS-47736

c321bd3

oleg-nenashev previously requested changes Dec 19, 2017

View reviewed changes

jglick added 2 commits December 19, 2017 14:30

Merge branch 'master' into whitelist-JENKINS-47736

24132a1

Review comments from @oleg-nenashev.

6ce3547

jglick requested a review from oleg-nenashev December 19, 2017 19:41

oleg-nenashev approved these changes Dec 22, 2017

View reviewed changes

oleg-nenashev added ready-to-merge and removed work-in-progress labels Dec 22, 2017

oleg-nenashev added the on-hold label Dec 26, 2017

Merge branch 'master' into whitelist-JENKINS-47736

cf82efa

[JENKINS-47736] - Add some annotations, mostly to kick-off CI

ded8f03

oleg-nenashev removed the on-hold label Jan 9, 2018

oleg-nenashev merged commit 1fda115 into jenkinsci:master Jan 9, 2018

jglick deleted the whitelist-JENKINS-47736 branch January 9, 2018 19:48

oleg-nenashev added a commit to oleg-nenashev/remoting that referenced this pull request Jan 10, 2018

Changelog: Fix the reference to jenkinsci#208

aa91e33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[JENKINS-47736] Introduced ClassFilter.setDefault #208

[JENKINS-47736] Introduced ClassFilter.setDefault #208

jglick commented Oct 30, 2017

oleg-nenashev commented Nov 20, 2017

jglick commented Nov 27, 2017

jglick commented Nov 28, 2017

oleg-nenashev left a comment

oleg-nenashev Dec 18, 2017

oleg-nenashev Dec 18, 2017

oleg-nenashev Dec 18, 2017

jglick Dec 19, 2017

oleg-nenashev Dec 19, 2017

oleg-nenashev left a comment

jglick commented Dec 22, 2017

oleg-nenashev commented Dec 26, 2017

oleg-nenashev commented Dec 28, 2017

rsandell commented Jan 4, 2018

jglick commented Jan 8, 2018

oleg-nenashev commented Jan 9, 2018

oleg-nenashev commented Jan 9, 2018

[JENKINS-47736] Introduced ClassFilter.setDefault #208

[JENKINS-47736] Introduced ClassFilter.setDefault #208

Conversation

jglick commented Oct 30, 2017

oleg-nenashev commented Nov 20, 2017

jglick commented Nov 27, 2017

jglick commented Nov 28, 2017

oleg-nenashev left a comment

Choose a reason for hiding this comment

oleg-nenashev Dec 18, 2017

Choose a reason for hiding this comment

oleg-nenashev Dec 18, 2017

Choose a reason for hiding this comment

oleg-nenashev Dec 18, 2017

Choose a reason for hiding this comment

jglick Dec 19, 2017

Choose a reason for hiding this comment

oleg-nenashev Dec 19, 2017

Choose a reason for hiding this comment

oleg-nenashev left a comment

Choose a reason for hiding this comment

jglick commented Dec 22, 2017

oleg-nenashev commented Dec 26, 2017

oleg-nenashev commented Dec 28, 2017

rsandell commented Jan 4, 2018

jglick commented Jan 8, 2018

oleg-nenashev commented Jan 9, 2018

oleg-nenashev commented Jan 9, 2018