Skip to content

Conversation

@ptziegler
Copy link
Contributor

The changes done with cee4631 cause an exception when URL is passed as an argument which contains improperly escaped characters (most noticeably whitespaces) and therefore causes an URISyntaxException when calling URL.toURI().

This does not occur when using IPath.fromOSString(), which is the approach that was done previously. But instead of converting this path back to an OS-dependent string, it is instead converted to a file, to check whether it exists and to only then convert it back to its String representation.

A test case with an ill-formed URL has been added to avoid a similar problem in the future.

The changes done with cee4631 cause an
exception when an URL is passed as argument, which contains improperly
escaped characters (most noticeably whitespaces) and therefore causes an
URISyntaxException when calling URL.toURI().

This does not occur when using IPath.fromOSString(), which is the
approach that was done previously. But instead of converting this path
back to an OS-dependent string, it is instead converted to a file, to
check whether it exists and to only then convert it back to its String
representation.

A test case with an ill-formed URL has been added to avoid a similar
problem in the future.
@ptziegler ptziegler force-pushed the uri-exception-in-image-descriptor branch from 4031a36 to 33616c6 Compare July 17, 2025 12:18
@github-actions
Copy link
Contributor

Test Results

 2 778 files  ±0   2 778 suites  ±0   1h 38m 17s ⏱️ - 10m 42s
 7 932 tests +1   7 703 ✅ ±0  228 💤 ±0  1 ❌ +1 
23 351 runs  +3  22 604 ✅ +2  746 💤 ±0  1 ❌ +1 

For more details on these failures, see this check.

Results for commit 33616c6. ± Comparison against base commit cee4631.

@ptziegler
Copy link
Contributor Author

ptziegler commented Jul 17, 2025

Test failure is unrelated again: #926

Unless there are any objections, I plan on merging this later today when I'm back home.

@ptziegler ptziegler merged commit 6c10cbf into eclipse-platform:master Jul 17, 2025
16 of 18 checks passed
@ptziegler ptziegler deleted the uri-exception-in-image-descriptor branch July 17, 2025 16:59
@HannesWell
Copy link
Member

HannesWell commented Jul 17, 2025

In general URL/URI handling and conversion to Path can be quite tricky and has a lot of corner-cases. I started to create a Utility that covers all these cases, but didn't have the time yet to finish it:

Once it is finished, it would probably be good to use it here as well.

@ptziegler
Copy link
Contributor Author

In general URL/URI handling and conversion to Path can be quite tricky and has a lot of corner-cases. I started to create a Utility that covers all these cases, but didn't have the time yet to finish it:

* [Add manual trigger for 'updateRelease'-workflow and use project-bot eclipse-equinox/equinox#702](https://github.com/eclipse-equinox/equinox/pull/702)

Once it is finished, it would probably be good to use it here as well.

I assume you got the numbers mixed up and meant eclipse-equinox/equinox#691?

@HannesWell
Copy link
Member

I assume you got the numbers mixed up and meant eclipse-equinox/equinox#691?

Yes, indeed. Thanks for the correction.
Please, note there is also eclipse-equinox/equinox#920. but I intend to combine both into one which simplifies development.

@HeikoKlare
Copy link
Contributor

Thank you for the quick fix, @ptziegler!

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.

3 participants