Skip to content

[KARAF-6321] - add configuration options to protect against CTRL-D/shell:logout exits#1627

Closed
awrb wants to merge 2 commits intoapache:mainfrom
awrb:KARAF-6321
Closed

[KARAF-6321] - add configuration options to protect against CTRL-D/shell:logout exits#1627
awrb wants to merge 2 commits intoapache:mainfrom
awrb:KARAF-6321

Conversation

@awrb
Copy link
Copy Markdown
Contributor

@awrb awrb commented Sep 24, 2022

No description provided.

awrb
awrb commented Sep 24, 2022
View reviewed changes
shell/core/src/main/java/org/apache/karaf/shell/impl/console/ConsoleSessionImpl.java
command = null;
boolean disableEofExit = (boolean) session.get(Session.DISABLE_EOF_EXIT);
if (disableEofExit) {
command = "";
Copy link
Copy Markdown
Contributor Author

@awrb awrb Sep 24, 2022

Choose a reason for hiding this comment

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

I tested it manually and it works fine but I couldn't figure out a way to write a test for this (e.g. how to send CTRL-D to Karaf in a pax exam test, or some other way)

Copy link
Copy Markdown
Member

@jbonofre jbonofre Oct 5, 2022

Choose a reason for hiding this comment

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

That's actually pretty hard in pax-exam as it controls the lifecycle. Manual test is fine and enough IMHO.

@jbonofre jbonofre self-requested a review September 25, 2022 04:41
jbonofre
jbonofre reviewed Sep 25, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@jbonofre jbonofre left a comment

Choose a reason for hiding this comment

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

Overall, it looks good (I will take a deeper look). Do you mind to update the document (in the manual module) ?

@awrb
Copy link
Copy Markdown
Contributor Author

awrb commented Sep 25, 2022 •
edited
Loading

Sure @jbonofre I will do it in a couple of hours. Actually some of my other pull requests might also need a change in the manual.
Do you have any ideas how to code a test for the disabled EOF?

Overall, it looks good (I will take a deeper look). Do you mind to update the document (in the manual module) ?

@awrb
Copy link
Copy Markdown
Contributor Author

awrb commented Sep 25, 2022

I updated console.adoc and remote.adoc in manual.

jbonofre
jbonofre approved these changes Oct 12, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@jbonofre jbonofre left a comment

Choose a reason for hiding this comment

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

Thanks for the documentation update! I will squash and merge.

@asfgit asfgit closed this in 32cb122 Oct 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jbonofre jbonofre jbonofre approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@awrb @jbonofre