Development of `EchoShell` - A simple shell emulator #39

gregorFeigel · 2023-06-19T09:14:35Z

Development of a shell emulator EchoShell that outputs the he user input and offers some basic commands like: help, history, clear, whoami, date and exit. In addition, a host key file handler was added.

The ShellDelegate and SSHShellContext changes provided by the repo owner have been adapted.

…ser input and offers some basic commands like: help, history, clear, whoami, date and exit.

Joannis · 2023-06-20T15:12:40Z

Package.swift

@@ -32,6 +33,7 @@ let package = Package(
                .product(name: "_CryptoExtras", package: "swift-crypto"),
                .product(name: "BigInt", package: "BigInt"),
                .product(name: "Logging", package: "swift-log"),
+                .productItem(name: "ColorizeSwift", package: "ColorizeSwift")


While I understand the desire to import this library - I don't want to add a dependency for this.

Ok, then I'll embed the necessary code parts of the library and mention the author above?

Joannis · 2023-06-20T15:13:15Z

Sources/Citadel/HostKeyFile.swift

+import Crypto
+import NIOSSH
+
+public extension NIOSSHPrivateKey {


These helpers cannot go in the library. But you can add them to your extension.

Would you consider to add this as an default directly to NIOSSHPrivateKey?

Joannis · 2023-06-20T15:13:36Z

Sources/Citadel/HostKeyFile.swift

+import NIOSSH
+
+public extension NIOSSHPrivateKey {
+    init(file: URL = FileManager.default.homeDirectoryForCurrentUser.appendingPathComponent("citadel_ssh_host_key")) throws {


The URL should not use this default. It's best to leave the defaults empty

Joannis · 2023-06-20T15:16:54Z

Sources/Citadel/Shell/EchoShell/BasicCommands.swift

+// Basic Commands
+// Each command must return an array of UINT8 or nil. Streams are currently not supported.
+
+public protocol Command {


While I understand and agree with the desire for these helpers and standardisation, I don't think this can be used as public API in Citadel itself. Though feel free to add it as part of the Example project for now.

I think they're very helpful, but would need to go in their own module - separate from the Citadel module itself. Though they can be in the same repo.

Joannis · 2023-06-20T15:18:12Z

Sources/Citadel/Shell/EchoShell/BasicCommands.swift

+public protocol Command {
+    var name: String { get }
+    var description: String { get }
+    func exec(input: [UInt8], session: EchoShellMaster) async throws -> [UInt8]?


Before we merge it as public API (so outside of an example), this exec function signature ha to change.

First of all, the EchoShellMaster type is very much an 'example' name. If we can rename this to CommandContext that'd be better.

The input should also be more helpful than [UInt8]. The 'bag of bytes' type we use in NIO (ByteBuffer) would definitely be preferred over this. But I think wrapping that ByteBuffer into a CommandInput type is better. That way we can add helpers.

public struct CommandInput: Sendable { private var buffer: ByteBuffer } extension CommandInput { func readString() -> String? { return buffer.getString(at: buffer.startIndex, length: buffer.readableBytes) } }

We should ensure there's room for improvement here, so that we can accept streams of data as well.

In addition to accepting streams of data, we should also allow the func exec to emit a stream of data. Or write data to the client in some way. Because you might have flexible input over time, or a form of subprocesses-commands.

I'm fine with changing the name to CommandContext and offering CommandInput with the ByteBuffer.
I personally tend to avoid working with strings at cmd-line level as basically everything is within the UTF-8/ASCI range and String just adds a lot of unnecessary overhead.

I'm happy with changing the return to an AsyncStream .

Joannis · 2023-06-20T15:32:19Z

Sources/Citadel/Shell/EchoShell/EchoShell.swift

+    private var previous_length: Int = 0
+
+    // Adds the input if it is not equal to the last one. This makes it easier to search the history.
+    mutating func add_line(_ line: [UInt8]) {


Maybe we can make a typealias here for a Line. Or a separate type.

do we agree on [UINT8] or ByteBuffer? I do like that you directly can iterate over [UINT8] and use all array/collection defaults. Are they available for ByteBuffer as well?

ByteBuffer, but you can use the readableBytesView in ByteBuffer for that

Joannis · 2023-06-20T15:33:04Z

Sources/Citadel/Shell/EchoShell/EchoShell.swift

+    mutating func go_back(currentLineCount: Int) -> ([UInt8], [UInt8])? {
+        let index = (history.count - 1) - (pointer + 1)
+
+        if index >= 0 && index <= history.count - 1 {


this if can be turned into a guard. that way we keep the code's control flow clean

Joannis · 2023-06-20T15:34:43Z

Sources/Citadel/Shell/EchoShell/EchoShell.swift

+        return nil
+    }
+
+    mutating func go_forward(currentLineCount: Int) -> ([UInt8], [UInt8])? {


Can we make a type for this tuple?

Joannis · 2023-06-20T15:35:13Z

Sources/Citadel/Shell/EchoShell/EchoShell.swift

+    }
+
+    // `Zero` is needed to reset the internal pointer when pressing the Enter key.
+    mutating func zero() {


mutating func return()

Note that the enter key is the return key. Hence that CR character is Carriage Return

Yes, zero() is named zero for convenience reasons, as it sets the counters to original (zero) state, but I'm fine with return().

Joannis · 2023-06-20T15:36:51Z

Sources/Citadel/Shell/EchoShell/Terminal.swift

+
+import Foundation
+
+public struct Terminal {


internal struct ControlCharacter

good idea. Would keep it public though. Some users might want to create their own commands and having access to the set of control character might be very helpful.

That's fair

Joannis and others added 3 commits May 28, 2023 22:04

Basic server-side shell setup

4e0bb33

Small tweaks to comments + minimum NIOSSH

f946d9d

Development of EchoShell a simple shell emulator that returns the u…

0fd41e8

…ser input and offers some basic commands like: help, history, clear, whoami, date and exit.

Joannis requested changes Jun 20, 2023

View reviewed changes

Joannis force-pushed the feature/jo/shell branch from f946d9d to ddae52b Compare April 18, 2024 16:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Development of `EchoShell` - A simple shell emulator #39

Development of `EchoShell` - A simple shell emulator #39

gregorFeigel commented Jun 19, 2023

Joannis Jun 20, 2023

gregorFeigel Jun 21, 2023

Joannis Jun 20, 2023

gregorFeigel Jun 21, 2023

Joannis Jun 20, 2023

gregorFeigel Jun 21, 2023

Joannis Jun 20, 2023

Joannis Jun 20, 2023

Joannis Jun 20, 2023

Joannis Jun 20, 2023

gregorFeigel Jun 21, 2023

gregorFeigel Jun 21, 2023

Joannis Jun 20, 2023

gregorFeigel Jun 21, 2023

gregorFeigel Jun 21, 2023

Joannis Jun 23, 2023

Joannis Jun 20, 2023

gregorFeigel Jun 21, 2023

Joannis Jun 20, 2023

gregorFeigel Jun 21, 2023

Joannis Jun 20, 2023

Joannis Jun 20, 2023

gregorFeigel Jun 21, 2023

Joannis Jun 20, 2023

gregorFeigel Jun 21, 2023

Joannis Jun 23, 2023

Development of EchoShell - A simple shell emulator #39

Are you sure you want to change the base?

Development of EchoShell - A simple shell emulator #39

Conversation

gregorFeigel commented Jun 19, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Development of `EchoShell` - A simple shell emulator #39

Development of `EchoShell` - A simple shell emulator #39