NuSkooler / enigma-bbs

ENiGMA½ BBS Software
https://nuskooler.github.io/enigma-bbs/
BSD 2-Clause "Simplified" License
537 stars 104 forks source link

ToggleMenuView causes Engima to crash when connecting with ConnectBot #83

Closed davestephens closed 8 years ago

davestephens commented 8 years ago

When connecting into Enigma with ConnectBot (Android telnet/ssh client), if there are ToggleMenuView menu prompts the server will crash. This has been tested connecting to the telnet server running on 8888.

Environment

Expected behaviour The prompt should show and allow the user to select Yes/No accordingly.

Actual behaviour The server crashes, stack trace:

assert.js:89
  throw new assert.AssertionError({
  ^
AssertionError: false == true
    at ToggleMenuView.View.setPosition (/home/david/dev/enigma-bbs/core/view.js:135:2)
    at ToggleMenuView.View (/home/david/dev/enigma-bbs/core/view.js:57:8)
    at ToggleMenuView.MenuView (/home/david/dev/enigma-bbs/core/menu_view.js:20:7)
    at new ToggleMenuView (/home/david/dev/enigma-bbs/core/toggle_menu_view.js:17:11)
    at MCIViewFactory.createFromMCI (/home/david/dev/enigma-bbs/core/mci_view_factory.js:183:11)
    at entry (/home/david/dev/enigma-bbs/core/view_controller.js:139:35)
    at /home/david/dev/enigma-bbs/node_modules/async/lib/async.js:181:20
    at Object.async.forEachOf.async.eachOf (/home/david/dev/enigma-bbs/node_modules/async/lib/async.js:233:13)
    at Object.async.forEach.async.each (/home/david/dev/enigma-bbs/node_modules/async/lib/async.js:209:22)
    at ViewController.handleActionWrapper.createViewsFromMCI (/home/david/dev/enigma-bbs/core/view_controller.js:137:9)

Steps to reproduce Start Enigma, connect with ConnectBot, log in and proceed to the "global new scan" part of the login process.

NuSkooler commented 8 years ago

@davestephens Thanks for the report! I just toyed with ConnectBot a bit on my dev machine and pushed some changes which I believe should fix this.

ConnectBot's Telnet handshake sequence was a bit different than others which was causing enig to think that term size/height had not been init via NAWS when in fact it had been. ...which resulted in an assert being hit later on in the code.

Please pull and try again. If this works, I should probably clean up those asserts to be warning logs - they are left over from very early versions.

Closing. If it doesn't work for you, please re-open!

davestephens commented 8 years ago

Awesome, thanks! I actually had a bit of a tinker with it myself and could see the screen size was being initiated as 0x0 which was causing it to try and draw to row 0, although my JavaScript sucks so I hadn't quite got to the bottom of the client init code yet. :-)

Will give it a bash when I get in later today.

On 13 Aug 2016 17:08, "Bryan Ashby" notifications@github.com wrote:

@davestephens https://github.com/davestephens Thanks for the report! I just toyed with ConnectBot a bit on my dev machine and pushed some changes which I believe should fix this.

ConnectBot's Telnet handshake sequence was a bit different than others which was causing enig to think that term size/height had not been init via NAWS when in fact it had been. ...which resulted in an assert being hit later on in the code.

Please pull and try again. If this works, I should probably clean up those asserts to be warning logs - they are left over from very early versions.

Closing. If it doesn't work for you, please re-open!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/NuSkooler/enigma-bbs/issues/83#issuecomment-239628388, or mute the thread https://github.com/notifications/unsubscribe-auth/AAozMN5RfrpwWh-IyXbPnj3dGJ-xPA5Qks5qfewGgaJpZM4JjqPC .

davestephens commented 8 years ago

@NuSkooler Hmmm...this still doesn't seem quite right. I can now see Enigma correctly reporting the terminal size:

{"name":"ENiGMA½ BBS","hostname":"mint-desktop","pid":12802,"clientId":0,"level":20,"termWidth":128,"termHeight":44,"source":"NAWS","msg":"Window size updated","time":"2016-08-13T17:46:38.388Z","v":0}

...but for some reason it's still trying to write to row 0, which obviously doesn't exist and is causing the same stack trace as earlier. What's also suspect is that some other drawn prompts are also one line out, ie the username and password prompts in the login matrix are drawn one line above where they should be.

So...there's an off-by-one error somewhere, likely caused by ConnectBot doing something weird - I've just not found where just yet :-)

(I'm also unable to re-open the issue!)

NuSkooler commented 8 years ago

@davestephens Intersting -- I could cause the crash with ConnectBot before this mornings fix. What version of ConnectBot are you using and on what type of device (I was on a phone). I did notice the off by one positioning, but when I inspected what enig was doing it seemed OK so I figured it was ConnectBot rendering... I've yet to find a Android terminal app that handles ANSI-BBS well.

I'll try throwing ConnectBot on a tablet and see if that gets me somewhere.

NuSkooler commented 8 years ago

I take that back: I just got the crash you mentioned. Something to debug!

NuSkooler commented 8 years ago

OK, it seems ConnectBot is using 0,0 as "home" while ANSI-BBS expects 1,1. Will need to figure out a good way to handle this. I think some detection @ connect could do.

davestephens commented 8 years ago

Ah, that'd do it! I suspected it was something odd ConnectBot was doing given it doesn't officially support ANSI-BBS, only CP437.

NuSkooler commented 8 years ago

As part of d50e505bd70de65a47bafbaa99f8a750ca50382e, reworked connect to go home then detect CPR. If we get back 0,0 (vs non ANSI-BBS / standard 1,1) we'll set a offset to future CPRs.

davestephens commented 8 years ago

Awesome! Have pulled and confirmed this is now fixed.

Really appreciate your efforts on this, thank you.

On 14 August 2016 at 18:47, Bryan Ashby notifications@github.com wrote:

Closed #83 https://github.com/NuSkooler/enigma-bbs/issues/83.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/NuSkooler/enigma-bbs/issues/83#event-755092783, or mute the thread https://github.com/notifications/unsubscribe-auth/AAozMFDER1LngR91BOCyuoYxCCq2mLpEks5qf1TIgaJpZM4JjqPC .