Skip to content
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

[JENKINS-74026][JENKINS-74027] Improve CSP compatibility #380

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

yaroslavafenkin
Copy link

@yaroslavafenkin yaroslavafenkin commented Nov 7, 2024

  • Extract inline JavaScript from DynamicReferenceParameter/index.jelly
  • Extract inline JavaScript from CascadeChoiceParameter/index.jelly

https://issues.jenkins.io/browse/JENKINS-74026
https://issues.jenkins.io/browse/JENKINS-74027

Testing done

I've relied a lot on the scenario from https://issues.jenkins.io/browse/JENKINS-71909. Made sure that isn't broken. Unit test that asserts that behavior is very much appreciated, helped me a lot along the way. I feel like I'm almost over relying on it though.

Before the fix
After the fix

Submitter checklist

  • Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
  • Ensure that the pull request title represents the desired changelog entry
  • Please describe what you did
  • Link to relevant issues in GitHub or Jira
  • Link to relevant pull requests, esp. upstream and downstream changes
  • Ensure you have provided tests - that demonstrates feature works or fixes the issue

@yaroslavafenkin yaroslavafenkin requested a review from a team as a code owner November 7, 2024 15:43
@yaroslavafenkin yaroslavafenkin changed the title [JENKINS-74027] Uxtract inline JavaScript from CascadeChoiceParameter/index.jelly [JENKINS-74026][JENKINS-74027] Improve CSP compatibility Nov 7, 2024
@yaroslavafenkin
Copy link
Author

While working on fixing https://issues.jenkins.io/browse/JENKINS-74026 individually I realized it makes more sense to fix both issues together since I'll apply the same approach for both of them. Videos with explanations do not cover that part of the fix. It has been tested manually the same way, I've just changed the last parameter from Active Choices Reactive Parameter to Active Choices Reactive Reference Parameter.

@@ -149,7 +149,7 @@ public void testReferenceParameterXss() throws IOException, SAXException {
"random-name",
script,
DynamicReferenceParameter.ELEMENT_TYPE_FORMATTED_HTML,
null,
"",
Copy link
Author

Choose a reason for hiding this comment

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

Never a null when saved via UI.

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.

1 participant