Skip to content

[JENKINS-48925] - Whitelist safe model classes from the tap4j library#20

Merged
kinow merged 2 commits intojenkinsci:masterfrom
oleg-nenashev:bug/JENKINS-48925-jep-200
Jan 25, 2018
Merged

[JENKINS-48925] - Whitelist safe model classes from the tap4j library#20
kinow merged 2 commits intojenkinsci:masterfrom
oleg-nenashev:bug/JENKINS-48925-jep-200

Conversation

@oleg-nenashev
Copy link
Copy Markdown
Member

It makes the Tap Plugin to pass existing tests against Jenkins 2.102 in Plugin Compat Tester. I do not think it is enough in general, because TapElement in that class includes the following object:

    /**
     * Iterable object returned by SnakeYAML.
     */
    private Map<String, Object> diagnostic = new LinkedHashMap<String, Object>();

I am not sure when this map contains restricted classes, but from Jenkins JEP-200 PoV is seems to be a dangerous construct.

https://issues.jenkins-ci.org/browse/JENKINS-48925

@reviewbybees @kinow @jglick

@oleg-nenashev oleg-nenashev requested review from jglick and kinow January 15, 2018 18:15
@ghost
Copy link
Copy Markdown

ghost commented Jan 15, 2018

This pull request originates from a CloudBees employee. At CloudBees, we require that all pull requests be reviewed by other CloudBees employees before we seek to have the change accepted. If you want to learn more about our process please see this explanation.

Copy link
Copy Markdown
Member

@jglick jglick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, assuming you actually looked at these classes and confirm they look safe.

FindBugs is failing but presumably that is also true in master. Needs to be suppressed for now.

@oleg-nenashev
Copy link
Copy Markdown
Member Author

sorry, forgot to push the FindBugs commit

@oleg-nenashev
Copy link
Copy Markdown
Member Author

Yes, all classes have been verified

@oleg-nenashev
Copy link
Copy Markdown
Member Author

@reviewbybees done

@oleg-nenashev
Copy link
Copy Markdown
Member Author

@kinow Please let me know if you need any additional info to review/release it. Currently the plugin is broken for Jenkins 2.102+

@kinow
Copy link
Copy Markdown
Member

kinow commented Jan 18, 2018

Thanks @oleg-nenashev ! I didn't follow what happened in JEP-200, so I want to finish reading it in order to review and test it.

I assume once this is done, and the pull request has been merged, we should release a new version? Or are there other changes required?

@oleg-nenashev
Copy link
Copy Markdown
Member Author

@kinow no other changes needed, I hope. Though in Testlink plugin I missed the TestNG support logic during the first iteration (see jenkinsci/testlink-plugin#29).

But yes, this patch should close some issues at least

@dreis2211
Copy link
Copy Markdown

Hi @oleg-nenashev ? Any ETA when this could be released?

@oleg-nenashev
Copy link
Copy Markdown
Member Author

@dreis2211 It depends on @kinow . I proposed a fix, but I do not maintain the plugin. If you need a quick fix, you can take the HPI file from the pull request builder

@kinow
Copy link
Copy Markdown
Member

kinow commented Jan 25, 2018

Just released 2.2. Should be available through the update centre in the next hours.

Thanks a lot @oleg-nenashev !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants