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-encode links to tests #660

Merged
merged 1 commit into from
Nov 8, 2024
Merged

URL-encode links to tests #660

merged 1 commit into from
Nov 8, 2024

Conversation

basil
Copy link
Member

@basil basil commented Nov 7, 2024

Fixes #659

Testing done

  • mvn clean verify
  • Reproduced the problem described in the ticket in Firefox and 2.479.1 before this PR, could no longer reproduce after this PR
  • Attempted but failed to write an automated test with HtmlUnit, as I could not reproduce the problem in HtmlUnit but could in Firefox

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

@basil
Copy link
Member Author

basil commented Nov 7, 2024

@jenkinsci/core-security-review Any objection to this? I can't think of a reason why URL-encoding the test name would be harmful, but maybe I'm missing something here.

Copy link
Member

@timja timja left a comment

Choose a reason for hiding this comment

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

LGTM

@basil basil merged commit b90591b into jenkinsci:master Nov 8, 2024
17 checks passed
@ahadalioglu
Copy link

ahadalioglu commented Nov 9, 2024

Thanks to everyone who contributed to fixing this bug.

Links under 'All Tests' are now percent-encoded, but spaces are being replaced with + signs, which breaks the links. Manually replacing + with spaces or %20 makes them work again.

Meanwhile, links under 'All Failed Tests' are still preserving square brackets ([]) as-is.

NOTE: Previously, only square brackets were an issue, and spaces were preserved.

@ahadalioglu
Copy link

Thank you to everyone who worked on resolving this issue. I can confirm that the "All Tests" link now functions as expected, with all associated links working correctly.

However, links for individual failed test reports (All Failed Tests) are still encountering the original issue: square brackets in the URLs are not being replaced with percent-encodings, which still results in link failures. I've attached a screenshot for additional context:
JUnit - SS

Thank you for your continued efforts on this.

@timja
Copy link
Member

timja commented Nov 11, 2024

see #659 (comment)

@basil basil deleted the squarebrackets branch November 11, 2024 23:48
@ahadalioglu
Copy link

ahadalioglu commented Nov 14, 2024

As I understand it, there's nothing more that can be done regarding the last report, correct? Then we'll rely solely on the 'All Tests' section and avoid using the 'All Failed Tests' section.

Thank you very much, though.
I really appreciate your prompt assistance so far.

@basil
Copy link
Member Author

basil commented Nov 14, 2024

As requested previously, please file a separate ticket for any other places where the URL is not properly encoded, ideally with a PR that resolves the issue.

@jenkinsci jenkinsci locked as off-topic and limited conversation to collaborators Nov 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

URL encoding issue in test report links with square brackets
3 participants