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

URL encoding issue in test report links with square brackets #659

Closed
ahadalioglu opened this issue Nov 6, 2024 · 11 comments · Fixed by #660
Closed

URL encoding issue in test report links with square brackets #659

ahadalioglu opened this issue Nov 6, 2024 · 11 comments · Fixed by #660

Comments

@ahadalioglu
Copy link

Jenkins and plugins versions report

Environment
Jenkins: 2.479.1
OS: Linux - 6.1.112-124.190.amzn2023.x86_64
Java: 17.0.13 - Amazon.com Inc. (OpenJDK 64-Bit Server VM)
---
<plugins_omitted>
junit:1307.vdd5b_2646279e
<plugins_omitted>

What Operating System are you using (both controller, and any agents involved in the problem)?

After upgrading Jenkins from version 2.462.3 to 2.479.1, all test result/report links containing square brackets ([ and ]) in the URL now fail with the following error:

URI: /badURI
STATUS: 400
MESSAGE: Illegal Path Character

However, if the square brackets are manually replaced with percent encodings (%5B for [ and %5D for ]), the links work correctly.

Reproduction steps

Access test results/reports via link:
https://<jenkins_url>/job/<job_name>/<build_number>/testReport/(root)/[RESULTS]

Expected Results

Convert link to this:
https://<jenkins_url>/job/<job_name>/<build_number>/testReport/(root)/%5BRESULTS%5D

Actual Results

HTTP ERROR 400 Illegal Path Character

URI: /badURI
STATUS: 400
MESSAGE: Illegal Path Character

Powered by Jetty:// 12.0.13

Anything else?

No response

Are you interested in contributing a fix?

No response

@timja
Copy link
Member

timja commented Nov 6, 2024

Did you also change JUnit versions at all or just Jenkins core versions?

@timja
Copy link
Member

timja commented Nov 6, 2024

cc @daniel-beck / @Wadeck in case you are aware of a security change that might have affected this?

@ahadalioglu
Copy link
Author

I typically update all plugins available for update after a Jenkins core update or upgrade, so I don’t recall the exact timing of my last JUnit plugin update. However, I’m currently using the latest version, and the previous version listed next to the downgrade button is 1300.v03d9d8a_cf1fb_.

As a side note, I'm fairly certain I updated all available plugins on or after October 17th. Therefore, it seems that I haven’t updated the JUnit plugin recently, as all previous JUnit releases are over a month old (ref. JUnit plugin releases).

@basil
Copy link
Member

basil commented Nov 7, 2024

Likely a side effect of jenkinsci/jenkins#9590, as Jetty 12 is more strict than Jetty 10 was about such URLs. See related discussion in the subtasks of JENKINS-73120. If the plugin is working when the links are URL-encoded, then I think the most practical option would be to simply generate URL-encoded links in the first place rather than the current (broken) links.

@timja
Copy link
Member

timja commented Nov 7, 2024

Do you know which one specifically? There's a lot of subtasks

@basil
Copy link
Member

basil commented Nov 7, 2024

JENKINS-73128 and JENKINS-73129 in particular, which resulted in changes to some tests in jenkinsci/jenkins#9590. The long and short of it is that the newer servlet specification is more strict than earlier versions about rejecting suspicious URLs, and newer versions of Jetty comply. The security team was generally positive about this increased level of protection, but this is the first case we have heard of where it is negatively impacting users. But it sounds like if we just generate URL-encoded links then this problem should be solvable.

@basil basil mentioned this issue Nov 7, 2024
6 tasks
@basil
Copy link
Member

basil commented Nov 7, 2024

URL-encoding links to tests as in #660 seems to fix it 🤷

@Wadeck
Copy link
Contributor

Wadeck commented Nov 8, 2024

in case you are aware of a security change that might have affected this?

Nope sorry. The hypothesis from Basil seems convincing to me 👍

@basil basil closed this as completed in #660 Nov 8, 2024
@basil
Copy link
Member

basil commented Nov 8, 2024

Fixed in #660. Released in 1308.vb_90591b_eb_996.

@timja timja reopened this Nov 9, 2024
@timja
Copy link
Member

timja commented Nov 9, 2024

see #660 (comment)

@basil
Copy link
Member

basil commented Nov 9, 2024

I released 1309.v0078b_fecd6ed using hudson.Util#rawEncode to fix the %20 encoding issue. Please file a separate ticket for any other screens that are affected, ideally with a PR that resolves the issue.

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 a pull request may close this issue.

4 participants