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

Fix URL validation and handling in URLUtil #12800

Open
wants to merge 36 commits into
base: main
Choose a base branch
from

Conversation

sfahaddev
Copy link

Closes #12186

Describe the changes you have made here: what, where, why, ...

  • What: Fixed URL validation and handling in the URLUtil class.
  • Where: Updated the isURL and create methods in URLUtil and added/modified tests in URLUtilTest.
  • Why: To resolve the issue where relative URLs (e.g., www.example.com) were not being handled correctly, and to ensure proper validation of URLs with various protocols (e.g., http://, https://, ftp://, file://).

Changes:

  1. isURL Method:

    • Removed unnecessary exception handling (MalformedURLException and IllegalArgumentException).
    • Now only catches URISyntaxException to correctly identify invalid URLs.
    • Ensures that only absolute URLs are considered valid.
  2. create Method:

    • Added logic to prepend http:// only to URLs without a protocol (e.g., www.example.comhttp://www.example.com).
    • Ensures that URLs with existing protocols (e.g., ftp://example.com, file:///path/to/file) are not modified.
  3. Tests:

    • Added new test cases for relative URLs, absolute URLs, and other protocols.
    • Fixed normalization issue for file URLs in tests (e.g., file:///path/to/filefile:/path/to/file).

Mandatory checks

  • I own the copyright of the code submitted and I license it under the MIT license
  • Change in CHANGELOG.md described in a way that is understandable for the average user (if change is visible to the user)
  • Tests created for changes (if applicable)
  • Manually tested changed features in running JabRef (always required)
  • Screenshots added in PR description (if change is visible to the user)
  • Checked developer's documentation: Is the information available and up to date? If not, I outlined it in this pull request.
  • Checked documentation: Is the information available and up to date? If not, I created an issue at https://github.com/JabRef/user-documentation/issues or, even better, I submitted a pull request to the documentation repository.

@sfahaddev sfahaddev force-pushed the fix-for-issue-12186 branch from 33a8fbf to e060945 Compare March 22, 2025 22:13
@sfahaddev sfahaddev force-pushed the fix-for-issue-12186 branch from 05de4cd to 4ea9e06 Compare March 23, 2025 22:34
@sfahaddev sfahaddev force-pushed the fix-for-issue-12186 branch from 04b92fe to eaf63e3 Compare March 23, 2025 23:16
@JabRef JabRef deleted a comment from github-actions bot Mar 24, 2025
@sfahaddev sfahaddev requested review from Siedlerchr and koppor March 24, 2025 23:28
@JabRef JabRef deleted a comment from koppor Mar 24, 2025
@sfahaddev sfahaddev requested review from koppor and InAnYan March 26, 2025 23:44
@sfahaddev sfahaddev requested a review from koppor March 30, 2025 13:36
Copy link

trag-bot bot commented Apr 1, 2025

@trag-bot didn't find any issues in the code! ✅✨

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.

IllegalArgumentException: URI is not absolute
4 participants