-
Notifications
You must be signed in to change notification settings - Fork 194
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
Don't await a StackOverflowError on a redirection loop #4452
base: main
Are you sure you want to change the base?
Conversation
In case of a redirection loop for URI of a p2 repository, fail fast on a redirection loop. In this case provide a warning to the user and let the rest be handled by Tycho. Otherwise there might be a StackOverflowError due to the recursion used and no check for whether the base URI is the same as the redirected URI. This links to eclipse-tycho#4451 but does not fix it, only raising visibility for such cases.
Test Results 603 files 603 suites 4h 25m 32s ⏱️ Results for commit 2c47d69. ♻️ This comment has been updated with latest results. |
Any chance you can enhance the existing integration test to show the behavior? |
@laeubi which one is the corresponding one? I was thinking about creating a new one with a web server redirecting to itself to mimic this. But I haven't finished yet reading the documentation of Jetty ^^ |
If you search for HttpServer references in the test you should find some test that already do something in that direction. |
Hi @laeubi, sorry for the late response. I will take some time later today to create an IT or to update a fitting existing one. |
Sadly, for whatever reason I wasn't able to run the ITs from within Eclipse itself due to the launch configuration I have to await the CI and then wait and see. |
In eclipse you can simpel right click and choose to run as unit test. But due to the nature of tycho/maven it is usually better to use maven CLI with remote debugging. For this simply do
|
Hey @laeubi, I have been trying for a few days now to create an IT for this behavior but with no success. Taking a look at all P2-related ITs, I could not really find a way to hook into one, so I created one on my own. But even having a Jetty where the |
@thahnen can you maybe share what "did not work"? Maybe just upload the test and explain what you wanted to behave differently and we can probably find a solution. |
Good idea! Currently OOF, will do so once I‘m back again. |
In case of a redirection loop for URI of a p2 repository, fail fast on a redirection loop. In this case provide a warning to the user and let the rest be handled by Tycho. Otherwise there might be a StackOverflowError due to the recursion used and no check for whether the base URI is the same as the redirected URI.
This links to #4451 but does not fix it, only raising visibility for such cases.