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

AnsiOutput.detectIfAnsiCapable broken on JDK22 #40160

Closed
kilink opened this issue Apr 3, 2024 · 7 comments
Closed

AnsiOutput.detectIfAnsiCapable broken on JDK22 #40160

kilink opened this issue Apr 3, 2024 · 7 comments
Assignees
Labels
status: superseded An issue that has been superseded by another type: bug A general bug

Comments

@kilink
Copy link
Contributor

kilink commented Apr 3, 2024

The detectIfAnsiCapable method in AnsiOutput does not work correctly on JDK22. It checks if System.console() returns null, but on JDK22 System.console() never returns null; there's a new method on Console to detect if the instance is a terminal or not.

Some details here about the change:

System.console() now returns a Console object when the standard streams are redirected or connected to a virtual terminal. Prior to JDK 22, System.console() instead returned null for these cases. This change may impact code that checks the return from System.console() to test if the JVM is connected to a terminal. If required, the -Djdk.console=java.base flag will restore the old behavior where the console is only returned when it is connected to a terminal. Starting JDK 22, one could also use the new Console.isTerminal() method to test if the console is connected to a terminal.

The result is that on JDK22, color / ANSI output is turned on when it shouldn't be.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Apr 3, 2024
@scottfrederick scottfrederick added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged labels Apr 3, 2024
@scottfrederick scottfrederick added this to the 3.1.x milestone Apr 3, 2024
@facewise
Copy link
Contributor

facewise commented Apr 4, 2024

@scottfrederick @kilink
Hello. Can I pick up this issue?

@philwebb
Copy link
Member

philwebb commented Apr 4, 2024

@facewise Feel free to have a go, but we're not 100% sure how to fix it since we need to keep our build on Java 17. We might be able to use reflection to make the call to Console.isTerminal().

It's also going to be pretty hard to test. We might be able to create a testcontainers test, or we might just have to accept manual testing for now.

Please ask if you have any questions.

@facewise
Copy link
Contributor

facewise commented Apr 4, 2024

@philwebb
Is it OK to add a testcontainers test in spring-boot-tests/spring-boot-integration-tests?

It seems great to make a new test similar to tests in LoaderIntegrationTests.

@philwebb
Copy link
Member

philwebb commented Apr 4, 2024

@facewise That sounds fine, we can always move it around when we merge the code if we find a better long-term home.

@wilkinsona
Copy link
Member

I think we may be able to test it by reverting most of 8efdc1e.

@facewise
Copy link
Contributor

@philwebb
I made a PR of it. Please check it out.

Should I accept the reversion @wilkinsona said?

@wilkinsona wilkinsona modified the milestones: 3.1.x, 3.2.x Apr 22, 2024
@scottfrederick
Copy link
Contributor

Closing in favor of #40172

@scottfrederick scottfrederick closed this as not planned Won't fix, can't repro, duplicate, stale Apr 24, 2024
@wilkinsona wilkinsona removed this from the 3.2.x milestone Apr 25, 2024
@wilkinsona wilkinsona added the status: superseded An issue that has been superseded by another label Apr 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: superseded An issue that has been superseded by another type: bug A general bug
Projects
None yet
Development

No branches or pull requests

6 participants